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

util: remove custom inspection function #20722

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion doc/api/deprecations.md
Original file line number Diff line number Diff line change
Expand Up @@ -717,7 +717,7 @@ Type: Runtime
<a id="DEP0079"></a>
### DEP0079: Custom inspection function on Objects via .inspect()

Type: Runtime
Type: End-of-Life

Using a property named `inspect` on an object to specify a custom inspection
function for [`util.inspect()`][] is deprecated. Use [`util.inspect.custom`][]
Expand Down
21 changes: 11 additions & 10 deletions doc/api/util.md
Original file line number Diff line number Diff line change
Expand Up @@ -393,8 +393,9 @@ changes:
* `colors` {boolean} If `true`, the output will be styled with ANSI color
codes. Colors are customizable, see [Customizing `util.inspect` colors][].
**Default:** `false`.
* `customInspect` {boolean} If `false`, then custom `inspect(depth, opts)`
functions will not be called. **Default:** `true`.
* `customInspect` {boolean} If `false`, then
`[util.inspect.custom](depth, opts)` functions will not be called.
Copy link
Contributor

@vsemozhetbyt vsemozhetbyt May 18, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can many functions be called during the inspection? Does this mean nested objects with own custom functions?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right, it is the same function (if it does not replace itself during custom inspection ;-) ).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at this again: functions do seem to be correct as it is not about a specific custom inspect function but about custom inspect functions in general.

**Default:** `true`.
* `showProxy` {boolean} If `true`, then objects and functions that are
`Proxy` objects will be introspected to show their `target` and `handler`
objects. **Default:** `false`.
Expand All @@ -416,7 +417,6 @@ changes:
objects the same as arrays. Note that no text will be reduced below 16
characters, no matter the `breakLength` size. For more information, see the
example below. **Default:** `true`.

* Returns: {string} The representation of passed object

The `util.inspect()` method returns a string representation of `object` that is
Expand All @@ -426,6 +426,11 @@ passed that alter certain aspects of the formatted string.
`util.inspect()` will use the constructor's name and/or `@@toStringTag` to make
an identifiable tag for an inspected value.

Values may supply their own custom inspection function
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As the next code example does not contain any mention of this, maybe we should link inspection function here to #util_util_inspect_custom or #util_custom_inspection_functions_on_objects?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I was also wrapping my head around this. I moved it because when looking at the current documentation it is confusing as it seems to be about something else than it really is. So +1 on that.

(`[util.inspect.custom](depth, opts)`), when called these receive the current
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

function -> functions or these receive -> this receives?

`depth` in the recursive inspection, as well as the options object passed to
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

options -> `options`?

`util.inspect()`.

```js
class Foo {
get [Symbol.toStringTag]() {
Expand All @@ -450,10 +455,6 @@ const util = require('util');
console.log(util.inspect(util, { showHidden: true, depth: null }));
```

Values may supply their own custom `inspect(depth, opts)` functions, when
called these receive the current `depth` in the recursive inspection, as well as
the options object passed to `util.inspect()`.

The following example highlights the difference with the `compact` option:

```js
Expand Down Expand Up @@ -568,9 +569,9 @@ terminals.

<!-- type=misc -->

Objects may also define their own `[util.inspect.custom](depth, opts)`
(or the equivalent but deprecated `inspect(depth, opts)`) function that
`util.inspect()` will invoke and use the result of when inspecting the object:
Objects may also define their own `[util.inspect.custom](depth, opts)` function
that `util.inspect()` will invoke and use the result of when inspecting the
object:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There’s also the customInspect option description which references inspect() { … }, fwiw

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed.


```js
const util = require('util');
Expand Down
12 changes: 1 addition & 11 deletions lib/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -456,17 +456,7 @@ function formatValue(ctx, value, recurseTimes, ln) {
// Provide a hook for user-specified inspect functions.
// Check that value is an object with an inspect function on it
if (ctx.customInspect) {
let maybeCustom = value[customInspectSymbol];

if (!maybeCustom && value.inspect !== exports.inspect &&
typeof value.inspect === 'function') {
maybeCustom = deprecate(
value.inspect,
'Custom inspection function on Objects via .inspect() is deprecated',
'DEP0079'
);
}

const maybeCustom = value[customInspectSymbol];
if (typeof maybeCustom === 'function' &&
// Filter out the util module, its inspect function is special
maybeCustom !== exports.inspect &&
Expand Down
11 changes: 7 additions & 4 deletions test/parallel/test-console.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
'use strict';
const common = require('../common');
const assert = require('assert');
const util = require('util');

assert.ok(process.stdout.writable);
assert.ok(process.stderr.writable);
Expand All @@ -46,8 +47,8 @@ assert.throws(() => console.timeEnd(Symbol('test')),
TypeError);


// an Object with a custom .inspect() function
const custom_inspect = { foo: 'bar', inspect: () => 'inspect' };
// An Object with a custom inspect function.
const custom_inspect = { foo: 'bar', [util.inspect.custom]: () => 'inspect' };

const strings = [];
const errStrings = [];
Expand Down Expand Up @@ -192,9 +193,11 @@ for (const expected of expectedStrings) {
}

assert.strictEqual(strings.shift(),
"{ foo: 'bar', inspect: [Function: inspect] }\n");
"{ foo: 'bar',\n [Symbol(util.inspect.custom)]: " +
'[Function: [util.inspect.custom]] }\n');
assert.strictEqual(strings.shift(),
"{ foo: 'bar', inspect: [Function: inspect] }\n");
"{ foo: 'bar',\n [Symbol(util.inspect.custom)]: " +
'[Function: [util.inspect.custom]] }\n');
assert.ok(strings.shift().includes('foo: [Object]'));
assert.strictEqual(strings.shift().includes('baz'), false);
assert.strictEqual(strings.shift(), 'inspect inspect\n');
Expand Down
19 changes: 0 additions & 19 deletions test/parallel/test-util-inspect-deprecated.js

This file was deleted.

91 changes: 21 additions & 70 deletions test/parallel/test-util-inspect.js
Original file line number Diff line number Diff line change
Expand Up @@ -532,10 +532,10 @@ assert.strictEqual(util.inspect(-5e-324), '-5e-324');
);
}

