thlorenz / browserify-shim

📩 Makes CommonJS incompatible files browserifyable.
MIT License
933 stars 87 forks source link

Make sure modules get the proper global object #183

Closed tilgovi closed 8 years ago

tilgovi commented 9 years ago

I wish to raise this issue here. It is already being discussed in substack/insert-module-globals#48 and substack/node-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 commented 8 years ago

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 commented 8 years ago

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.

bendrucker commented 8 years ago

Agreed!