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.

Sign up for GitHub

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

Jump to bottom

util: remove custom inspection function #20722

Closed
wants to merge 5 commits into from
Closed

util: remove custom inspection function #20722

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
50c95a9
util: remove custom inspection function
BridgeAR May 14, 2018
79d7790
fixup: address comment
BridgeAR May 15, 2018
2906612
fixup: remove docs
BridgeAR May 17, 2018
5e9d5d5
fixup: address comment
BridgeAR May 17, 2018
ca82165
fixup: remove unnecessary docs part
BridgeAR May 19, 2018
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
View file
Open in desktop
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
16 changes: 6 additions & 10 deletions doc/api/util.md
View file
Open in desktop
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 •
edited
Loading

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 Down Expand Up @@ -450,10 +450,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 +564,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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop

This file was deleted.

91 changes: 21 additions & 70 deletions test/parallel/test-util-inspect.js
View file
Open in desktop
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