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

--insert-globals option doesn't work #655

Closed
shuhei opened this issue Feb 15, 2014 · 4 comments
Closed

--insert-globals option doesn't work #655

shuhei opened this issue Feb 15, 2014 · 4 comments

Comments

@shuhei
Copy link

shuhei commented Feb 15, 2014

This issue doesn't break anything because globals are inserted anyway if they are found in source code. Also, detecting globals are now very fast thanks to mine.

However, cli's --insert-globals option doesn't seem to be working. It fails as follows:

  1. browserify --insert-globals=true foo.js
  2. bundle()'s insertGlobals option becomes a string "true".
  3. insert-module-globals module gets the string "true" as always option.
  4. It strictly compares the string "true" against the boolean true and doesn't insert globals.

https://github.com/substack/insert-module-globals/blob/34e5fb7167b858b13ffcc2c73a7827045f2e478e/index.js#L57

This kind of issues can happen to other options too. I couldn't figure out where the right place to fix is, so just opening this issue.

@shuhei
Copy link
Author

shuhei commented Feb 15, 2014

Also, this issue is obscuring issues related to insertGlobals. Some issues happen on API, but don't on CLI.

@andreypopp
Copy link
Contributor

What if you run browserify --insert-globals foo.js? I think it should be a boolean option.

It strictly compares the string "true" against the boolean true and doesn't insert globals.

That probably should be fixed though.

@shuhei
Copy link
Author

shuhei commented Feb 15, 2014

What if you run browserify --insert-globals foo.js?

Then it becomes { insertGlobals: "foo.js" } and no entry.

I agree that it should be { insertGlobals: true } though.

@ghost
Copy link

ghost commented Jul 25, 2014

In v5 --insert-globals is a boolean option so this issue should be fixed.

@ghost ghost closed this as completed Jul 25, 2014
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants