Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace self with globalThis for use in Worklets #617

Merged
merged 1 commit into from
Feb 1, 2023

Conversation

mwszekely
Copy link
Contributor

@mwszekely mwszekely commented Feb 1, 2023

This is to fix runtime errors that occur when Comlink is used in a Worklet because self doesn't exist there.

I ran into a problem suddenly using Comlink in an AudioWorklet getting runtime errors on line 425 because it's looking for the existence of a property on the self object -- since self isn't available in Worklets I just replaced it with globalThis and it resolved the issue.

(The suddenness of it was just an unrelated tree-shaking issue -- if expose is optimized out by whatever bundler you're using then the Worklet never inspects self, and never crashes. It's possible that this issue could suddenly affect others if they change how they use Comlink in their projects, even without calling expose) I completely misunderstood the timing and presumably need to get some sleep 🙃

@benjamind benjamind merged commit cdf89e1 into GoogleChromeLabs:main Feb 1, 2023
@benjamind
Copy link
Collaborator

Nice work thank you I believe this should also fix #616. I'll get a nodejs test written for this tomorrow and get a patch release out.

@Kasthooriraja
Copy link

Replacing **self** with **globalThis** creates problems in older versions.

var getGlobal = function () { if (typeof self !== 'undefined') { return self; } if (typeof window !== 'undefined') { return window; } if (typeof global !== 'undefined') { return global; } throw new Error('unable to locate global object'); };

Selecting the available global object using the above function will resolve this issue.

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