Skip to content

Conversation

@rekmarks
Copy link
Member

No description provided.

@rekmarks rekmarks requested a review from a team as a code owner October 10, 2024 20:01
@rekmarks rekmarks force-pushed the rekm/offscreen-changes branch from 11d04ae to 0f867c0 Compare October 10, 2024 20:01
}
})(),
kernelWorker.receiveMessages(),
return undefined;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We trigger an ESLint rule without this.

await Promise.all([
(async () => {
kernelWorker.receiveMessages(),
kernelInitKit.promise.then(async () => {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now we only await the init promise once.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice.

Comment on lines +104 to +105
logger.error('Received unexpected reply', message);
continue;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was this downgrade in preparation for some future change? We never seem to hit this condition on your branch.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it's an artifact of a hypothetical era. Since the worker stream is typed and guarded we could just remove this check altogether.

{ method: typeof KernelCommandMethod.InitKernel }
>['params']
>();
const kernelInitKit = makePromiseKit<void>();
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing actually used the resolved value of this promise anyway.

@grypez grypez merged commit 4113bec into grypez/move-kernel-to-dedicated-worker-with-message-channel Oct 10, 2024
@grypez grypez deleted the rekm/offscreen-changes branch October 10, 2024 21:21
grypez pushed a commit that referenced this pull request Oct 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants