Node: Array and Object have different values in REPL and in files

[calebsander]

• Version: v6.3.0
• Platform: 4.4.0-31-generic #50-Ubuntu SMP Wed Jul 13 00:07:12 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux

I have one file, say is.js, that contains the following:

exports.isObject = (obj) => obj.constructor === Object
exports.isNumber = (obj) => obj.constructor === Number

In the console, require('./is.js').isObject({}) returns false. However, ({}).constructor === Object returns true. Strangely, isNumber(2) works as expected. When required from another file, everything works as expected.

Affected classes:

• Object
• Array
• Date
• Error
• RegExp

Unaffected classes:

• Number
• String
• Boolean
• Map
• Set
• Symbol
• Typed arrays

[cjihrig]

https://github.com/nodejs/node/pull/5703 may be the cause of this.

[addaleax]

Yep, confirmed by bisecting.

[cjihrig]

I opened #7793 as a known issue test. Once this is resolved, the test can be moved to the normal test suite.

[addaleax]

Tbh I think reverting #5703 might be the best idea for now, this doesn’t seem like something that’s easy to resolve… @lance?

[cjihrig]

Revert is in #7795. I can combine the test from #7793 into that PR.

[lance]

@addaleax @cjihrig reverting makes sense, I suppose. It's not obvious to me why this is happening at the moment.

Since https://github.com/nodejs/node/pull/5703 was also a fix for https://github.com/nodejs/node/issues/6802 should this be reopened?

[cjihrig]

Yes, it should be reopened if the revert PR lands.

[cjihrig]

@lance as a side note, I believe the problem is that require() runs code in the global context, and when useGlobal is false, the code run in the CLI executes in a different context.

[lance]

@cjihrig @addaleax - I think the problem is instead with some of the code here: https://github.com/nodejs/node/blob/master/lib/repl.js#L602-L618. In fact, @cjihrig, your known issue test will continue to fail even if we revert https://github.com/nodejs/node/pull/5703 won't it? Because reverting that PR only causes the default to change and that's all. The known issue test is explicitly setting useGlobal to false so the code path will be identical after the revert. The behavior will be mitigated in the CLI REPL by a revert, but I don't that fixes the underlying cause.

I think that perhaps this issue is related to https://github.com/nodejs/node/pull/7369/files#r68335215.

[cjihrig]

The known issue test shows the relationship between useGlobal: false and require(). The revert PR in #7795 has a slightly modified test that is tied to the CLI REPL. That test currently fails, and passes with the revert. The known issue test will live on independent of the revert.

[lance]

/me should have read the revert PR first :)

Still, I think it would be better to figure out what the real problem is and fix that rather than revert which just masks the problem. The issue doesn't go a way with a programmatic REPL started with { useGlobal: false }.

[addaleax]

https://github.com/nodejs/node/issues/7788#issuecomment-234074155 – Object refers to different things in different vm contexts, and e.g. {} will have a different prototype depending on where it’s coming from. I forgot why, but apparently that’s the intended behaviour of V8.

[cjihrig]

I agree that we should try to get to the bottom of it, but if it isn't a straightforward fix, we should revert until we have a fix.

[addaleax]

See also e.g. https://github.com/nodejs/node/issues/7351

[lance]

And maybe this is related somehow? https://github.com/nodejs/node/issues/855

I don't really understand V8 Global proxy yet, so still trying to wrap my head around this.