Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

In REPL, required modules have different Object than command-line #1482

Closed
sethml opened this issue Aug 9, 2011 · 10 comments
Closed

In REPL, required modules have different Object than command-line #1482

sethml opened this issue Aug 9, 2011 · 10 comments
Labels

Comments

@sethml
Copy link

sethml commented Aug 9, 2011

In the node REPL, it appears that Object is different than in required modules. For example, given a simple module test.js:

exports.Object = Object;
exports.isObject = function(o) {
  return o instanceof Object;
};
exports.anObject = {a:1};

I get the following behavior in the REPL:

> var test = require('./test.js');
> test.isObject({a:1})
false
> test.isObject(test.anObject)
true
> test.Object == Object
false
> test.anObject instanceof Object
false

I noticed this issue when attempting to use node-mongodb-native from the command-line:

> var mongodb = require('mongodb');
> mongodb.pure().BSON.serialize({foo: 1});
Error: Not a valid object
    at Function.serialize (/Users/seth/development/service/node_modules/mongodb/lib/mongodb/bson/bson.js:1249:11)
@bnoordhuis
Copy link
Member

Not a bug, it's how JS objects work. Consider:

> {a:1} == {a:1}
false
> {a:1} === {a:1}
false

Object equality is identity based, not value based.

@neonstalwart
Copy link

@bnoordhuis i think you may have misunderstood the problem

@bnoordhuis
Copy link
Member

The other part of the equation is that modules live in a context separate from the REPL. It's the same behaviour you get with NODE_MODULE_CONTEXTS=1.

@neonstalwart
Copy link

i see, so that is why Object !== test.Object.

@bnoordhuis
Copy link
Member

Yes. It's slightly confusing but there was a good reason to do it like this - I just can't quite remember what it was.

@isaacs - do you remember? Clobbering of the global context? Security issues in a multi-user setup?

@sethml
Copy link
Author

sethml commented Aug 9, 2011

It looks to me like the reason was to allow the .clear command to work: #771

Personally, I think having the REPL behave this differently from non-REPL code is a really bad idea - it leads newbies to waste significant time trying to figure out what the heck their code is doing. I've personally wasted more time than I'd like on it. Here's another person wasting time on this issue: https://github.com/christkv/node-mongodb-native/issues/314

I'd gladly give up the .clear command to get sane behavior in the REPL.

@isaacs
Copy link

isaacs commented Aug 9, 2011

I'm coming around to this as well. If you really want to clear, just restart. It does allow for very nice restricted access from the repl to the rest of a program, if you listen on a socket and use it to check in with a running server or something. But even still, it's not much security, since at that point someone's logged onto your machine and socat'ed a random socket, so they can probably do anything anyway.

I'll open a new issue.

@isaacs
Copy link

isaacs commented Aug 9, 2011

#1484

@sethml
Copy link
Author

sethml commented Aug 9, 2011

One more example of subtle breakage caused by this: lots of modules use 'if (func instanceof Function)' to handle variable argument lists. This will break subtly when called from the REPL, since functions created in the REPL will not be instances of the module's Function.

isaacs added a commit to isaacs/node-v0.x-archive that referenced this issue Oct 18, 2011
Fix nodejs#1484
Fix nodejs#1834
Fix nodejs#1482
Fix nodejs#771

It's been a while now, and we've seen how this separate context thing
works.  It constantly confuses people, and no one actually uses '.clear'
anyway, so the benefit of that feature does not justify the constant
WTFery.

This makes repl.context actually be a getter that returns the global
object, and prints a deprecation warning.  The '.clear' command is gone,
and will report that it's an invalid repl keyword.  Tests updated to
allow the require, module, and exports globals, which are still
available in the repl just like they were before, by making them global.
bnoordhuis pushed a commit to bnoordhuis/node that referenced this issue Oct 19, 2011
Fix nodejs#1484
Fix nodejs#1834
Fix nodejs#1482
Fix nodejs#771

It's been a while now, and we've seen how this separate context thing
works.  It constantly confuses people, and no one actually uses '.clear'
anyway, so the benefit of that feature does not justify the constant
WTFery.

This makes repl.context actually be a getter that returns the global
object, and prints a deprecation warning.  The '.clear' command is gone,
and will report that it's an invalid repl keyword.  Tests updated to
allow the require, module, and exports globals, which are still
available in the repl just like they were before, by making them global.
@netpedro-com
Copy link

netpedro-com commented Jan 11, 2017

I just wasted 2 hours stuck into this issue:

> var my_module = require('./module_test.js');
> my_module.cool_func({a:1})
false
// cool_func eval "{a:1} instanceof Object" to false

That's terrible. I can't live without REPL. For someone who want to know more, see https://github.com/nwjs/nw.js/wiki/Differences-of-JavaScript-contexts#working-around-differences-of-contexts

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

5 participants