require() should not accept configuration as a first argument
kitsonk opened this issue · 4 comments
According to the AMD Spec, require() should not take a configuration as part of its argument.
While there is no specification for how to configure a load, the common configuration is defined. By convention though, it seems RequireJS uses require.config() to pass the configuration and the dojo/loader already exposes it as require.config() as well.
Since it isn't required to be AMD compliant, I suggest we drop it, as it makes the API confusing and puts extra run-time code to try to disambiguate the call signatures.
PR raised to add functionality of passing config to plugin load function exposed that when calling require.config(config) followed by require(dependencies), the configure function would be called twice, the second time with an empty object. I think we've got lucky so far that this hasn't caused problems.
Merge of config still occurs and merge test passes if two calls are made to require.config rather than one to require.config and the second to require with config as the first argument.
On inspection of the requireJS code, it exposes it's require.config function but also accepts config as the first argument to require. In fact, the require.config function simply points at require passing only config. I think it's better if we just remove support for this from our loader. Most apps should be using require.config anyway. - our only loader test that was passing config to require is the aforementioned merge test.
I suspect accepting config as the first argument was both a legacy for RequireJS and the Dojo loader. It obviously can cause confusion. I would prefer we adhere to the spec, which means it shouldn't be passed as the first argument.
I can't personally thing of a decent use case for it that we wouldn't want to rewrite to make more clear via require.config().