cujojs / when

A solid, fast Promises/A+ and when() implementation, plus other async goodies.
Other
3.44k stars 396 forks source link

Not working with browserify, minifyify, uglifyify etc... #367

Closed mariusk closed 9 years ago

mariusk commented 10 years ago

It seems the way this library uses require by reference breaks normal minification/obfuscation tools like browerify. In my specific case, when is pulled in by react-router, which after uglifyify or minifyify bombs with the following error message in the console:

Uncaught Error: Cannot find module './makePromise' 

Any way this could be changed to browserify (and similar tools based on the bytecode produced by uglifyjs if I understand it correctly) would be able to pick up these "require by reference" uses and treat them correctly?

briancavalier commented 10 years ago

@mariusk When.js works great with browserify and other tools. We use browserify to build the es6 shim, and we support a full browserify build for folks who want/need it.

It sounds like you're using when's es6 Promise shim (when/es6-shim/Promise.js), which has already been browserified using the standalone option, which is the recommended way to do it. So, you shouldn't need to browserify that file again. Have a look at that thread and let us know if that helps.

briancavalier commented 10 years ago

Ah, in fact I just found substack/node-browserify#885 that deals with exactly this issue--it's even in reference to when.js specifically. It looks like there is a solution there. Hope that helps!

mariusk commented 10 years ago

Pasting in my comment on the derequire solution suggested by briancavalier above, which I posted on the referenced closed thread:

I'm not sure derequire is a solution here. The docs I've read is to run it after browserify has run, but by then there are no longer any references to dereq or similar, it's all been minified to "e(..)" calls or similar. Also keep in mind I'm not directly using this library; it's pulled in by react-router. So does this mean the "correct" place to fix it would be in react-router? Alternatively I guess I could search for a browserify-derequire module (which gets a few hits), but they are all undocumented and not in the standard npm repo which makes me a bit skeptical. A solution right now is just to run uglifyjs AFTER running browserify (without any minification), but that is not ideal either (for a couple of reasons).

vvo commented 9 years ago

var Promise = require('when/es6-shim/Promise.browserify-es6'); works for me, is that a nice way?

briancavalier commented 9 years ago

@vvo yes, absolutely, that should work just fine. I wonder if that's the solution: use the pre-browserified module (when/es6-shim/Promise) if you want/need a pre-browserified file, and use when/es6-shim/Promise.browserify-es6 if you want an ES6 shim, and want to browserify it youself (possibly along with the rest of your app). If that would be a good solution, then we could probably rename Promise.browserify-es6 to something a little easier to type :)

@mariusk, any thoughts on that?

mariusk commented 9 years ago

As long as the solution does not require anything but including that module explicitely before any other module that uses it (like my use-case with react-router) that sounds like a great solution. A better solution would make this kind of fiddling unecessary, but I am not qualified to say if this is possible currently. Regardless, thanks for looking into it.

briancavalier commented 9 years ago

@mariusk Yeah, we'll make sure that still works.

Promises are in a tricky spot right now in general, that's the nature of the fiddling right now, unfortunately. The problem is that forward-looking code should depend only on the implicit Promise global, but that's not practical yet, since not all environments support it natively. Perhaps just as importantly, in those that do, their Promise impls are slow and/or buggy, and certainly not battle-tested. So, we're doing our best to support the various ways someone might want to have the implicit global created (script tag, module loading side-effect, etc.)

I'm going to close this issue, but open a new one specifically to address the naming issue for when/es6-shim/Promise.browserify-es6 so we can make it easier for folks who want an ES6 shim that isn't pre-browserified*.

Thanks for the discussion everyone!