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

umd-packaged modules don't work with packaging systems that detect the require calls and assume they are global #10

Closed
petehunt opened this issue Dec 4, 2013 · 9 comments

Comments

@petehunt
Copy link

petehunt commented Dec 4, 2013

UMD-packaged modules are designed to work in any environment. Unfortunately they don't work in very many client-side packaging systems (I tried browserify and requirejs, webpack works).

Here's the scenario: ./x.js requires ./y.js and we do browserify --standalone x x.js > ./temp/a.js. If I require a from Node or drop it in a browser everything works. Great.

Now I create ./temp/b.js which requires ./a.js and I do browserify b.js from ./temp/. It throws with Error: Cannot find module './y' because the word require() is used within the source still.

This is common behavior in many of the popular browser packaging systems I mentioned. They just look for calls to a function named require().

My solution to this is to syntax transform the require literal when used as a function argument or function call to muffin. Would a pull request that does something like this (with a more mangled identifier) be accepted? It would require esprima.

@andreypopp
Copy link

+1, UMD isn't Universal Module Definition without that.

@ForbesLindesay
Copy link
Owner

Agreed, we should probably do this via uglify-js since we already depend on that.

@cburgmer
Copy link

I don't really understand the muffinization here. Wouldn't it be browserify's job to make sure that requires get properly treated? AFAIK umd does not know what content gets embedded between pre- and postlude.

I think what we are missing is something in umd to support generating code such like https://github.com/umdjs/umd/blob/master/returnExports.js#L25 with an easy interface for browserify to tap into.

@ghost
Copy link

ghost commented Dec 10, 2013

This is probably something that detective should handle, similarly to how in insert-module-globals there are checks to make sure the expected names aren't from local definitions using lexical-scope.

@petehunt
Copy link
Author

browserify isn't the only packaging system that gets bitten by this though. It would be nice if any consumer of this module could have this fixed for them. I'm with @andreypopp that UMD isn't universal unless require is removed from the source (though I know this makes the module considerably more complex). But you guys know more about these projects than I do. It would be nice to get an idea of if you guys think this is worth fixing so I can decide whether to wait for it to be fixed or if I should apply a post-packaging transform to React.

@ForbesLindesay
Copy link
Owner

I think it's worth fixing but it will be a while before I get any time to do it myself.

@calvinmetcalf
Copy link

I actually wrote a very rough grunt task to do this, don't use it in production, it's terrible, but it works and could serve as a starting off point.

edit, extracted the relevant code into it's own repo

@mikermcneil
Copy link

@calvinmetcalf thanks for that 👍

@substack makes sense! @petehunt maybe umd calls out to detective?

@ForbesLindesay ForbesLindesay changed the title umd-packaged modules don't work with many packaging systems umd-packaged modules don't work with packaging systems that detect the require calls and assume they are global Feb 4, 2015
@ForbesLindesay
Copy link
Owner

See #11. I think this is best left as out of scope for this module.

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 a pull request may close this issue.

6 participants