// GH-1941
// https://github.com/nodejs/node-v0.x-archive/issues/1941
assert.strictEqual(util.inspect(Object.create(Date.prototype)), 'Date {}');

// GH-1944
// https://github.com/nodejs/node-v0.x-archive/issues/1944
{
const d = new Date();
d.toUTCString = null;
Expand All @@ -550,20 +550,20 @@ assert.strictEqual(util.inspect(Object.create(Date.prototype)), 'Date {}');
}

// Should not throw.
const r = /regexp/;
r.toString = null;
util.inspect(r);

// Bug with user-supplied inspect function returns non-string.
util.inspect([{ inspect: () => 123 }]);
{
const r = /regexp/;
r.toString = null;
util.inspect(r);
}

// GH-2225
// See https://github.com/nodejs/node-v0.x-archive/issues/2225
{
const x = { inspect: util.inspect };
assert.strictEqual(util.inspect(x).includes('inspect'), true);
const x = { [util.inspect.custom]: util.inspect };
assert(util.inspect(x).includes(
'[Symbol(util.inspect.custom)]: \n { [Function: inspect]'));
}

// util.inspect should display the escaped value of a key.
// `util.inspect` should display the escaped value of a key.
{
const w = {
'\\': 1,
Expand Down Expand Up @@ -661,8 +661,8 @@ util.inspect({ hasOwnProperty: null });
}

{
// "customInspect" option can enable/disable calling inspect() on objects.
const subject = { inspect: () => 123 };
// "customInspect" option can enable/disable calling [util.inspect.custom]().
const subject = { [util.inspect.custom]: () => 123 };

assert.strictEqual(
util.inspect(subject, { customInspect: true }).includes('123'),
Expand All @@ -681,31 +681,6 @@ util.inspect({ hasOwnProperty: null });
true
);

// Custom inspect() functions should be able to return other Objects.
subject.inspect = () => ({ foo: 'bar' });

assert.strictEqual(util.inspect(subject), '{ foo: \'bar\' }');

subject.inspect = (depth, opts) => {
assert.strictEqual(opts.customInspectOptions, true);
};

util.inspect(subject, { customInspectOptions: true });
}

{
// "customInspect" option can enable/disable calling [util.inspect.custom]().
const subject = { [util.inspect.custom]: () => 123 };

assert.strictEqual(
util.inspect(subject, { customInspect: true }).includes('123'),
true
);
assert.strictEqual(
util.inspect(subject, { customInspect: false }).includes('123'),
false
);

// A custom [util.inspect.custom]() should be able to return other Objects.
subject[util.inspect.custom] = () => ({ foo: 'bar' });

Expand All @@ -718,43 +693,16 @@ util.inspect({ hasOwnProperty: null });
util.inspect(subject, { customInspectOptions: true });
}

{
// [util.inspect.custom] takes precedence over inspect.
const subject = {
[util.inspect.custom]() { return 123; },
inspect() { return 456; }
};

assert.strictEqual(
util.inspect(subject, { customInspect: true }).includes('123'),
true
);
assert.strictEqual(
util.inspect(subject, { customInspect: false }).includes('123'),
false
);
assert.strictEqual(
util.inspect(subject, { customInspect: true }).includes('456'),
false
);
assert.strictEqual(
util.inspect(subject, { customInspect: false }).includes('456'),
false
);
}

{
// Returning `this` from a custom inspection function works.
assert.strictEqual(util.inspect({ a: 123, inspect() { return this; } }),
'{ a: 123, inspect: [Function: inspect] }');

const subject = { a: 123, [util.inspect.custom]() { return this; } };
const UIC = 'util.inspect.custom';
assert.strictEqual(util.inspect(subject),
`{ a: 123,\n [Symbol(${UIC})]: [Function: [${UIC}]] }`);
}

// util.inspect with "colors" option should produce as many lines as without it.
// Using `util.inspect` with "colors" option should produce as many lines as
// without it.
{
function testLines(input) {
const countLines = (str) => (str.match(/\n/g) || []).length;
Expand Down Expand Up @@ -1162,8 +1110,11 @@ util.inspect(process);

// Setting custom inspect property to a non-function should do nothing.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/property/symbol/ ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But it is still a property. It is just accessed with a symbol.

{
const obj = { inspect: 'fhqwhgads' };
assert.strictEqual(util.inspect(obj), "{ inspect: 'fhqwhgads' }");
const obj = { [util.inspect.custom]: 'fhqwhgads' };
assert.strictEqual(
util.inspect(obj),
"{ [Symbol(util.inspect.custom)]: 'fhqwhgads' }"
);
}

{
Expand Down