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

Enable including libraries with a preamble in the browser directly #29

Merged
merged 2 commits into from
Feb 22, 2023

Conversation

mnito
Copy link
Contributor

@mnito mnito commented Apr 9, 2022

Mutating the self variable with Object.create on the window global makes objects included on js.context within Dart unavailable in the browser environment. Fix this issue by only preventing encapsulation via Object.create when necessary and not in the browser.

Additionally, browsers tend to not have CommonJS globals available. To prevent ReferenceErrors when running in the browser, check if these globals are available before setting them on self.

EDIT: Did a slightly larger refactor of the Node.js and global check with the second commit, to address #28 and support service workers

Mutating the self variable with Object.create on the window global
makes objects included on js.context within Dart unavailable in the
browser environment. Fix this issue by only preventing encapsulation
via Object.create when necessary and not in the browser.

Additionally, browsers tend to not have CommonJS globals available. To
prevent ReferenceErrors when running in the browser, check if these
globals are available before setting them on self.
Use globalThis which is the standard way to access the global this

Check for Node.js explicitly to eliminate specific service worker
and Electron checks
@mnito mnito changed the title Enable including libraries with a preamble via the HTML script tag Enable including libraries with a preamble in the browser directly Apr 9, 2022
@nex3 nex3 merged commit ac443f5 into mbullington:master Feb 22, 2023
@nex3
Copy link
Collaborator

nex3 commented Feb 22, 2023

Thanks for the fix! Sorry for the long delay!

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.

None yet

2 participants