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

weird r.js optimization error #514

Closed
majodev opened this issue Aug 21, 2013 · 4 comments
Closed

weird r.js optimization error #514

majodev opened this issue Aug 21, 2013 · 4 comments

Comments

@majodev
Copy link

majodev commented Aug 21, 2013

Hi!

I'm running into troubles optimizing my project with r.js and a third-party lib leapjs. I'm not quite sure what is exactly going on, but it seems that require runs into some sort of racing-condition with leapjs, but only at the optimized version. Also tried almond.js and various config flags without any success.

The global object "Leap" isn't available when the first define occurs in the optimized code.

error_after_build

I've uploaded an example project for further analysis: https://github.com/majodev/leap-requirejs-optimization-error

Hope you can help me. :)

@paulirish
Copy link

cc @joshbuddy

@majodev
Copy link
Author

majodev commented Aug 23, 2013

OK, I have further debugged the error and compared the development and build code during runtime. It seems that the shim configuration for the Leap module is ignored at the optimized version. I've tracked it to the fetch function within require.js (line 807):

//If the manager is for a plugin managed resource,
//ask the plugin to load it now.
if (this.shim) {
  context.makeRequire(this.map, {
    enableBuildCallback: true
  })(this.shim.deps || [], bind(this, function () {
    return map.prefix ? this.callPlugin() : this.load();
  }));
} else {
  //Regular dependency.
  return map.prefix ? this.callPlugin() : this.load();
}

this.shim is false for the Leap object (this) at the optimized version while it contains the respective shim config object for Leap during the development version. To further demonstrate, that I'm using the same require.config I've updated the build.js script to use mainConfigFile: "scripts/main.js". However, it results in the same error as before.

bildschirmfoto 2013-08-23 um 17 42 33
bildschirmfoto 2013-08-23 um 17 43 02

Any ideas?

@jrburke
Copy link
Member

jrburke commented Aug 28, 2013

So the issue is with the construction of the node_modules/leapjs/leap.js file: it looks like it is a file generated by a browserify command, which is not aware of AMD modules. However, gl-matrix, registers as an anonymous define call (in gl-matrix/dist/gl-matrix.js, and that anon define call gets seen as the Leaf definition by the r.js optimizer.

So, not sure yet how to proceed. Ideally leafjs would not use browserify to combine all the files, and you could intake the sub-parts of the leaf library in source form. However, that requires a bunch of social agreement on module formats by the leafjs team and sub dependencies.

That, or if you could try to convert the plain node modules that are installed with an npm command to AMD, by using volo's npmrel command. But that also means installing the browserify shims for native node modules in the baseUrl, and probably involves a requirejs map config to get it loaded.

I could try to detect when a browserify bundle section is in play and not parse inside of it. However, the grawlix browserify dumps at the top of the file does not give me confidence that the structure/format of it will stay consistent over time. Plus, it should probably just use AMD as the combo format. But I can appreciate that is probably a rougher sell for browserify given the mismatch between how node's require works and what is sane for dynamic loading in the browser.

The other option is for browserify to strip out define calls internally to files it combines, but I am not sure if that project is interested in figuring that out.

I will be happy when ES6 modules land. And/or when people figure out npm is not a great fit for modular, dynamic front end code, and using bundles of browserify bundles is not a great idea for the dynamic front end either.

In the meantime, I was able to hack around the problem by using this config in the scripts/build.js:

var config = {
  baseUrl: "src/",
  paths: {
    "Leap": "../node_modules/leapjs/leap"
  },
  optimize: "none",
  onBuildRead: function(id, url, contents) {
    if (id === 'Leap') {
        return 'define(\'Leap\', [], function() {var define;\n ' + contents + ' return Leap; });';
    } else {
        return contents;
    }
  },
  logLevel: 0,
  name: "issue", // Name of script to start building from
  insertRequire: ["issue"],
  out: 'build/' + pjson.name + '-' + pjson.version + '.min.js' // Where to output
};

I would like the story to be smoother here. Even though it sucks to have jump through more hoops to shoehorn code from npm into modular, dynamic front end development, I think it could be something like volo's npmrel, plus auto install of some native node shims, but bundle that in a reusable node package. I have something in my queue for that, but not sure when I will get to it yet, and it will be a separate project from this one.

So I will close this for now as it will not be addressed by the optimizer, but more by package install-related tools. Feel free to continue discussion here though.

@jrburke jrburke closed this as completed Aug 28, 2013
@majodev
Copy link
Author

majodev commented Aug 30, 2013

Hi James. Thank you very much for figuring out how to use leap.js with your awesome optimizer. I would really love a more streamlined landscape at using modules, especially in an AMD way, however I also feel it won't happen until ES6.

Anyway, keep up your great work, I'm going to cover requirejs and r.js in my master thesis. :) Greetings from Austria.

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

No branches or pull requests

3 participants