Skip to content

util: remove custom inspection function #20722

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

Closed
BridgeAR wants to merge 5 commits into from
Closed

util: remove custom inspection function #20722

Show file tree
Hide file tree
Changes from 1 commit
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
Next Next commit
util: remove custom inspection function 
This removes the deprecated custom inspection function and fixes
all tests accordingly.

Refs: #15549
  • Loading branch information
@BridgeAR
BridgeAR committed May 14, 2018
commit 50c95a97348f2a22fecc829825005bbaddf02e64
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
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.
Copy link
Copy Markdown
Contributor

@mscdex mscdex May 14, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Perhaps this should be slightly re-worded to instead say "custom inspect function" or "custom inspect symbol" or similar now, to avoid confusion.

Copy link
Copy Markdown
Member Author

@BridgeAR BridgeAR May 15, 2018

Choose a reason for hiding this comment

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

I would say it is still a custom inspect function but it might be best to just write .inspect() without dot and brackets.

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
Copy Markdown
Contributor

@mscdex mscdex May 14, 2018

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
Copy Markdown
Member Author

@BridgeAR BridgeAR May 15, 2018

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