Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

console.log(require.cache) fails when koa is present #837

Closed
paulpflug opened this issue Oct 20, 2016 · 7 comments
Closed

console.log(require.cache) fails when koa is present #837

paulpflug opened this issue Oct 20, 2016 · 7 comments

Comments

@paulpflug
Copy link

paulpflug commented Oct 20, 2016

koa = require('koa')
console.log(require.cache)

will fail because of the custom inspect implementation in context.js, which uses the custom toJSON, which relies on a properly set this, which is not the case.

not very important. But noteworthy - I guess
At least is costed me some time to figure it out.

@skbolton
Copy link

skbolton commented Jan 3, 2017

I've been chasing down multiple angles at this and I can't seem to get my finger on whats going on.
console.dir(require.cache) works fine.require.cache even works fine as a variable for whatever reason console.log clashes with inspect() inside of context.js.

@skbolton
Copy link

skbolton commented Jan 3, 2017

Found the culprit
In nodes documentation of util.js package starting at line 342

// Provide a hook for user-specified inspect functions.
  // Check that value is an object with an inspect function on it
  if (ctx.customInspect && value) {
    const maybeCustomInspect = value[customInspectSymbol] || value.inspect;

    if (typeof maybeCustomInspect === 'function' &&
        // Filter out the util module, its inspect function is special
        maybeCustomInspect !== exports.inspect &&
        // Also filter out any prototype objects using the circular check.
        !(value.constructor && value.constructor.prototype === value)) {
      let ret = maybeCustomInspect.call(value, recurseTimes, ctx);

      // If the custom inspection method returned `this`, don't go into
      // infinite recursion.
      if (ret !== value) {
        if (typeof ret !== 'string') {
          ret = formatValue(ctx, ret, recurseTimes);
        }
        return ret;
      }
    }
  }

When require.cache is called with console.log there is a lot of formatting that is done and apparently if a module exports an inspect property they will use that for the formatting. So if this corner case is important than the exported value shoule either not be an object (which is why changing it to a class worked), or change the name of inspect to something different.

@juliangruber
Copy link
Contributor

this was discussed before, and while I can't remember the exact issue we decided this is a node problem and we want to keep our inspect property

@PlasmaPower
Copy link
Contributor

I don't think we discussed this exact question before, but we probably did discuss this being undefined with another function. https://github.com/koajs/koa/issues?utf8=%E2%9C%93&q=is%3Aissue%20inspect%20OR%20toJSON%20

Since this is more builtin to Node and doesn't involve rebinding functions manually I'm more inclined to support it.

@skbolton
Copy link

skbolton commented Jan 3, 2017

Well if #605 happens (changing from a shared object to a class) then this error will be gone as well.

@PlasmaPower
Copy link
Contributor

@skbolton you can use the #605 which automatically links the issue or pull request: #605 (also your link doesn't work)

@skbolton
Copy link

skbolton commented Jan 3, 2017

Learn something everyday, fixed.

LukeBousfield pushed a commit to LukeBousfield/koa that referenced this issue Jun 25, 2017
jonathanong pushed a commit that referenced this issue Jul 16, 2017
* Fix context.inspect when called on the prototype

Fixes #837

* Add tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants