CommonJS wrapper doesn't like being bundled #833
Description
I have a third party script which is an optimized AMD file that contains a CommonJS wrapper:
// bar.js
define('bar', function(require) {
var foo = require("./foo");
});
define('foo', function () {});When I try to bundle this file into my own file:
// app.js
define(['bar'], function () {});r.js doesn't notice that foo is already defined within the bar file, leading to:
>> Error: ENOENT, no such file or directory
>> '/Users/OliverJAsh/Development/frontend/foo.js'
>> In module tree:
>> appI'm guessing this is because r.js has to perform static analysis of the CommonJS wrapper. What do you recommend to work around this issue?
Activity
jrburke commentedon Jul 17, 2015
Sounds like it could be two different things:
If the file is a result of some other concatenation process and it has a UMD wrapper around it, ideally the optimizer would not be diving inside the inner part of the UMD wrapper that had these define() calls if they are not publicly visible. If that sounds like the issue, getting a more complete look at the structure of the file would help. The optimizer does try to avoid looking inside UMD wrapped files, but as is the case with static analysis of JS, there can be multiple ways to construct the UMD wrapper and that particular signature may creep by the existing analysis.
If there is no wrapper around the define() calls, and the examples are basically the contents minus the actual factory function definitions, I see that failing locally too, and I will want to fix that part. Looks like it will actually be a fix in requirejs to account for items in the define() queue.
The main bug in 2) is that it should not really matter what order the define() calls are in, but unfortunately looks like a bug got in where the order of dependencies is assumed to be least dependencies to more dependencies (so assuming define('foo') would come before define('bar') in this example).
As for workarounds, if it is just an issue of a build and the only issue is 2), rawText config could be used to seed the 'foo' value. It would likely mean a duplicate define('foo') in the output, but that is fine. However, if the real code has an actual factory function definition for foo, and it is not just a
function(){}stub, then that will not be the path to take.You could use the onBuildRead/onBuildWrite hooks to seed some stub define('foo') at the top of the file with onBuildRead just to get it the optimizer to see it first, then maybe remove that stub with onBuildWrite.
Or if you have control over how that bar.js file concatenates the modules, ordering them where leaf dependencies are first should avoid the bug.
However, if there is a UMD wrapper around the define() calls (issue 1), then it would be good to get a more fleshed out example of the file's structure.
OliverJAsh commentedon Jul 17, 2015
For context, the actual file I'm trying to bundle is https://github.com/cujojs/when/blob/master/es6-shim/Promise.js. The problem starts at https://github.com/cujojs/when/blob/ed92496f0db26f72b3471b65440023b42d92cedd/es6-shim/Promise.js#L24.
jrburke commentedon Jul 17, 2015
Thanks for the example file. It was indeed the UMD detection not correctly identifying the UMD wrapper. I filed requirejs/requirejs#1389 to address the separate, unrelated issue of allowing out of order define() calls (the 2) issue above), just happened across that separate bug while looking into this one.
The fix for this issue will be in 2.1.20. I am not sure yet when I will publish it, but given that UMD wrapped files have been showing up more often, I am thinking on the order of a week or so. Do not want to go too long without this fix going out. I may even get it out this weekend, just want to scrub through some other open issues before locking down the release.
If you want a fix today, the r.js master snapshot contains the fix:
https://raw.githubusercontent.com/jrburke/r.js/master/dist/r.js
For this specific fix on top of 2.1.19 (where the above link will change as master changes):
https://raw.githubusercontent.com/jrburke/r.js/3407e2d45ff6dfd268b8e3e13f7c712180daf6ad/dist/r.js
OliverJAsh commentedon Jul 19, 2015
Thank you very much for the fast response! :-)
On Fri, 17 Jul 2015 at 18:17 James Burke notifications@github.com wrote: