This repository has been archived by the owner. It is now read-only.

Weird 'require' behaviour #410

Closed
ArtS opened this Issue Nov 10, 2010 · 14 comments

Comments

Projects
None yet
7 participants

ArtS commented Nov 10, 2010

There are three files:

a.js:

 var b = require('./b');
 console.log(b);

b.js:

 exports.b = require('./c');

c.js:

 exports.inspect = function(obj) {};

running

node ./a.js

produces error:

node.js:50
throw e; // process.nextTick error, or 'error' event on first tick
^
TypeError: Cannot call method 'indexOf' of undefined
at util.js:187:19
at Array.map (native)
at format (util.js:162:23)
at Object.inspect (util.js:244:10)
at Object.format (node.js:501:25)
at Object. (node.js:527:31)
at Object. (/home/ci-user/dev/nimblegecko-2/a.js:3:9)
at Module._compile (node.js:348:23)
at Object..js (node.js:356:12)
at Module.load (node.js:279:25)

As soon as I rename 'inspect' into something else, all works fine.

Any ideas why?

@ghost

ghost commented Feb 17, 2011

broken on 0.4.0

koichik commented Jul 17, 2011

The inspect is a special name of hook for user-specified formatting functions. It must return a string.

Is it too common name?

ArtS commented Jul 17, 2011

I don't think it matters whether it is too common or not - I'd say all built-in things like that should be prefixed to avoid clashes. Something like two underscores in the beginning of the member's name; that would also help when reading the code.

bnoordhuis added a commit to bnoordhuis/node that referenced this issue Sep 6, 2011

Member

bnoordhuis commented Sep 6, 2011

Confirmed, still happens. Can someone review 4cf91ed?

@bnoordhuis +1, would be useful for the REPL: a = require('./a') (will try in inspect() it since it's a global, not a var).

koichik commented Sep 7, 2011

@bnoordhuis - The patch LGTM.
But @ArtS wants the change of the name inspect(), i.e. __inspect().
His problem is a collision of the names.
I think that the patch does not fix his problem. If his inspect() has side effects?
On the other hand, we need to keep compatibility... :-<

Well when do you really need to util.inspect() your module's exports?

-1 on changing the name.

koichik commented Sep 7, 2011

exports is unrelated.
example:

var util = require('util');
var a = { inspect: function() { throw new Error() } };
util.inspect(a);

Please imagine that a.inspect() (It may be a third party object) does not have intention of being called by util.inspect().
result:

$ ./node /tmp/410.js

node.js:203
        throw e; // process.nextTick error, or 'error' event on first tick
              ^
Error
    at Object.inspect (/tmp/410.js:2:39)
    at format (util.js:147:20)
    at Object.inspect (util.js:316:10)
    at Object.<anonymous> (/tmp/410.js:3:6)
    at Module._compile (module.js:416:26)
    at Object..js (module.js:434:10)
    at Module.load (module.js:335:31)
    at Function._load (module.js:294:12)
    at Array.<anonymous> (module.js:454:10)
    at EventEmitter._tickCallback (node.js:195:26)

Good point.

On Tue, Sep 6, 2011 at 11:41 PM, Koichi Kobayashi <
reply@reply.github.com>wrote:

exports is unrelated.
example:

var util = require('util');
var a = { inspect: function() { throw new Error() } };
util.inspect(a);

Please imagine that a.inspect() (It may be a third party object) does not
have intention of being called by util.inspect().
result:

$ ./node /tmp/410.js

node.js:203
       throw e; // process.nextTick error, or 'error' event on first tick
             ^
Error
   at Object.inspect (/tmp/410.js:2:39)
   at format (util.js:147:20)
   at Object.inspect (util.js:316:10)
   at Object.<anonymous> (/tmp/410.js:3:6)
   at Module._compile (module.js:416:26)
   at Object..js (module.js:434:10)
   at Module.load (module.js:335:31)
   at Function._load (module.js:294:12)
   at Array.<anonymous> (module.js:454:10)
   at EventEmitter._tickCallback (node.js:195:26)

Reply to this email directly or view it on GitHub:
joyent#410 (comment)

Member

bnoordhuis commented Sep 7, 2011

But @ArtS wants the change of the name inspect(), i.e. __inspect().

That's step 2: deprecate inspect() in 0.6 and remove it in 0.7. I prefer __inspect__(), that's probably the pythonista in me. Objections?

-1 Leave inspect as is, returning a string. Its very handy. If there's confusion its b/c the docs don't specify this behavior. I would consider the docs as the bug, not the name.

tj commented Sep 7, 2011

on a side note console.dir should ignore inspect (too small for an issue?)

isaacs commented Feb 28, 2012

This is not a big enough issue to justify changing util.inspect. (Or, even worse! renaming it!)

Closing.

@isaacs isaacs closed this Feb 28, 2012

@kwonoj kwonoj referenced this issue in ReactiveX/rxjs Feb 26, 2016

Closed

Problem with `inspect` and Node repl #1387

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.