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

Make sure modules get the proper global object #183

Closed
tilgovi opened this issue Aug 4, 2015 · 3 comments
Closed

Make sure modules get the proper global object #183

tilgovi opened this issue Aug 4, 2015 · 3 comments

Comments

@tilgovi
Copy link

tilgovi commented Aug 4, 2015

I wish to raise this issue here. It is already being discussed in browserify/insert-module-globals#48 and browserify/browserify#1189 but it may impact browserify-shim. Alternatively, if no solution is merged in either of those projects, it may be that browserify-shim can provide a solution.

The problem arises easily is browserify-shim because the wrapper that browserify-shim inserts contains the symbol global. The presence of global causes browserify to insert module globals that results in insert-module-globals trying to inject the proper global object.

If self or global is defined already and is not the global object then insert-module-globals will inject the wrong thing.

It's possible for browserify-shim to fix this by setting both global and self to undefined in its wrapper, or perhaps setting global explicitly to window if that's appropriate. I think browserify-shim may be more browser-centric than insert-module-globals, and so the call to do this is maybe more sane here than reversing the detection order in insert-module-globals.

I am using browserify-shim and find it incredibly useful, so thanks to all the contributors. I use it heavily for browserify bundles that get injected as third party code and its invaluable for isolating my code from AMD loaders that might also be present on the page. Since this takes care of most of my isolation and global related needs, I thought I would see if perhaps browserify-shim would take care of this, too.

If so, I'm happy to write the patch.

@bendrucker
Copy link
Collaborator

PR is welcome. Careful about using window though. The tests run in Node so if you assume it exists you may break them (and other people's code).

@tilgovi
Copy link
Author

tilgovi commented Sep 23, 2015

I think we can close this here. It would be a very invasive, tricky, or impossible thing to get this right because we are putting a wrapper inside the browserify module wrapper, not around it.

@tilgovi tilgovi closed this as completed Sep 23, 2015
@bendrucker
Copy link
Collaborator

Agreed!

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

No branches or pull requests

2 participants