From 84689d1731f1c002b12b19f15ef2a10bbe274f3d Mon Sep 17 00:00:00 2001 From: Jovi De Croock Date: Thu, 28 Nov 2024 08:54:29 +0100 Subject: [PATCH 1/4] Improve performance of printer --- benchmark/benchmark.js | 2 +- benchmark/fixtures.js | 5 ++ benchmark/kitchen-sink.graphql | 69 +++++++++++++++++++++ benchmark/printer-benchmark.js | 15 +++++ src/language/printer.ts | 106 ++++++++++++++++++++------------- src/language/visitor.ts | 20 +++---- 6 files changed, 165 insertions(+), 52 deletions(-) create mode 100644 benchmark/kitchen-sink.graphql create mode 100644 benchmark/printer-benchmark.js diff --git a/benchmark/benchmark.js b/benchmark/benchmark.js index 9288d1f273..2948dd198e 100644 --- a/benchmark/benchmark.js +++ b/benchmark/benchmark.js @@ -243,7 +243,7 @@ function maxBy(array, fn) { // Prepare all revisions and run benchmarks matching a pattern against them. async function runBenchmarks(benchmarks, benchmarkProjects) { - for (const benchmark of benchmarks) { + for (const benchmark of benchmarks.filter(x => x.includes('visit-') || x.includes('printer-'))) { const results = []; for (let i = 0; i < benchmarkProjects.length; ++i) { const { revision, projectPath } = benchmarkProjects[i]; diff --git a/benchmark/fixtures.js b/benchmark/fixtures.js index d057a80526..d665e552d0 100644 --- a/benchmark/fixtures.js +++ b/benchmark/fixtures.js @@ -8,6 +8,11 @@ exports.bigSchemaSDL = fs.readFileSync( 'utf8', ); +exports.bigDocument = fs.readFileSync( + path.join(__dirname, 'kitchen-sink.graphql'), + 'utf8', +); + exports.bigSchemaIntrospectionResult = JSON.parse( fs.readFileSync(path.join(__dirname, 'github-schema.json'), 'utf8'), ); diff --git a/benchmark/kitchen-sink.graphql b/benchmark/kitchen-sink.graphql new file mode 100644 index 0000000000..c1bb2d8038 --- /dev/null +++ b/benchmark/kitchen-sink.graphql @@ -0,0 +1,69 @@ +# Copyright (c) 2015-present, Facebook, Inc. +# +# This source code is licensed under the MIT license found in the +# LICENSE file in the root directory of this source tree. + +query queryName($foo: ComplexType, $site: Site = MOBILE) @onQuery { + whoever123is: node(id: [123, 456]) { + id + ... on User @onInlineFragment { + field2 { + id + alias: field1(first: 10, after: $foo) @include(if: $foo) { + id + ...frag @onFragmentSpread + } + } + } + ... @skip(unless: $foo) { + id + } + ... { + id + } + } +} + +mutation likeStory @onMutation { + like(story: 123) @onField { + story { + id @onField + } + } +} + +subscription StoryLikeSubscription($input: StoryLikeSubscribeInput) +@onSubscription { + storyLikeSubscribe(input: $input) { + story { + likers { + count + } + likeSentence { + text + } + } + } +} + +fragment frag on Friend @onFragmentDefinition { + foo( + size: $site + bar: 12 + obj: { + key: "value" + block: """ + block string uses \""" + """ + } + ) +} + +query teeny { + unnamed(truthy: true, falsey: false, nullish: null) + query +} + +query tiny { + __typename +} diff --git a/benchmark/printer-benchmark.js b/benchmark/printer-benchmark.js new file mode 100644 index 0000000000..f578700e79 --- /dev/null +++ b/benchmark/printer-benchmark.js @@ -0,0 +1,15 @@ +'use strict'; + +const { parse } = require('graphql/language/parser.js'); +const { print } = require('graphql/language/printer.js'); +const { bigDocument } = require('./fixtures') + +const document = parse(bigDocument) + +module.exports = { + name: 'Print ktichen-sink query', + count: 1000, + measure() { + print(document); + }, +}; diff --git a/src/language/printer.ts b/src/language/printer.ts index e95c118d8b..f29680ed34 100644 --- a/src/language/printer.ts +++ b/src/language/printer.ts @@ -23,17 +23,18 @@ const printDocASTReducer: ASTReducer = { // Document Document: { - leave: (node) => join(node.definitions, '\n\n'), + leave: (node) => truthyJoin(node.definitions, '\n\n'), }, OperationDefinition: { leave(node) { - const varDefs = wrap('(', join(node.variableDefinitions, ', '), ')'); + const varDefs = wrap('(', truthyJoin(node.variableDefinitions, ', '), ')'); const prefix = join( [ node.operation, + // TODO: optimize join([node.name, varDefs]), - join(node.directives, ' '), + truthyJoin(node.directives, ' '), ], ' ', ); @@ -50,20 +51,20 @@ const printDocASTReducer: ASTReducer = { ': ' + type + wrap(' = ', defaultValue) + - wrap(' ', join(directives, ' ')), + wrap(' ', truthyJoin(directives, ' ')), }, SelectionSet: { leave: ({ selections }) => block(selections) }, Field: { leave({ alias, name, arguments: args, directives, selectionSet }) { const prefix = wrap('', alias, ': ') + name; - let argsLine = prefix + wrap('(', join(args, ', '), ')'); + let argsLine = prefix + wrap('(', truthyJoin(args, ', '), ')'); if (argsLine.length > MAX_LINE_LENGTH) { - argsLine = prefix + wrap('(\n', indent(join(args, '\n')), '\n)'); + argsLine = prefix + wrap('(\n', indent(truthyJoin(args, '\n')), '\n)'); } - return join([argsLine, join(directives, ' '), selectionSet], ' '); + return join([argsLine, truthyJoin(directives, ' '), selectionSet], ' '); }, }, @@ -73,7 +74,7 @@ const printDocASTReducer: ASTReducer = { FragmentSpread: { leave: ({ name, directives }) => - '...' + name + wrap(' ', join(directives, ' ')), + '...' + name + wrap(' ', truthyJoin(directives, ' ')), }, InlineFragment: { @@ -82,7 +83,7 @@ const printDocASTReducer: ASTReducer = { [ '...', wrap('on ', typeCondition), - join(directives, ' '), + truthyJoin(directives, ' '), selectionSet, ], ' ', @@ -99,8 +100,8 @@ const printDocASTReducer: ASTReducer = { }) => // Note: fragment variable definitions are experimental and may be changed // or removed in the future. - `fragment ${name}${wrap('(', join(variableDefinitions, ', '), ')')} ` + - `on ${typeCondition} ${wrap('', join(directives, ' '), ' ')}` + + `fragment ${name}${wrap('(', truthyJoin(variableDefinitions, ', '), ')')} ` + + `on ${typeCondition} ${wrap('', truthyJoin(directives, ' '), ' ')}` + selectionSet, }, @@ -115,15 +116,15 @@ const printDocASTReducer: ASTReducer = { BooleanValue: { leave: ({ value }) => (value ? 'true' : 'false') }, NullValue: { leave: () => 'null' }, EnumValue: { leave: ({ value }) => value }, - ListValue: { leave: ({ values }) => '[' + join(values, ', ') + ']' }, - ObjectValue: { leave: ({ fields }) => '{' + join(fields, ', ') + '}' }, + ListValue: { leave: ({ values }) => '[' + truthyJoin(values, ', ') + ']' }, + ObjectValue: { leave: ({ fields }) => '{' + truthyJoin(fields, ', ') + '}' }, ObjectField: { leave: ({ name, value }) => name + ': ' + value }, // Directive Directive: { leave: ({ name, arguments: args }) => - '@' + name + wrap('(', join(args, ', '), ')'), + '@' + name + wrap('(', truthyJoin(args, ', '), ')'), }, // Type @@ -147,7 +148,7 @@ const printDocASTReducer: ASTReducer = { ScalarTypeDefinition: { leave: ({ description, name, directives }) => wrap('', description, '\n') + - join(['scalar', name, join(directives, ' ')], ' '), + join(['scalar', name, truthyJoin(directives, ' ')], ' '), }, ObjectTypeDefinition: { @@ -157,8 +158,8 @@ const printDocASTReducer: ASTReducer = { [ 'type', name, - wrap('implements ', join(interfaces, ' & ')), - join(directives, ' '), + wrap('implements ', truthyJoin(interfaces, ' & ')), + truthyJoin(directives, ' '), block(fields), ], ' ', @@ -170,18 +171,18 @@ const printDocASTReducer: ASTReducer = { wrap('', description, '\n') + name + (hasMultilineItems(args) - ? wrap('(\n', indent(join(args, '\n')), '\n)') - : wrap('(', join(args, ', '), ')')) + + ? wrap('(\n', indent(truthyJoin(args, '\n')), '\n)') + : wrap('(', truthyJoin(args, ', '), ')')) + ': ' + type + - wrap(' ', join(directives, ' ')), + wrap(' ', truthyJoin(directives, ' ')), }, InputValueDefinition: { leave: ({ description, name, type, defaultValue, directives }) => wrap('', description, '\n') + join( - [name + ': ' + type, wrap('= ', defaultValue), join(directives, ' ')], + [name + ': ' + type, wrap('= ', defaultValue), truthyJoin(directives, ' ')], ' ', ), }, @@ -193,8 +194,8 @@ const printDocASTReducer: ASTReducer = { [ 'interface', name, - wrap('implements ', join(interfaces, ' & ')), - join(directives, ' '), + wrap('implements ', truthyJoin(interfaces, ' & ')), + truthyJoin(directives, ' '), block(fields), ], ' ', @@ -205,7 +206,7 @@ const printDocASTReducer: ASTReducer = { leave: ({ description, name, directives, types }) => wrap('', description, '\n') + join( - ['union', name, join(directives, ' '), wrap('= ', join(types, ' | '))], + ['union', name, truthyJoin(directives, ' '), wrap('= ', truthyJoin(types, ' | '))], ' ', ), }, @@ -213,18 +214,18 @@ const printDocASTReducer: ASTReducer = { EnumTypeDefinition: { leave: ({ description, name, directives, values }) => wrap('', description, '\n') + - join(['enum', name, join(directives, ' '), block(values)], ' '), + join(['enum', name, truthyJoin(directives, ' '), block(values)], ' '), }, EnumValueDefinition: { leave: ({ description, name, directives }) => - wrap('', description, '\n') + join([name, join(directives, ' ')], ' '), + wrap('', description, '\n') + join([name, truthyJoin(directives, ' ')], ' '), }, InputObjectTypeDefinition: { leave: ({ description, name, directives, fields }) => wrap('', description, '\n') + - join(['input', name, join(directives, ' '), block(fields)], ' '), + join(['input', name, truthyJoin(directives, ' '), block(fields)], ' '), }, DirectiveDefinition: { @@ -233,24 +234,24 @@ const printDocASTReducer: ASTReducer = { 'directive @' + name + (hasMultilineItems(args) - ? wrap('(\n', indent(join(args, '\n')), '\n)') - : wrap('(', join(args, ', '), ')')) + + ? wrap('(\n', indent(truthyJoin(args, '\n')), '\n)') + : wrap('(', truthyJoin(args, ', '), ')')) + (repeatable ? ' repeatable' : '') + ' on ' + - join(locations, ' | '), + truthyJoin(locations, ' | '), }, SchemaExtension: { leave: ({ directives, operationTypes }) => join( - ['extend schema', join(directives, ' '), block(operationTypes)], + ['extend schema', truthyJoin(directives, ' '), block(operationTypes)], ' ', ), }, ScalarTypeExtension: { leave: ({ name, directives }) => - join(['extend scalar', name, join(directives, ' ')], ' '), + join(['extend scalar', name, truthyJoin(directives, ' ')], ' '), }, ObjectTypeExtension: { @@ -259,8 +260,8 @@ const printDocASTReducer: ASTReducer = { [ 'extend type', name, - wrap('implements ', join(interfaces, ' & ')), - join(directives, ' '), + wrap('implements ', truthyJoin(interfaces, ' & ')), + truthyJoin(directives, ' '), block(fields), ], ' ', @@ -273,8 +274,8 @@ const printDocASTReducer: ASTReducer = { [ 'extend interface', name, - wrap('implements ', join(interfaces, ' & ')), - join(directives, ' '), + wrap('implements ', truthyJoin(interfaces, ' & ')), + truthyJoin(directives, ' '), block(fields), ], ' ', @@ -287,8 +288,8 @@ const printDocASTReducer: ASTReducer = { [ 'extend union', name, - join(directives, ' '), - wrap('= ', join(types, ' | ')), + truthyJoin(directives, ' '), + wrap('= ', truthyJoin(types, ' | ')), ], ' ', ), @@ -296,12 +297,12 @@ const printDocASTReducer: ASTReducer = { EnumTypeExtension: { leave: ({ name, directives, values }) => - join(['extend enum', name, join(directives, ' '), block(values)], ' '), + join(['extend enum', name, truthyJoin(directives, ' '), block(values)], ' '), }, InputObjectTypeExtension: { leave: ({ name, directives, fields }) => - join(['extend input', name, join(directives, ' '), block(fields)], ' '), + join(['extend input', name, truthyJoin(directives, ' '), block(fields)], ' '), }, }; @@ -313,7 +314,30 @@ function join( maybeArray: Maybe>, separator = '', ): string { - return maybeArray?.filter((x) => x).join(separator) ?? ''; + if (!maybeArray) return '' + + const list = maybeArray.filter((x) => x); + const listLength = list.length; + let result = ''; + for (let i = 0; i < listLength; i++) { + if (i === listLength - 1) return result + list[i]; + else result += list[i] + separator; + } + return result +} + +function truthyJoin( + list: ReadonlyArray | undefined, + separator: string, +): string { + if (!list) return '' + const listLength = list.length; + let result = ''; + for (let i = 0; i < listLength; i++) { + if (i === listLength - 1) return result + list[i]; + else result += list[i] + separator; + } + return result } /** diff --git a/src/language/visitor.ts b/src/language/visitor.ts index daf96497bf..d87dbb91c5 100644 --- a/src/language/visitor.ts +++ b/src/language/visitor.ts @@ -17,6 +17,8 @@ type KindVisitor = { | EnterLeaveVisitor; }; +const ALL_KINDS = Object.values(Kind); + interface EnterLeaveVisitor { readonly enter?: ASTVisitFn; readonly leave?: ASTVisitFn; @@ -182,7 +184,7 @@ export function visit( visitorKeys: ASTVisitorKeyMap = QueryDocumentKeys, ): any { const enterLeaveMap = new Map>(); - for (const kind of Object.values(Kind)) { + for (const kind of ALL_KINDS) { enterLeaveMap.set(kind, getEnterLeaveForKind(visitor, kind)); } @@ -202,7 +204,7 @@ export function visit( do { index++; const isLeaving = index === keys.length; - const isEdited = isLeaving && edits.length !== 0; + const isEdited = edits.length !== 0; if (isLeaving) { key = ancestors.length === 0 ? undefined : path[path.length - 1]; node = parent; @@ -212,7 +214,8 @@ export function visit( node = node.slice(); let editOffset = 0; - for (const [editKey, editValue] of edits) { + for (let i = 0; i < edits.length; i++) { + const { 0: editKey, 1: editValue } = edits[i]; const arrayKey = editKey - editOffset; if (editValue === null) { node.splice(arrayKey, 1); @@ -222,12 +225,9 @@ export function visit( } } } else { - node = Object.defineProperties( - {}, - Object.getOwnPropertyDescriptors(node), - ); - for (const [editKey, editValue] of edits) { - node[editKey] = editValue; + node = { ...node } + for (let i = 0; i < edits.length; i++) { + node[edits[i][0]] = edits[i][1]; } } } @@ -267,7 +267,7 @@ export function visit( } else if (result !== undefined) { edits.push([key, result]); if (!isLeaving) { - if (isNode(result)) { + if (isNode(result) && result !== node) { node = result; } else { path.pop(); From 2d0415d2a171d0c72a16a09f99e893ce5f1bf88f Mon Sep 17 00:00:00 2001 From: Jovi De Croock Date: Thu, 28 Nov 2024 08:58:23 +0100 Subject: [PATCH 2/4] Fixes --- benchmark/benchmark.js | 2 +- benchmark/printer-benchmark.js | 5 +- src/language/printer.ts | 148 +++++++++++++++++++-------------- src/language/visitor.ts | 12 +-- 4 files changed, 95 insertions(+), 72 deletions(-) diff --git a/benchmark/benchmark.js b/benchmark/benchmark.js index 2948dd198e..9288d1f273 100644 --- a/benchmark/benchmark.js +++ b/benchmark/benchmark.js @@ -243,7 +243,7 @@ function maxBy(array, fn) { // Prepare all revisions and run benchmarks matching a pattern against them. async function runBenchmarks(benchmarks, benchmarkProjects) { - for (const benchmark of benchmarks.filter(x => x.includes('visit-') || x.includes('printer-'))) { + for (const benchmark of benchmarks) { const results = []; for (let i = 0; i < benchmarkProjects.length; ++i) { const { revision, projectPath } = benchmarkProjects[i]; diff --git a/benchmark/printer-benchmark.js b/benchmark/printer-benchmark.js index f578700e79..6878c23c27 100644 --- a/benchmark/printer-benchmark.js +++ b/benchmark/printer-benchmark.js @@ -2,9 +2,10 @@ const { parse } = require('graphql/language/parser.js'); const { print } = require('graphql/language/printer.js'); -const { bigDocument } = require('./fixtures') -const document = parse(bigDocument) +const { bigDocument } = require('./fixtures.js'); + +const document = parse(bigDocument); module.exports = { name: 'Print ktichen-sink query', diff --git a/src/language/printer.ts b/src/language/printer.ts index f29680ed34..9138c48518 100644 --- a/src/language/printer.ts +++ b/src/language/printer.ts @@ -23,18 +23,17 @@ const printDocASTReducer: ASTReducer = { // Document Document: { - leave: (node) => truthyJoin(node.definitions, '\n\n'), + leave: (node) => join(node.definitions, '\n\n'), }, OperationDefinition: { leave(node) { - const varDefs = wrap('(', truthyJoin(node.variableDefinitions, ', '), ')'); - const prefix = join( + const varDefs = wrap('(', join(node.variableDefinitions, ', '), ')'); + const prefix = maybeJoin( [ node.operation, - // TODO: optimize - join([node.name, varDefs]), - truthyJoin(node.directives, ' '), + maybeJoin([node.name, varDefs]), + join(node.directives, ' '), ], ' ', ); @@ -51,20 +50,20 @@ const printDocASTReducer: ASTReducer = { ': ' + type + wrap(' = ', defaultValue) + - wrap(' ', truthyJoin(directives, ' ')), + wrap(' ', join(directives, ' ')), }, SelectionSet: { leave: ({ selections }) => block(selections) }, Field: { leave({ alias, name, arguments: args, directives, selectionSet }) { const prefix = wrap('', alias, ': ') + name; - let argsLine = prefix + wrap('(', truthyJoin(args, ', '), ')'); + let argsLine = prefix + wrap('(', join(args, ', '), ')'); if (argsLine.length > MAX_LINE_LENGTH) { - argsLine = prefix + wrap('(\n', indent(truthyJoin(args, '\n')), '\n)'); + argsLine = prefix + wrap('(\n', indent(join(args, '\n')), '\n)'); } - return join([argsLine, truthyJoin(directives, ' '), selectionSet], ' '); + return maybeJoin([argsLine, join(directives, ' '), selectionSet], ' '); }, }, @@ -74,16 +73,16 @@ const printDocASTReducer: ASTReducer = { FragmentSpread: { leave: ({ name, directives }) => - '...' + name + wrap(' ', truthyJoin(directives, ' ')), + '...' + name + wrap(' ', join(directives, ' ')), }, InlineFragment: { leave: ({ typeCondition, directives, selectionSet }) => - join( + maybeJoin( [ '...', wrap('on ', typeCondition), - truthyJoin(directives, ' '), + join(directives, ' '), selectionSet, ], ' ', @@ -100,8 +99,8 @@ const printDocASTReducer: ASTReducer = { }) => // Note: fragment variable definitions are experimental and may be changed // or removed in the future. - `fragment ${name}${wrap('(', truthyJoin(variableDefinitions, ', '), ')')} ` + - `on ${typeCondition} ${wrap('', truthyJoin(directives, ' '), ' ')}` + + `fragment ${name}${wrap('(', join(variableDefinitions, ', '), ')')} ` + + `on ${typeCondition} ${wrap('', join(directives, ' '), ' ')}` + selectionSet, }, @@ -116,15 +115,15 @@ const printDocASTReducer: ASTReducer = { BooleanValue: { leave: ({ value }) => (value ? 'true' : 'false') }, NullValue: { leave: () => 'null' }, EnumValue: { leave: ({ value }) => value }, - ListValue: { leave: ({ values }) => '[' + truthyJoin(values, ', ') + ']' }, - ObjectValue: { leave: ({ fields }) => '{' + truthyJoin(fields, ', ') + '}' }, + ListValue: { leave: ({ values }) => '[' + join(values, ', ') + ']' }, + ObjectValue: { leave: ({ fields }) => '{' + join(fields, ', ') + '}' }, ObjectField: { leave: ({ name, value }) => name + ': ' + value }, // Directive Directive: { leave: ({ name, arguments: args }) => - '@' + name + wrap('(', truthyJoin(args, ', '), ')'), + '@' + name + wrap('(', join(args, ', '), ')'), }, // Type @@ -138,7 +137,10 @@ const printDocASTReducer: ASTReducer = { SchemaDefinition: { leave: ({ description, directives, operationTypes }) => wrap('', description, '\n') + - join(['schema', join(directives, ' '), block(operationTypes)], ' '), + maybeJoin( + ['schema', maybeJoin(directives, ' '), block(operationTypes)], + ' ', + ), }, OperationTypeDefinition: { @@ -148,18 +150,18 @@ const printDocASTReducer: ASTReducer = { ScalarTypeDefinition: { leave: ({ description, name, directives }) => wrap('', description, '\n') + - join(['scalar', name, truthyJoin(directives, ' ')], ' '), + maybeJoin(['scalar', name, join(directives, ' ')], ' '), }, ObjectTypeDefinition: { leave: ({ description, name, interfaces, directives, fields }) => wrap('', description, '\n') + - join( + maybeJoin( [ 'type', name, - wrap('implements ', truthyJoin(interfaces, ' & ')), - truthyJoin(directives, ' '), + wrap('implements ', join(interfaces, ' & ')), + join(directives, ' '), block(fields), ], ' ', @@ -171,18 +173,18 @@ const printDocASTReducer: ASTReducer = { wrap('', description, '\n') + name + (hasMultilineItems(args) - ? wrap('(\n', indent(truthyJoin(args, '\n')), '\n)') - : wrap('(', truthyJoin(args, ', '), ')')) + + ? wrap('(\n', indent(join(args, '\n')), '\n)') + : wrap('(', join(args, ', '), ')')) + ': ' + type + - wrap(' ', truthyJoin(directives, ' ')), + wrap(' ', join(directives, ' ')), }, InputValueDefinition: { leave: ({ description, name, type, defaultValue, directives }) => wrap('', description, '\n') + - join( - [name + ': ' + type, wrap('= ', defaultValue), truthyJoin(directives, ' ')], + maybeJoin( + [name + ': ' + type, wrap('= ', defaultValue), join(directives, ' ')], ' ', ), }, @@ -190,12 +192,12 @@ const printDocASTReducer: ASTReducer = { InterfaceTypeDefinition: { leave: ({ description, name, interfaces, directives, fields }) => wrap('', description, '\n') + - join( + maybeJoin( [ 'interface', name, - wrap('implements ', truthyJoin(interfaces, ' & ')), - truthyJoin(directives, ' '), + wrap('implements ', join(interfaces, ' & ')), + join(directives, ' '), block(fields), ], ' ', @@ -205,8 +207,8 @@ const printDocASTReducer: ASTReducer = { UnionTypeDefinition: { leave: ({ description, name, directives, types }) => wrap('', description, '\n') + - join( - ['union', name, truthyJoin(directives, ' '), wrap('= ', truthyJoin(types, ' | '))], + maybeJoin( + ['union', name, join(directives, ' '), wrap('= ', join(types, ' | '))], ' ', ), }, @@ -214,18 +216,19 @@ const printDocASTReducer: ASTReducer = { EnumTypeDefinition: { leave: ({ description, name, directives, values }) => wrap('', description, '\n') + - join(['enum', name, truthyJoin(directives, ' '), block(values)], ' '), + maybeJoin(['enum', name, join(directives, ' '), block(values)], ' '), }, EnumValueDefinition: { leave: ({ description, name, directives }) => - wrap('', description, '\n') + join([name, truthyJoin(directives, ' ')], ' '), + wrap('', description, '\n') + + maybeJoin([name, join(directives, ' ')], ' '), }, InputObjectTypeDefinition: { leave: ({ description, name, directives, fields }) => wrap('', description, '\n') + - join(['input', name, truthyJoin(directives, ' '), block(fields)], ' '), + maybeJoin(['input', name, join(directives, ' '), block(fields)], ' '), }, DirectiveDefinition: { @@ -234,34 +237,34 @@ const printDocASTReducer: ASTReducer = { 'directive @' + name + (hasMultilineItems(args) - ? wrap('(\n', indent(truthyJoin(args, '\n')), '\n)') - : wrap('(', truthyJoin(args, ', '), ')')) + + ? wrap('(\n', indent(join(args, '\n')), '\n)') + : wrap('(', join(args, ', '), ')')) + (repeatable ? ' repeatable' : '') + ' on ' + - truthyJoin(locations, ' | '), + join(locations, ' | '), }, SchemaExtension: { leave: ({ directives, operationTypes }) => - join( - ['extend schema', truthyJoin(directives, ' '), block(operationTypes)], + maybeJoin( + ['extend schema', join(directives, ' '), block(operationTypes)], ' ', ), }, ScalarTypeExtension: { leave: ({ name, directives }) => - join(['extend scalar', name, truthyJoin(directives, ' ')], ' '), + maybeJoin(['extend scalar', name, join(directives, ' ')], ' '), }, ObjectTypeExtension: { leave: ({ name, interfaces, directives, fields }) => - join( + maybeJoin( [ 'extend type', name, - wrap('implements ', truthyJoin(interfaces, ' & ')), - truthyJoin(directives, ' '), + wrap('implements ', join(interfaces, ' & ')), + join(directives, ' '), block(fields), ], ' ', @@ -270,12 +273,12 @@ const printDocASTReducer: ASTReducer = { InterfaceTypeExtension: { leave: ({ name, interfaces, directives, fields }) => - join( + maybeJoin( [ 'extend interface', name, - wrap('implements ', truthyJoin(interfaces, ' & ')), - truthyJoin(directives, ' '), + wrap('implements ', join(interfaces, ' & ')), + join(directives, ' '), block(fields), ], ' ', @@ -284,12 +287,12 @@ const printDocASTReducer: ASTReducer = { UnionTypeExtension: { leave: ({ name, directives, types }) => - join( + maybeJoin( [ 'extend union', name, - truthyJoin(directives, ' '), - wrap('= ', truthyJoin(types, ' | ')), + join(directives, ' '), + wrap('= ', join(types, ' | ')), ], ' ', ), @@ -297,12 +300,18 @@ const printDocASTReducer: ASTReducer = { EnumTypeExtension: { leave: ({ name, directives, values }) => - join(['extend enum', name, truthyJoin(directives, ' '), block(values)], ' '), + maybeJoin( + ['extend enum', name, join(directives, ' '), block(values)], + ' ', + ), }, InputObjectTypeExtension: { leave: ({ name, directives, fields }) => - join(['extend input', name, truthyJoin(directives, ' '), block(fields)], ' '), + maybeJoin( + ['extend input', name, join(directives, ' '), block(fields)], + ' ', + ), }, }; @@ -310,40 +319,51 @@ const printDocASTReducer: ASTReducer = { * Given maybeArray, print an empty string if it is null or empty, otherwise * print all items together separated by separator if provided */ -function join( +function maybeJoin( maybeArray: Maybe>, separator = '', ): string { - if (!maybeArray) return '' + if (!maybeArray) { + return ''; + } const list = maybeArray.filter((x) => x); const listLength = list.length; let result = ''; for (let i = 0; i < listLength; i++) { - if (i === listLength - 1) return result + list[i]; - else result += list[i] + separator; + if (i === listLength - 1) { + result += list[i]; + } else { + result += list[i] + separator; + } } - return result + return result; } -function truthyJoin( +function join( list: ReadonlyArray | undefined, separator: string, ): string { - if (!list) return '' + if (!list) { + return ''; + } const listLength = list.length; let result = ''; for (let i = 0; i < listLength; i++) { - if (i === listLength - 1) return result + list[i]; - else result += list[i] + separator; + if (i === listLength - 1) { + result += list[i]; + } else { + result += list[i] + separator; + } } - return result + + return result; } /** * Given array, print each item on its own line, wrapped in an indented `{ }` block. */ -function block(array: Maybe>): string { +function block(array: ReadonlyArray | undefined): string { return wrap('{\n', indent(join(array, '\n')), '\n}'); } diff --git a/src/language/visitor.ts b/src/language/visitor.ts index d87dbb91c5..04f0f4353c 100644 --- a/src/language/visitor.ts +++ b/src/language/visitor.ts @@ -214,8 +214,7 @@ export function visit( node = node.slice(); let editOffset = 0; - for (let i = 0; i < edits.length; i++) { - const { 0: editKey, 1: editValue } = edits[i]; + for (const [editKey, editValue] of edits) { const arrayKey = editKey - editOffset; if (editValue === null) { node.splice(arrayKey, 1); @@ -225,9 +224,12 @@ export function visit( } } } else { - node = { ...node } - for (let i = 0; i < edits.length; i++) { - node[edits[i][0]] = edits[i][1]; + node = Object.defineProperties( + {}, + Object.getOwnPropertyDescriptors(node), + ); + for (const [editKey, editValue] of edits) { + node[editKey] = editValue; } } } From e87c74c89f88f0ed2d58a678911759f04047bf28 Mon Sep 17 00:00:00 2001 From: Jovi De Croock Date: Thu, 28 Nov 2024 09:06:54 +0100 Subject: [PATCH 3/4] Improve spelling --- benchmark/printer-benchmark.js | 2 +- cspell.yml | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/benchmark/printer-benchmark.js b/benchmark/printer-benchmark.js index 6878c23c27..efe5d46e58 100644 --- a/benchmark/printer-benchmark.js +++ b/benchmark/printer-benchmark.js @@ -8,7 +8,7 @@ const { bigDocument } = require('./fixtures.js'); const document = parse(bigDocument); module.exports = { - name: 'Print ktichen-sink query', + name: 'Print kitchen-sink query', count: 1000, measure() { print(document); diff --git a/cspell.yml b/cspell.yml index ff26b0902b..8382683188 100644 --- a/cspell.yml +++ b/cspell.yml @@ -51,6 +51,7 @@ words: # TODO: contribute upstream - deno - codecov + - falsey # Website tech - Nextra From 9b884ef62b3a877d8b6e8b3cd75f8d2a16021ba2 Mon Sep 17 00:00:00 2001 From: Jovi De Croock Date: Thu, 28 Nov 2024 09:11:47 +0100 Subject: [PATCH 4/4] Identify massive deopt --- src/language/printer.ts | 11 ++--------- src/language/visitor.ts | 5 +---- 2 files changed, 3 insertions(+), 13 deletions(-) diff --git a/src/language/printer.ts b/src/language/printer.ts index 9138c48518..1eb690ca8c 100644 --- a/src/language/printer.ts +++ b/src/language/printer.ts @@ -137,10 +137,7 @@ const printDocASTReducer: ASTReducer = { SchemaDefinition: { leave: ({ description, directives, operationTypes }) => wrap('', description, '\n') + - maybeJoin( - ['schema', maybeJoin(directives, ' '), block(operationTypes)], - ' ', - ), + maybeJoin(['schema', join(directives, ' '), block(operationTypes)], ' '), }, OperationTypeDefinition: { @@ -320,13 +317,9 @@ const printDocASTReducer: ASTReducer = { * print all items together separated by separator if provided */ function maybeJoin( - maybeArray: Maybe>, + maybeArray: ReadonlyArray, separator = '', ): string { - if (!maybeArray) { - return ''; - } - const list = maybeArray.filter((x) => x); const listLength = list.length; let result = ''; diff --git a/src/language/visitor.ts b/src/language/visitor.ts index 04f0f4353c..1e0c56e4a8 100644 --- a/src/language/visitor.ts +++ b/src/language/visitor.ts @@ -224,10 +224,7 @@ export function visit( } } } else { - node = Object.defineProperties( - {}, - Object.getOwnPropertyDescriptors(node), - ); + node = { ...node }; for (const [editKey, editValue] of edits) { node[editKey] = editValue; }