Add parentType to path to avoid path ambiguity (#2669)

Co-authored-by: Ivan Goncharov <ivan.goncharov.ua@gmail.com>
graphql · Jun 28, 2020 · b489677 · b489677
1 parent 2232ebd
commit b489677
Show file tree

Hide file tree

Showing 6 changed files with 76 additions and 9 deletions.
diff --git a/src/execution/__tests__/executor-test.js b/src/execution/__tests__/executor-test.js
@@ -17,6 +17,7 @@ import {
   GraphQLScalarType,
   GraphQLInterfaceType,
   GraphQLObjectType,
+  GraphQLUnionType,
 } from '../../type/definition';
 
 import { execute, executeSync } from '../execute';
@@ -306,11 +307,70 @@ describe('Execute: Handles basic execution tasks', () => {
     const field = operation.selectionSet.selections[0];
     expect(resolvedInfo).to.deep.include({
       fieldNodes: [field],
-      path: { prev: undefined, key: 'result' },
+      path: { prev: undefined, key: 'result', typename: 'Test' },
       variableValues: { var: 'abc' },
     });
   });
 
+  it('populates path correctly with complex types', () => {
+    let path;
+    const someObject = new GraphQLObjectType({
+      name: 'SomeObject',
+      fields: {
+        test: {
+          type: GraphQLString,
+          resolve(_val, _args, _ctx, info) {
+            path = info.path;
+          },
+        },
+      },
+    });
+    const someUnion = new GraphQLUnionType({
+      name: 'SomeUnion',
+      types: [someObject],
+      resolveType() {
+        return 'SomeObject';
+      },
+    });
+    const testType = new GraphQLObjectType({
+      name: 'SomeQuery',
+      fields: {
+        test: {
+          type: new GraphQLNonNull(
+            new GraphQLList(new GraphQLNonNull(someUnion)),
+          ),
+        },
+      },
+    });
+    const schema = new GraphQLSchema({ query: testType });
+    const rootValue = { test: [{}] };
+    const document = parse(`
+      query {
+        l1: test {
+          ... on SomeObject {
+            l2: test
+          }
+        }
+      }
+    `);
+
+    executeSync({ schema, document, rootValue });
+
+    expect(path).to.deep.equal({
+      key: 'l2',
+      typename: 'SomeObject',
+      prev: {
+        key: 0,
+        typename: undefined,
+        prev: {
+          key: 'l1',
+          typename: 'SomeQuery',
+          prev: undefined,
+        },
+      },
+    });
+  });
+
   it('threads root value context correctly', () => {
     let resolvedRootValue;
     const schema = new GraphQLSchema({

diff --git a/src/execution/execute.js b/src/execution/execute.js
@@ -416,7 +416,7 @@ function executeFieldsSerially(
     Object.keys(fields),
     (results, responseName) => {
       const fieldNodes = fields[responseName];
-      const fieldPath = addPath(path, responseName);
+      const fieldPath = addPath(path, responseName, parentType.name);
       const result = resolveField(
         exeContext,
         parentType,
@@ -456,7 +456,7 @@ function executeFields(
 
   for (const responseName of Object.keys(fields)) {
     const fieldNodes = fields[responseName];
-    const fieldPath = addPath(path, responseName);
+    const fieldPath = addPath(path, responseName, parentType.name);
     const result = resolveField(
       exeContext,
       parentType,
@@ -934,7 +934,7 @@ function completeListValue(
   const completedResults = arrayFrom(result, (item, index) => {
     // No need to modify the info object containing the path,
     // since from here on it is not ever accessed by resolver functions.
-    const fieldPath = addPath(path, index);
+    const fieldPath = addPath(path, index, undefined);
     const completedItem = completeValueCatchingError(
       exeContext,
       itemType,

diff --git a/src/jsutils/Path.d.ts b/src/jsutils/Path.d.ts
@@ -1,12 +1,17 @@
 export interface Path {
   prev: Path | undefined;
   key: string | number;
+  typename: string | undefined;
 }
 
 /**
  * Given a Path and a key, return a new Path containing the new key.
  */
-export function addPath(prev: Path | undefined, key: string | number): Path;
+export function addPath(
+  prev: Path | undefined,
+  key: string | number,
+  typename: string | undefined,
+): Path;
 
 /**
  * Given a Path, return an Array of the path keys.

diff --git a/src/jsutils/Path.js b/src/jsutils/Path.js
@@ -3,6 +3,7 @@
 export type Path = {|
   +prev: Path | void,
   +key: string | number,
+  +typename: string | void,
 |};
 
 /**
@@ -11,8 +12,9 @@ export type Path = {|
 export function addPath(
   prev: $ReadOnly<Path> | void,
   key: string | number,
+  typename: string | void,
 ): Path {
-  return { prev, key };
+  return { prev, key, typename };
 }
 
 /**

diff --git a/src/subscription/subscribe.js b/src/subscription/subscribe.js
@@ -253,7 +253,7 @@ export function createSourceEventStream(
     // AsyncIterable yielding raw payloads.
     const resolveFn = fieldDef.subscribe ?? exeContext.fieldResolver;
 
-    const path = addPath(undefined, responseName);
+    const path = addPath(undefined, responseName, type.name);
 
     const info = buildResolveInfo(exeContext, fieldDef, fieldNodes, type, path);
 

diff --git a/src/utilities/coerceInputValue.js b/src/utilities/coerceInputValue.js
@@ -82,7 +82,7 @@ function coerceInputValueImpl(
     const itemType = type.ofType;
     if (isCollection(inputValue)) {
       return arrayFrom(inputValue, (itemValue, index) => {
-        const itemPath = addPath(path, index);
+        const itemPath = addPath(path, index, undefined);
         return coerceInputValueImpl(itemValue, itemType, onError, itemPath);
       });
     }
@@ -126,7 +126,7 @@ function coerceInputValueImpl(
         fieldValue,
         field.type,
         onError,
-        addPath(path, field.name),
+        addPath(path, field.name, type.name),
       );
     }