Use postMessage instead of a comlink produced MessageChannel #148
Conversation
|
|
| super(); | ||
| this._sendWorkerMessage = (msg: any) => { | ||
| // use postMessage, but in a format, that comlink would not process. | ||
| postMessage({ jMsg: msg }); |
There was a problem hiding this comment.
Wondering if something a bit more explicit like _kernelMessage could also work? With or without the leading _, but having it could help communicate it's used for "internal" message handling?
There was a problem hiding this comment.
Ok, but then also the coincident worker needs to rename the change.
There was a problem hiding this comment.
I meant for the jMsg key, we could still keep _sendWorkerMessage as the method name:
| postMessage({ jMsg: msg }); | |
| postMessage({ _kernelMessage: msg }); |
There was a problem hiding this comment.
Ok, got it. and it is also unique enough that comlink ignores it. (comlink ignores it if no id field is set...)
yarn.lock
Outdated
| version: 4.4.1 | ||
| resolution: "comlink@npm:4.4.1" | ||
| checksum: 16d58a8f590087fc45432e31d6c138308dfd4b75b89aec0b7f7bb97ad33d810381bd2b1e608a1fb2cf05979af9cbfcdcaf1715996d5fcf77aeb013b6da3260af | ||
| languageName: node | ||
| linkType: hard | ||
|
|
||
| "comlink@npm:^4.4.2": |
There was a problem hiding this comment.
Looks like the two versions of comlink could be deduplicated with:
Line 20 in fb9c10d
There was a problem hiding this comment.
Ok, I will try this. I am not so fluent with yarn, I normally use npm. I'll be back in the evening at my dev computer.
There was a problem hiding this comment.
Running jlpm deduplicate at the top-level should normally perform the deduplication step.
|
OK, I have included the suggested changes. The deduplication affected more than just the comlink package, I hope this is fine. |
Closes #144
Fixes #133
Fixes jupyterlite/jupyterlite#1465
Fixes jupyter-book/thebe#771
Fixes jupyter/try-jupyter#54