From b489677b6651f90a4564c570c43972f068f40c85 Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Sun, 28 Jun 2020 15:34:25 +0100 Subject: [PATCH] Add parentType to path to avoid path ambiguity (#2669) Co-authored-by: Ivan Goncharov --- src/execution/__tests__/executor-test.js | 62 +++++++++++++++++++++++- src/execution/execute.js | 6 +-- src/jsutils/Path.d.ts | 7 ++- src/jsutils/Path.js | 4 +- src/subscription/subscribe.js | 2 +- src/utilities/coerceInputValue.js | 4 +- 6 files changed, 76 insertions(+), 9 deletions(-) diff --git a/src/execution/__tests__/executor-test.js b/src/execution/__tests__/executor-test.js index e37c561ec9..1260ec4e49 100644 --- 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 index 33f2892a93..36c9eb6212 100644 --- 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 index 28bba41712..9a2233dd60 100644 --- 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 index f477354e07..3ba629d5e6 100644 --- 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 | 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 index 0651e84dfc..f4eece9ec9 100644 --- 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 index 36e4eae0b2..f595acea86 100644 --- 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), ); }