Closed l0rdn1kk0n closed 5 years ago
Also it might be tough but it would be nice if we could get some tests for this since this piece seems brittle.
Hmmm. It is hard to discern the changes made here - perhaps you could arrange the diff so that it is more obvious to see the changes? A few things have been moved around...
Perhaps you could also explain the intention of the change to help read the code. It isn't obvious to me.
As stated in https://github.com/webjars/webjars-play/issues/28#issuecomment-33906697, I think that the real fix here is in RequireJs. What'd think?
@l0rdn1kk0n - may be you could raise a PR for https://github.com/jrburke/requirejs/issues/1013 if you agree.
there's a possible race condition when using require.callback
to add a plugin because it will be called during configuration of the requirejs context as a require([],callback)
call. In my solution the plugin is added after requirejs was loaded (and before first app based require/define call). I'll reformat the code a bit that the changes can be reviewed a bit easier.
@huntc i'll check jrburke/requirejs#1013 and see what i can do.
i've read your discussion and according to @guybedford suggestion we could use the define('myModule', ['webjars!myModule'],...);
way. The only problem i see with this solution is, that we would have to rewrite all webjars (don't think that this is a good solution). But there's another option, i'm still not happy with it because an additional 404 error, but it seems to work. Require.js allows overriding the global onError
method which we could use to redirect all unavailable modules to the webjars plugin (if they are defined there) and if load was successful define('myModule', ['webjars!myModule'],...);
gets called.
/**
* The global error callback is called whenever a module can't be loaded.
*
* @param {Object} err the require.js error object
*/
requirejs.onError = function onErrorCallback(err) {
var failedId = err.requireModules && err.requireModules[0];
if (failedId && (err.requireType === 'nodefine' || err.requireType === 'scripterror')) {
// clear internal knowledge of 'failedId'. Any modules
// that were dependent on "failedId" and in the middle of loading
// will not be loaded yet, they will wait until a valid 'failedId'
// does load.
requirejs.undef(failedId);
var fullPath = getReverseFullPath(failedId),
webjarsId = 'webjars!' + failedId;
if (fullPath) {
// retry with webjars plugin loader, Note that the original require callback
// that has requested 'failedId' will be called if this new attempt to load
// succeeds.
require([webjarsId], function (exportedVar) {
// define an alias to originally requested module
define(failedId, [webjarsId], function () {
return exportedVar;
});
});
return;
}
}
throw err;
}
@guybedford is that what you meant?
We are already using the define method...
I'd rather avoid hacking this plugin and see that requirejs is enhanced to support the module loader syntax in shim dependencies. This would make issues disappear and potentially simplify our code.
I have a possible new way to do RequireJS with WebJars. Check it out: https://github.com/webjars/webjars-locator/pull/40
this is probably not relevant anymore. Last comment dates from 2014
Thanks @l0rdn1kk0n! This looks good but I'd like @huntc to review it.