WorkerDOM.upgrade should wait for Worker init. #1047
Conversation
| * @returns {Promise<void>} | ||
| */ | ||
| ready() { | ||
| return (this.worker as IframeWorker).readyPromise || Promise.resolve(); |
There was a problem hiding this comment.
In what circumstance would readyPromise be falsey if it's being initialized in the constructor?
There was a problem hiding this comment.
Worker will never have readyPromise (as well as WorkerImpl which is used in tests), whereas IframeWorker will.
| @@ -65,11 +71,14 @@ class IframeWorker { | |||
| fetch(this.url.toString()) | |||
| .then((res) => res.text()) | |||
| .then((code) => { | |||
| if ((event.data as MessageFromIframe).type == 'iframe-ready') { | |||
| const data = event.data as MessageFromIframe; | |||
| if (data.type == 'iframe-ready') { | |||
There was a problem hiding this comment.
Is there a reason to use a string literal instead of MESSAGE_TYPES.ready etc here? Wondering specifically if the latter might get compiled/minified better
There was a problem hiding this comment.
TBH I'm fine with it either way. As of now, we don't have the struct for it here. There's less of a need in TS when it is properly typed, whereas in JS its helpful to avoid typos
There was a problem hiding this comment.
Is there typechecking on string equality comparisons like this? I don't think there would be?
There was a problem hiding this comment.
Yep! The error looks like this:
This condition will always return 'false' since the types ... have no overlap
summary
Regular web workers w/ blob URLs are initialized synchronously, whereas the new
IframeWorkerintroduced recently is initialized async. In order to ensure all sent messages end up received we have two options:IframeWorkerand send once it has been completely initialized.WorkerDOM.upgradeto only resolve once the worker is ready.I've opted to do both in this PR