memoize Path generation so paths can be used as keys directly across branches

Author: yaacovCR

src/execution/__tests__/defer-test.ts

@@ -591,7 +591,7 @@ describe('Execute: defer directive', () => {
     ]);
   });
 
-  it('Can deduplicate initial fields with deferred fragments at multiple levels', async () => {
+  it('Can deduplicate fields with deferred fragments at multiple levels', async () => {
     const document = parse(`
       query {
         hero {
@@ -732,7 +732,7 @@ describe('Execute: defer directive', () => {
     ]);
   });
 
-  it('can deduplicate initial fields with deferred fragments in different branches at multiple non-overlapping levels', async () => {
+  it('can deduplicate fields with deferred fragments in different branches at multiple non-overlapping levels', async () => {
     const document = parse(`
       query {
         a {

src/execution/__tests__/executor-test.ts

@@ -5,10 +5,12 @@ import { expectJSON } from '../../__testUtils__/expectJSON.js';
 import { resolveOnNextTick } from '../../__testUtils__/resolveOnNextTick.js';
 
 import { inspect } from '../../jsutils/inspect.js';
+import type { Path } from '../../jsutils/Path.js';
 
 import { Kind } from '../../language/kinds.js';
 import { parse } from '../../language/parser.js';
 
+import type { GraphQLResolveInfo } from '../../type/definition.js';
 import {
   GraphQLInterfaceType,
   GraphQLList,
@@ -191,7 +193,7 @@ describe('Execute: Handles basic execution tasks', () => {
   });
 
   it('provides info about current execution state', () => {
-    let resolvedInfo;
+    let resolvedInfo: GraphQLResolveInfo | undefined;
     const testType = new GraphQLObjectType({
       name: 'Test',
       fields: {
@@ -239,13 +241,18 @@ describe('Execute: Handles basic execution tasks', () => {
     const field = operation.selectionSet.selections[0];
     expect(resolvedInfo).to.deep.include({
       fieldNodes: [field],
-      path: { prev: undefined, key: 'result', typename: 'Test' },
       variableValues: { var: 'abc' },
     });
+
+    expect(resolvedInfo?.path).to.deep.include({
+      prev: undefined,
+      key: 'result',
+      typename: 'Test',
+    });
   });
 
   it('populates path correctly with complex types', () => {
-    let path;
+    let path: Path | undefined;
     const someObject = new GraphQLObjectType({
       name: 'SomeObject',
       fields: {
@@ -288,18 +295,20 @@ describe('Execute: Handles basic execution tasks', () => {
 
     executeSync({ schema, document, rootValue });
 
-    expect(path).to.deep.equal({
+    expect(path).to.deep.include({
       key: 'l2',
       typename: 'SomeObject',
-      prev: {
-        key: 0,
-        typename: undefined,
-        prev: {
-          key: 'l1',
-          typename: 'SomeQuery',
-          prev: undefined,
-        },
-      },
+    });
+
+    expect(path?.prev).to.deep.include({
+      key: 0,
+      typename: undefined,
+    });
+
+    expect(path?.prev?.prev).to.deep.include({
+      key: 'l1',
+      typename: 'SomeQuery',
+      prev: undefined,
     });
   });
 

src/execution/execute.ts

@@ -7,8 +7,8 @@ import { isPromise } from '../jsutils/isPromise.js';
 import type { Maybe } from '../jsutils/Maybe.js';
 import { memoize3 } from '../jsutils/memoize3.js';
 import type { ObjMap } from '../jsutils/ObjMap.js';
-import type { Path } from '../jsutils/Path.js';
-import { addPath, pathToArray } from '../jsutils/Path.js';
+import type { Path, PathFactory } from '../jsutils/Path.js';
+import { createPathFactory, pathToArray } from '../jsutils/Path.js';
 import { promiseForObject } from '../jsutils/promiseForObject.js';
 import type { PromiseOrValue } from '../jsutils/PromiseOrValue.js';
 import { promiseReduce } from '../jsutils/promiseReduce.js';
@@ -121,9 +121,10 @@ export interface ExecutionContext {
   fieldResolver: GraphQLFieldResolver<any, any>;
   typeResolver: GraphQLTypeResolver<any, any>;
   subscribeFieldResolver: GraphQLFieldResolver<any, any>;
+  addPath: PathFactory;
   errors: Array<GraphQLError>;
   subsequentPayloads: Set<AsyncPayloadRecord>;
-  branches: WeakMap<GroupedFieldSet, Set<string>>;
+  branches: WeakMap<GroupedFieldSet, Set<Path | undefined>>;
 }
 
 /**
@@ -501,6 +502,7 @@ export function buildExecutionContext(
     fieldResolver: fieldResolver ?? defaultFieldResolver,
     typeResolver: typeResolver ?? defaultTypeResolver,
     subscribeFieldResolver: subscribeFieldResolver ?? defaultFieldResolver,
+    addPath: createPathFactory(),
     subsequentPayloads: new Set(),
     branches: new WeakMap(),
     errors: [],
@@ -514,6 +516,7 @@ function buildPerEventExecutionContext(
   return {
     ...exeContext,
     rootValue: payload,
+    addPath: createPathFactory(),
     subsequentPayloads: new Set(),
     branches: new WeakMap(),
     errors: [],
@@ -526,13 +529,12 @@ function shouldBranch(
   path: Path | undefined,
 ): boolean {
   const set = exeContext.branches.get(groupedFieldSet);
-  const key = pathToArray(path).join('.');
   if (set === undefined) {
-    exeContext.branches.set(groupedFieldSet, new Set([key]));
+    exeContext.branches.set(groupedFieldSet, new Set([path]));
     return true;
   }
-  if (!set.has(key)) {
-    set.add(key);
+  if (!set.has(path)) {
+    set.add(path);
     return true;
   }
   return false;
@@ -626,7 +628,7 @@ function executeFieldsSerially(
   return promiseReduce(
     groupedFieldSet,
     (results, [responseName, fieldSet]) => {
-      const fieldPath = addPath(path, responseName, parentType.name);
+      const fieldPath = exeContext.addPath(path, responseName, parentType.name);
 
       const fieldName = fieldSet[0].fieldNode.name.value;
       const fieldDef = exeContext.schema.getField(parentType, fieldName);
@@ -701,7 +703,7 @@ function executeFields(
 
   try {
     for (const [responseName, fieldSet] of groupedFieldSet) {
-      const fieldPath = addPath(path, responseName, parentType.name);
+      const fieldPath = exeContext.addPath(path, responseName, parentType.name);
 
       const fieldName = fieldSet[0].fieldNode.name.value;
       const fieldDef = exeContext.schema.getField(parentType, fieldName);
@@ -1141,7 +1143,7 @@ async function completeAsyncIteratorValue(
       break;
     }
 
-    const itemPath = addPath(path, index, undefined);
+    const itemPath = exeContext.addPath(path, index, undefined);
     let iteration;
     try {
       // eslint-disable-next-line no-await-in-loop
@@ -1227,7 +1229,7 @@ function completeListValue(
   for (const item of result) {
     // No need to modify the info object containing the path,
     // since from here on it is not ever accessed by resolver functions.
-    const itemPath = addPath(path, index, undefined);
+    const itemPath = exeContext.addPath(path, index, undefined);
 
     if (
       stream &&
@@ -1815,7 +1817,7 @@ function executeSubscription(
     );
   }
 
-  const path = addPath(undefined, responseName, rootType.name);
+  const path = exeContext.addPath(undefined, responseName, rootType.name);
   const info = buildResolveInfo(exeContext, fieldDef, fieldSet, rootType, path);
 
   try {
@@ -2102,7 +2104,7 @@ async function executeStreamIterator(
   let previousAsyncPayloadRecord = parentContext ?? undefined;
   // eslint-disable-next-line no-constant-condition
   while (true) {
-    const itemPath = addPath(path, index, undefined);
+    const itemPath = exeContext.addPath(path, index, undefined);
     const asyncPayloadRecord = new StreamRecord({
       path: itemPath,
       deferDepth,

src/jsutils/Path.ts

@@ -1,20 +1,39 @@
 import type { Maybe } from './Maybe.js';
 
-export interface Path {
+/**
+ * @internal
+ */
+export class Path {
   readonly prev: Path | undefined;
   readonly key: string | number;
   readonly typename: string | undefined;
-}
 
-/**
- * Given a Path and a key, return a new Path containing the new key.
- */
-export function addPath(
-  prev: Readonly<Path> | undefined,
-  key: string | number,
-  typename: string | undefined,
-): Path {
-  return { prev, key, typename };
+  readonly _subPaths: Map<string | number, Path>;
+
+  constructor(
+    prev: Path | undefined,
+    key: string | number,
+    typename: string | undefined,
+  ) {
+    this.prev = prev;
+    this.key = key;
+    this.typename = typename;
+    this._subPaths = new Map();
+  }
+
+  /**
+   * Given a Path and a key, return a new Path containing the new key.
+   */
+  addPath(key: string | number, typeName: string | undefined): Path {
+    let path = this._subPaths.get(key);
+    if (path !== undefined) {
+      return path;
+    }
+
+    path = new Path(this, key, typeName);
+    this._subPaths.set(key, path);
+    return path;
+  }
 }
 
 /**
@@ -31,3 +50,27 @@ export function pathToArray(
   }
   return flattened.reverse();
 }
+
+export type PathFactory = (
+  path: Path | undefined,
+  key: string | number,
+  typeName: string | undefined,
+) => Path;
+
+export function createPathFactory(): PathFactory {
+  const paths = new Map<string, Path>();
+  return (path, key, typeName) => {
+    if (path !== undefined) {
+      return path.addPath(key, typeName);
+    }
+
+    let newPath = paths.get(key as string);
+    if (newPath !== undefined) {
+      return newPath;
+    }
+
+    newPath = new Path(undefined, key, typeName);
+    paths.set(key as string, newPath);
+    return newPath;
+  };
+}

src/jsutils/__tests__/Path-test.ts

@@ -1,34 +1,34 @@
 import { expect } from 'chai';
 import { describe, it } from 'mocha';
 
-import { addPath, pathToArray } from '../Path.js';
+import { Path, pathToArray } from '../Path.js';
 
 describe('Path', () => {
   it('can create a Path', () => {
-    const first = addPath(undefined, 1, 'First');
+    const first = new Path(undefined, 1, 'First');
 
-    expect(first).to.deep.equal({
+    expect(first).to.deep.include({
       prev: undefined,
       key: 1,
       typename: 'First',
     });
   });
 
   it('can add a new key to an existing Path', () => {
-    const first = addPath(undefined, 1, 'First');
-    const second = addPath(first, 'two', 'Second');
+    const first = new Path(undefined, 1, 'First');
+    const second = first.addPath('two', 'Second');
 
-    expect(second).to.deep.equal({
+    expect(second).to.deep.include({
       prev: first,
       key: 'two',
       typename: 'Second',
     });
   });
 
   it('can convert a Path to an array of its keys', () => {
-    const root = addPath(undefined, 0, 'Root');
-    const first = addPath(root, 'one', 'First');
-    const second = addPath(first, 2, 'Second');
+    const root = new Path(undefined, 0, 'Root');
+    const first = root.addPath('one', 'First');
+    const second = first.addPath(2, 'Second');
 
     const path = pathToArray(second);
     expect(path).to.deep.equal([0, 'one', 2]);

src/utilities/coerceInputValue.ts

@@ -3,8 +3,7 @@ import { inspect } from '../jsutils/inspect.js';
 import { invariant } from '../jsutils/invariant.js';
 import { isIterableObject } from '../jsutils/isIterableObject.js';
 import { isObjectLike } from '../jsutils/isObjectLike.js';
-import type { Path } from '../jsutils/Path.js';
-import { addPath, pathToArray } from '../jsutils/Path.js';
+import { Path, pathToArray } from '../jsutils/Path.js';
 import { printPathArray } from '../jsutils/printPathArray.js';
 import { suggestionList } from '../jsutils/suggestionList.js';
 
@@ -48,6 +47,16 @@ function defaultOnError(
   throw error;
 }
 
+function addPath(
+  path: Path | undefined,
+  key: string | number,
+  typeName: string | undefined,
+): Path {
+  return path
+    ? path.addPath(key, typeName)
+    : new Path(undefined, key, typeName);
+}
+
 function coerceInputValueImpl(
   inputValue: unknown,
   type: GraphQLInputType,