From 48739466dfcb6abc6e78795892781eedce196543 Mon Sep 17 00:00:00 2001 From: Ivan Goncharov Date: Sun, 14 Feb 2021 19:54:59 +0200 Subject: [PATCH] Remove Node's custom inspect function (#2914) --- src/jsutils/__tests__/inspect-test.js | 45 ++++++++---------------- src/jsutils/defineInspect.js | 19 ---------- src/jsutils/inspect.js | 26 ++++---------- src/jsutils/nodejsCustomInspectSymbol.js | 7 ---- src/language/__tests__/lexer-test.js | 6 ---- src/language/__tests__/parser-test.js | 4 --- src/language/ast.js | 8 ----- src/type/definition.js | 25 ------------- src/type/directives.js | 4 --- 9 files changed, 21 insertions(+), 123 deletions(-) delete mode 100644 src/jsutils/defineInspect.js delete mode 100644 src/jsutils/nodejsCustomInspectSymbol.js diff --git a/src/jsutils/__tests__/inspect-test.js b/src/jsutils/__tests__/inspect-test.js index a3fedb9d15d..ba7ca9a86cb 100644 --- a/src/jsutils/__tests__/inspect-test.js +++ b/src/jsutils/__tests__/inspect-test.js @@ -3,7 +3,6 @@ import { describe, it } from 'mocha'; import inspect from '../inspect'; import invariant from '../invariant'; -import nodejsCustomInspectSymbol from '../nodejsCustomInspectSymbol'; describe('inspect', () => { it('undefined', () => { @@ -84,54 +83,40 @@ describe('inspect', () => { expect(inspect(map)).to.equal('{ a: true, b: null }'); }); - it('custom inspect', () => { + it('use toJSON if provided', () => { const object = { - inspect() { - return ''; + toJSON() { + return ''; }, }; - expect(inspect(object)).to.equal(''); + expect(inspect(object)).to.equal(''); }); - it('custom inspect that return `this` should work', () => { + it('handles toJSON that return `this` should work', () => { const object = { - inspect() { + toJSON() { return this; }, }; - expect(inspect(object)).to.equal('{ inspect: [function inspect] }'); + expect(inspect(object)).to.equal('{ toJSON: [function toJSON] }'); }); - it('custom symbol inspect is take precedence', () => { + it('handles toJSON returning object values', () => { const object = { - // istanbul ignore next (Never called and use just as a placeholder) - inspect() { - invariant(false); - }, - [String(nodejsCustomInspectSymbol)]() { - return ''; - }, - }; - - expect(inspect(object)).to.equal(''); - }); - - it('custom inspect returning object values', () => { - const object = { - inspect() { - return { custom: 'inspect' }; + toJSON() { + return { json: 'value' }; }, }; - expect(inspect(object)).to.equal('{ custom: "inspect" }'); + expect(inspect(object)).to.equal('{ json: "value" }'); }); - it('custom inspect function that uses this', () => { + it('handles toJSON function that uses this', () => { const object = { str: 'Hello World!', - inspect() { + toJSON() { return this.str; }, }; @@ -160,11 +145,11 @@ describe('inspect', () => { expect(inspect(mixed)).to.equal('{ array: [[Circular]] }'); const customA = { - inspect: () => customB, + toJSON: () => customB, }; const customB = { - inspect: () => customA, + toJSON: () => customA, }; expect(inspect(customA)).to.equal('[Circular]'); diff --git a/src/jsutils/defineInspect.js b/src/jsutils/defineInspect.js deleted file mode 100644 index b0773a9a79f..00000000000 --- a/src/jsutils/defineInspect.js +++ /dev/null @@ -1,19 +0,0 @@ -import invariant from './invariant'; -import nodejsCustomInspectSymbol from './nodejsCustomInspectSymbol'; - -/** - * The `defineInspect()` function defines `inspect()` prototype method as alias of `toJSON` - */ -export default function defineInspect( - classObject: Class | ((...args: Array) => mixed), -): void { - const fn = classObject.prototype.toJSON; - invariant(typeof fn === 'function'); - - classObject.prototype.inspect = fn; - - // istanbul ignore else (See: 'https://github.com/graphql/graphql-js/issues/2317') - if (nodejsCustomInspectSymbol) { - classObject.prototype[nodejsCustomInspectSymbol] = fn; - } -} diff --git a/src/jsutils/inspect.js b/src/jsutils/inspect.js index b33f70c8251..f841f7da8d3 100644 --- a/src/jsutils/inspect.js +++ b/src/jsutils/inspect.js @@ -1,5 +1,4 @@ /* eslint-disable flowtype/no-weak-types */ -import nodejsCustomInspectSymbol from './nodejsCustomInspectSymbol'; const MAX_ARRAY_LENGTH = 10; const MAX_RECURSIVE_DEPTH = 2; @@ -36,16 +35,15 @@ function formatObjectValue( } const seenValues = [...previouslySeenValues, value]; - const customInspectFn = getCustomFn(value); - if (customInspectFn !== undefined) { - const customValue = customInspectFn.call(value); + if (typeof value.toJSON === 'function') { + const jsonValue = value.toJSON(value); // check for infinite recursion - if (customValue !== value) { - return typeof customValue === 'string' - ? customValue - : formatValue(customValue, seenValues); + if (jsonValue !== value) { + return typeof jsonValue === 'string' + ? jsonValue + : formatValue(jsonValue, seenValues); } } else if (Array.isArray(value)) { return formatArray(value, seenValues); @@ -98,18 +96,6 @@ function formatArray(array: Array, seenValues: Array): string { return '[' + items.join(', ') + ']'; } -function getCustomFn(object: Object) { - const customInspectFn = object[String(nodejsCustomInspectSymbol)]; - - if (typeof customInspectFn === 'function') { - return customInspectFn; - } - - if (typeof object.inspect === 'function') { - return object.inspect; - } -} - function getObjectTag(object: Object): string { const tag = Object.prototype.toString .call(object) diff --git a/src/jsutils/nodejsCustomInspectSymbol.js b/src/jsutils/nodejsCustomInspectSymbol.js deleted file mode 100644 index 9a646e60cf9..00000000000 --- a/src/jsutils/nodejsCustomInspectSymbol.js +++ /dev/null @@ -1,7 +0,0 @@ -// istanbul ignore next (See: 'https://github.com/graphql/graphql-js/issues/2317') -const nodejsCustomInspectSymbol = - typeof Symbol === 'function' && typeof Symbol.for === 'function' - ? Symbol.for('nodejs.util.inspect.custom') - : undefined; - -export default nodejsCustomInspectSymbol; diff --git a/src/language/__tests__/lexer-test.js b/src/language/__tests__/lexer-test.js index 2aba2d8b780..7f6317b5cac 100644 --- a/src/language/__tests__/lexer-test.js +++ b/src/language/__tests__/lexer-test.js @@ -1,6 +1,3 @@ -// eslint-disable-next-line import/no-nodejs-modules -import { inspect as nodeInspect } from 'util'; - import { expect } from 'chai'; import { describe, it } from 'mocha'; @@ -121,9 +118,6 @@ describe('Lexer', () => { expect(JSON.stringify(token)).to.equal( '{"kind":"Name","value":"foo","line":1,"column":1}', ); - expect(nodeInspect(token)).to.equal( - "{ kind: 'Name', value: 'foo', line: 1, column: 1 }", - ); expect(inspect(token)).to.equal( '{ kind: "Name", value: "foo", line: 1, column: 1 }', ); diff --git a/src/language/__tests__/parser-test.js b/src/language/__tests__/parser-test.js index 9b9c91f387b..3a0924680ac 100644 --- a/src/language/__tests__/parser-test.js +++ b/src/language/__tests__/parser-test.js @@ -1,6 +1,3 @@ -// eslint-disable-next-line import/no-nodejs-modules -import { inspect as nodeInspect } from 'util'; - import { expect } from 'chai'; import { describe, it } from 'mocha'; @@ -379,7 +376,6 @@ describe('Parser', () => { const result = parse('{ id }'); expect(JSON.stringify(result.loc)).to.equal('{"start":0,"end":6}'); - expect(nodeInspect(result.loc)).to.equal('{ start: 0, end: 6 }'); expect(inspect(result.loc)).to.equal('{ start: 0, end: 6 }'); }); diff --git a/src/language/ast.js b/src/language/ast.js index 0b69454977c..75e50f542df 100644 --- a/src/language/ast.js +++ b/src/language/ast.js @@ -1,5 +1,3 @@ -import defineInspect from '../jsutils/defineInspect'; - import type { Source } from './source'; import type { TokenKindEnum } from './tokenKind'; @@ -46,9 +44,6 @@ export class Location { } } -// Print a simplified form when appearing in `inspect` and `util.inspect`. -defineInspect(Location); - /** * Represents a range of characters represented by a lexical token * within a Source. @@ -126,9 +121,6 @@ export class Token { } } -// Print a simplified form when appearing in `inspect` and `util.inspect`. -defineInspect(Token); - /** * @internal */ diff --git a/src/type/definition.js b/src/type/definition.js index f68a2efebbe..4ab794fc363 100644 --- a/src/type/definition.js +++ b/src/type/definition.js @@ -17,7 +17,6 @@ import instanceOf from '../jsutils/instanceOf'; import didYouMean from '../jsutils/didYouMean'; import isObjectLike from '../jsutils/isObjectLike'; import identityFunc from '../jsutils/identityFunc'; -import defineInspect from '../jsutils/defineInspect'; import suggestionList from '../jsutils/suggestionList'; import { GraphQLError } from '../error/GraphQLError'; @@ -370,9 +369,6 @@ export class GraphQLList<+T: GraphQLType> { } } -// Print a simplified form when appearing in `inspect` and `util.inspect`. -defineInspect(GraphQLList); - /** * Non-Null Type Wrapper * @@ -419,9 +415,6 @@ export class GraphQLNonNull<+T: GraphQLNullableType> { } } -// Print a simplified form when appearing in `inspect` and `util.inspect`. -defineInspect(GraphQLNonNull); - /** * These types wrap and modify other types */ @@ -631,9 +624,6 @@ export class GraphQLScalarType { } } -// Print a simplified form when appearing in `inspect` and `util.inspect`. -defineInspect(GraphQLScalarType); - export type GraphQLScalarSerializer = ( outputValue: mixed, ) => ?TExternal; @@ -778,9 +768,6 @@ export class GraphQLObjectType { } } -// Print a simplified form when appearing in `inspect` and `util.inspect`. -defineInspect(GraphQLObjectType); - function defineInterfaces( config: $ReadOnly< | GraphQLObjectTypeConfig @@ -1098,9 +1085,6 @@ export class GraphQLInterfaceType { } } -// Print a simplified form when appearing in `inspect` and `util.inspect`. -defineInspect(GraphQLInterfaceType); - export type GraphQLInterfaceTypeConfig = {| name: string, description?: ?string, @@ -1208,9 +1192,6 @@ export class GraphQLUnionType { } } -// Print a simplified form when appearing in `inspect` and `util.inspect`. -defineInspect(GraphQLUnionType); - function defineTypes( config: $ReadOnly>, ): Array { @@ -1389,9 +1370,6 @@ export class GraphQLEnumType /* */ { } } -// Print a simplified form when appearing in `inspect` and `util.inspect`. -defineInspect(GraphQLEnumType); - function didYouMeanEnumValue( enumType: GraphQLEnumType, unknownValueStr: string, @@ -1541,9 +1519,6 @@ export class GraphQLInputObjectType { } } -// Print a simplified form when appearing in `inspect` and `util.inspect`. -defineInspect(GraphQLInputObjectType); - function defineInputFieldMap( config: $ReadOnly, ): GraphQLInputFieldMap { diff --git a/src/type/directives.js b/src/type/directives.js index cb26388cafa..c92cc10641d 100644 --- a/src/type/directives.js +++ b/src/type/directives.js @@ -6,7 +6,6 @@ import toObjMap from '../jsutils/toObjMap'; import devAssert from '../jsutils/devAssert'; import instanceOf from '../jsutils/instanceOf'; import isObjectLike from '../jsutils/isObjectLike'; -import defineInspect from '../jsutils/defineInspect'; import type { DirectiveDefinitionNode } from '../language/ast'; import type { DirectiveLocationEnum } from '../language/directiveLocation'; @@ -109,9 +108,6 @@ export class GraphQLDirective { } } -// Print a simplified form when appearing in `inspect` and `util.inspect`. -defineInspect(GraphQLDirective); - export type GraphQLDirectiveConfig = {| name: string, description?: ?string,