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

repl breaks something about constructor/type info #1834

Closed
eventualbuddha opened this issue Oct 6, 2011 · 4 comments
Closed

repl breaks something about constructor/type info #1834

eventualbuddha opened this issue Oct 6, 2011 · 4 comments
Labels

Comments

@eventualbuddha
Copy link

As illustrated by this gist, Node.js v0.4.8 (and apparently v0.5.8, tested independently in #node.js) exhibits a strange behavior in the repl where loading a module function that passes a constructor (e.g. String or Number) in to be checked against an object's constructor doesn't behave as expected. Here is the function to be run:

// 00-type.js
exports.is = function(object, constructor) {
  return object instanceof constructor || (object != null ? object.constructor : void 0) === constructor;
};
$ node
> type = require('./00-type')
{ is: [Function] }
> type.is('foo', String)
false

However, if you then re-create this simple is function and use the new version, it works fine:

> eval('global.test = '+type.is.toString())
[Function]
> test('foo', String)
true

This is different from how this works when running node with a file argument and not in a repl:

// 01-test.js
type = require('./00-type')

console.log('type.is =>', type.is('foo', String))
eval('global.test = '+type.is.toString())
console.log('test    =>', test('foo', String))
$ node 01-test.js
type.is => true
test    => true

In that case, both return the expected true. Something is clearly wrong here, but I'm not sure what.

@bnoordhuis
Copy link
Member

Not a bug. Modules and the REPL run in separate VM contexts. Each context has its own set of "root" objects (Object, String, Date, etc.). Look at e.g. isRegExp() in lib/util.js to see how we handle that:

function isRegExp(re) {
  return re instanceof RegExp ||
      (typeof re === 'object' && objectToString(re) === '[object RegExp]');
}

@eventualbuddha
Copy link
Author

I had a feeling it might be something like that. Nevertheless, it's pretty strange behavior that can make testing code in the REPL somewhat less useful. Yes, real tests should be written in Jasmine or something but it's nice to be able to duck into the REPL sometimes to "see how it works", and sharp corners like this make that kinda dangerous. You don't see this kind of behavior, afaik, in Ruby, Python, or Scala.

@eventualbuddha
Copy link
Author

Anyway, thanks for checking it out. Maybe this implies that there should be bigger warnings about this kind of behavior.

@bnoordhuis
Copy link
Member

It's a common source of confusion. #1484 intends to address it but it's one of those TODOs no one ever seems to get around to.

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.
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

2 participants