fix: passing options to custom inspect

[martinheidegger]

Node internal objects (like URL) expect options to be passed in to inspect call. Without these options inspecting some native objects will result in unexpected errors. I noticed this problem when inspecting a URL object:

node:internal/url:689
    if (opts.showHidden) {
             ^

TypeError: Cannot read properties of undefined (reading 'showHidden')
    at [nodejs.util.inspect.custom] (node:internal/url:689:14)
    at inspect_ (/inspect-js/object-inspect/index.js:197:38)
    at Test.<anonymous> (/inspect-js/object-inspect/test/inspect.js:110:9)
    at Test.bound [as _cb] (/inspect-js/object-inspect/node_modules/tape/lib/test.js:99:32)
    at Test.run (/inspect-js/object-inspect/node_modules/tape/lib/test.js:117:31)
    at Test.bound [as run] (/inspect-js/object-inspect/node_modules/tape/lib/test.js:99:32)
    at Immediate.next (/inspect-js/object-inspect/node_modules/tape/lib/results.js:88:19)
    at process.processImmediate (node:internal/timers:471:21)

This PR fixes this by also passing inspect options to inspect calls.

[codecov]

Codecov Report

Merging #40 (ab0b3e0) into main (1db93bc) will increase coverage by 0.01%.
The diff coverage is 100.00%.

❗ Current head ab0b3e0 differs from pull request most recent head e243bf2. Consider uploading reports for the commit e243bf2 to get more accurate results

@@            Coverage Diff             @@
##             main      #40      +/-   ##
==========================================
+ Coverage   96.18%   96.19%   +0.01%     
==========================================
  Files           2        2              
  Lines         341      342       +1     
  Branches      147      147              
==========================================
+ Hits          328      329       +1     
  Misses         13       13              

Impacted Files	Coverage Δ
index.js	96.18% <100.00%> (+0.01%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1db93bc...e243bf2. Read the comment docs.

[ljharb]

Thanks, good catch.

[ljharb]

hardcoding these defaults seems problematic - i do get that node probably passes them explicitly. is there a way we could look up the values so that if node changes them, ours change too?

[martinheidegger]

🤔 They could be retrieved by doing a dummy inspect call.

[ljharb]

util.inspect.defaultOptions might be helpful here

[ljharb]

passing this is a very bad idea.

Suggested change

seen: seen,

[martinheidegger]

I am passing this because it is also passed by the nodejs inspect

[ljharb]

i'm pretty sure node's cache is different than ours, but either way, i'd rather not pass it here even if that causes breakage for a custom inspect function.

[martinheidegger]

Would it be better to pass an empty array? There is likely some code somewhere expect this to be an array?

[ljharb]

when i do var o = { [util.inspect.custom]() { console.log(arguments); } }; util.inspect(o); in the repl, this is what i see as the first argument:

{
    stylize: [Function: stylizeNoColor],
    showHidden: false,
    depth: 2,
    colors: false,
    customInspect: true,
    showProxy: false,
    maxArrayLength: 100,
    maxStringLength: 10000,
    breakLength: 80,
    compact: 3,
    sorted: false,
    getters: false,
    numericSeparator: false
  }

which doesn't contain a "seen" property.

I also note that there's a third argument, which is the original inspect function. should we pass that?

[martinheidegger]

The third argument is a fancy new feature which, yeah, probably is a good idea to have.
seen has been passed in older versions of node, I couldn't locate the commit where this changed.

[martinheidegger]

Since util.inspect is not really compatible between node.js versions, I thought it is a fools errand to try to make it match somehow 🤷 . In ab0b3e0 I tried to fix it by simply using node's inspect function for custom inspects. It is not a perfect solution as the inspect calls will receive a wrong "opts.depth" property, but I think its a solution that can be lived with. If you are okay with this, I am happy to squash the commits.

[ljharb]

simple approach, i like it