support being evaluated with "use strict" prefixed

[jakutis]

Fixes use case:

var build = new Function('module', '"use strict";\n' + <string contents of /src/bean.js>);
var bean = {exports:{}};
build.call(window, bean);
bean = bean.exports;

This use case is a minimal working example extracted from my custom module loader for the browser (think of yet another browserify).

[rvagg]

Sorry, I'm not going to add any more crazy to this crazy boilerplate, I'm tempted enough to pull out AMD support as it is. Perhaps you can write a wrapper that does what you need instead?

[jakutis]

In strict mode this at line 7 is undefined (not a window and thus context[name] throws) and there is no way to fix it without modifying the source code.

I could write a wrapper just for bean, but bean is the only module among dozens of others I load that would need a special wrapper.

I don't need to touch AMD support. Making only this change accfc729fa1857bd9db2749131091f1df73d3512 fixes the issue - I just evaluate that code with this set to window - as in the example code in the description of this issue.

[rvagg]

@ded help! I get pain in my head just looking at those top 5 lines these days, I just want to rip it out completely. So I need a second opinion here.

[jakutis]

As I see modules by @ded (bonzo, valentine, domready, klass, reqwest):

• everyone of them have essentially the same top 5 lines as bean
• neither of them has .noConflict() and context[name] inside as bean

So my thinking is that to have .noConflict(), bean should adjust it's module system support.

[jakutis]

Another way to fix this issue just popped into my head - 0c09d5f5dbc17467e1f1fa9125f79e6755cd699e - this is the order that @umdjs uses. This would work, because I could make my module loader pass define which evaluates definition with this set to some object.

So now I have two proposals for adjusting bean module system support:

• accfc729fa1857bd9db2749131091f1df73d3512 just change module.exports = definition() to module.exports = definition(name, context)
• 0c09d5f5dbc17467e1f1fa9125f79e6755cd699e just swap the order of AMD and CommonJS checks (essentially just swapping lines 2 and 3) (as in umdjs/umd)

[ded]

Well, don't rip it yet. Let me have a look with fresh eyes in the morning

[ryanve]

@jakutis context can be any object. I suggest you use build.call({}) and then assign to window if needed. definition only uses context for noConflict()

[ryanve]

I suspect (name, context) as parameters is problematic for AMD because define(definition) assumes default dependencies. name would be require. context would be exports. If noConflict() is needed, I'd create it up top with the logic or move those to the var statement.

var name = 'bean'
  , context = this

[jakutis]

@ryanve, the problem with current /src/bean.js is that when it is evaluated in strict mode (with "use strict";) there is no way for context inside definition be anything other than undefined (and when it is undefined the line 10 throws!) - this at line 7 of /src/bean.js in strict mode is always undefined - if I do build.call(anything), definition does not receive anything because of lines 2-4 of /src/bean.js.

To sum up, in strict mode context in definition is always undefined - whether with build.call(window) or build.call({}).

By the way, I will still do build.call(window), because this same code snippet build = new Function(...); etc.. is used for all modules and some depend on this being window. I can paste the exact lines from my module loader:

module.builder = new Function('define', 'global', 'process', 'module', 'exports', 'require', '"use strict";\n' + code + '//# sourceURL=' + realpath + '\n');
...
module.builder.apply(window, [define, window, process, module.commonjs, module.commonjs.exports, r]);

Here code will be the contents of /bean.js.