Calling a strict-scoped function defined via eval in a non-strict context forgoes it's closure

[amasad]

• Create foo.js and add:

var code = [
'function x() {',
  '"use strict";',
  'console.log(x);',
'}',
'x();'
].join('\n');

eval(code);

• Run using node v0.10.x or v0.11.x node foo.js
• Expected output: [Function x]
• Actual output:

undefined:3
console.log(x);
            ^
ReferenceError: x is not defined

This repros in v0.10.x and v0.11.x (including v0.11.14) but not in v0.8.x. Additionally, this doesn't repro in latest stable chrome.

[joliss]

I can reproduce this on Node v0.10.33, and interestingly it only seems to happen when you run node foo.js. If you paste it in interactive mode, it works as expected:

> var code = ['function x() {', '"use strict";', 'console.log(x);', '}', 'x();'].join('\n'); eval(code);
[Function: x]

[WebReflection]

FWIW this works as expected for me in OSX and v0.10.28

[WebReflection]

@joliss … true that, it also works in foo.js if you set everything as strict:

"use strict";
var code = [
'function x() {',
  '"use strict";',
  'console.log(x);',
'}',
'x();'
].join('\n');

eval(code);

interesting bug!

[amasad]

I should mention, my use case is I'm testing a compile-to-js so I'm eval'ing the compiled code.

@joliss that's an interesting find, I think it's because the repl uses the vm module. If you create another file, call it bar.js:

var fs = require('fs')
var vm = require('vm')
var script = vm.createScript(fs.readFileSync('foo.js').toString())
script.runInThisContext()

Now, it works fine. So it has to do with the way node cli runs scripts

[a0viedo]

I'm able to reproduce this on branches v0.10 and v0.11 but not in branch v0.8.

[jasnell]

Interestingly, this doesn't happen within the repl. @joyent/node-coreteam ... thoughts?

[jasnell]

@domenic ... any thoughts? @a0viedo ... can you give this a quick test on the latest io.js?

[a0viedo]

@jasnell Using Node 0.10.39, 0.12.5 and iojs 2.3.1 the result is the same:

undefined:3
console.log(x);
            ^
ReferenceError: x is not defined

[trevnorris]

Just built and ran from io.js next branch. Which uses V8 4.4. Got same error.

/cc @domenic This also affects io.js.

[domenic]

That's bizarre. I wonder what we are doing to confuse V8 about this.

Maybe it has to do with how all Node code is wrapped in a (sloppy) module-closure?

[trevnorris]

@domenic Good point. I'll test the code in d8 and see if we get the same result.

[trevnorris]

@domenic I just ran the following slightly modified example on latest d8 4.2:

var code = [
'function x() {',
  '"use strict";',
  'print(x);',
'}',
'x();'
].join('\n');

eval(code);

And there was no issue. Though I tried the following:

var code = [
'(function (exports, require, module, __filename, __dirname) {',
  'function x() {',
    '"use strict";',
    'print(x);',
  '}',
  'x();',
'\n})();'
].join('\n');

eval(code);

Which also didn't have an issue. Thoughts?

[domenic]

Well, the latter should not evil the wrapper, to better emulate io.js. But yeah, if that doesn't show the difference, I've got nothing off the top of my head...

[trevnorris]

Another slight variation that does work:

var code = [
  '"use strict"',
  'function x() {',
    '"use strict"',
    'console.log(x);',
  '}',
  'x();',
].join('\n');
eval(code);

I tested this all the way back to v0.10 and it succeeds with the extra 'use strict' put in. This must be something to do with variable scope. The following also succeeds:

var code = [
'(function() {',
  'function x() {',
    '"use strict"',
    'console.log(x);',
  '}',
  'x();',
'}())',
].join('\n');
new Function(code)();

I'm out of ideas. Though this does appear to be a bug.

[domenic]

To be clear, now that I'm at a computer, I was suggesting testing

(function (exports, require, module, __filename, __dirname) {

var code = [
  'function x() {',
    '"use strict";',
    'print(x);',
  '}',
  'x();',
].join('\n');

eval(code);

}();

since that is closer to how modules work. But yeah, my next guess is that vm.runInThisContext (which is used for modules, I believe) is responsible.

[amasad]

Actually, it worked using the vm module. See my earlier comment #8721 (comment)

[domenic]

Right, but I mean that Node executes modules via vm.runInThisContext('wrapper stuff' + fs.readFileSync(...) + ' wrapper stuff'), instead of directly like d8 does.