From e05db77641000919edc8c4ca8db888917ffea5d9 Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Fri, 19 Jun 2020 17:36:21 +0100 Subject: [PATCH 1/7] Add parentType to path to avoid path ambiguity --- src/execution/__tests__/executor-test.js | 2 +- 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, 16 insertions(+), 9 deletions(-) diff --git a/src/execution/__tests__/executor-test.js b/src/execution/__tests__/executor-test.js index c6684372e7..7c71aadca9 100644 --- a/src/execution/__tests__/executor-test.js +++ b/src/execution/__tests__/executor-test.js @@ -306,7 +306,7 @@ 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', parentType: 'Test' }, variableValues: { var: 'abc' }, }); }); diff --git a/src/execution/execute.js b/src/execution/execute.js index 550dfeab4a..9a6f69aba9 100644 --- a/src/execution/execute.js +++ b/src/execution/execute.js @@ -400,7 +400,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, @@ -440,7 +440,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, @@ -918,7 +918,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..bebca6c3d8 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; + parentType: 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, + parentType: 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..b1d1f41bd3 100644 --- a/src/jsutils/Path.js +++ b/src/jsutils/Path.js @@ -3,6 +3,7 @@ export type Path = {| +prev: Path | void, +key: string | number, + +parentType: string | void, |}; /** @@ -11,8 +12,9 @@ export type Path = {| export function addPath( prev: $ReadOnly | void, key: string | number, + parentType: string | void, ): Path { - return { prev, key }; + return { prev, key, parentType }; } /** 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..69bd20710d 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, undefined), ); } From 3251530b14060838aa029989291f817a5c965bc3 Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Fri, 19 Jun 2020 18:10:39 +0100 Subject: [PATCH 2/7] Add a test for complex path --- src/execution/__tests__/executor-test.js | 55 ++++++++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/src/execution/__tests__/executor-test.js b/src/execution/__tests__/executor-test.js index 7c71aadca9..8d86531279 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 } from '../execute'; @@ -311,6 +312,60 @@ describe('Execute: Handles basic execution tasks', () => { }); }); + it('populates path correctly with complex types', () => { + let path; + const a = new GraphQLObjectType({ + name: 'A', + fields: { + test: { + type: GraphQLString, + resolve(_val, _args, _ctx, info) { + path = info.path; + }, + }, + }, + }); + const testUnion = new GraphQLUnionType({ + name: 'TestUnion', + types: [a], + resolveType() { + return a; + }, + }); + const testType = new GraphQLObjectType({ + name: 'Test', + fields: { + test: { + type: new GraphQLNonNull( + new GraphQLList(new GraphQLNonNull(testUnion)), + ), + resolve(_val, _args, _ctx, _info) { + return [{ type: 'A' }]; + }, + }, + }, + }); + const schema = new GraphQLSchema({ query: testType }); + + const document = parse('query { l1: test { ... on A { l2: test } } }'); + + execute({ schema, document }); + + expect(path).to.deep.equal({ + key: 'l2', + parentType: 'A', + prev: { + key: 0, + parentType: undefined, + prev: { + key: 'l1', + parentType: 'Test', + prev: undefined, + }, + }, + }); + }); + it('threads root value context correctly', () => { let resolvedRootValue; const schema = new GraphQLSchema({ From 48c4b32b9cd96adb34404e2d6cae7046789e5661 Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Fri, 19 Jun 2020 18:13:08 +0100 Subject: [PATCH 3/7] Add parentType to input object paths --- src/utilities/coerceInputValue.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utilities/coerceInputValue.js b/src/utilities/coerceInputValue.js index 69bd20710d..f595acea86 100644 --- a/src/utilities/coerceInputValue.js +++ b/src/utilities/coerceInputValue.js @@ -126,7 +126,7 @@ function coerceInputValueImpl( fieldValue, field.type, onError, - addPath(path, field.name, undefined), + addPath(path, field.name, type.name), ); } From 26ff817e60495ca230795a75c7cd04b12d249b9b Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Sat, 20 Jun 2020 10:27:46 +0100 Subject: [PATCH 4/7] Use name of type rather than type itself in resolveType Co-authored-by: Ivan Goncharov --- src/execution/__tests__/executor-test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/execution/__tests__/executor-test.js b/src/execution/__tests__/executor-test.js index 8d86531279..4e3c320db2 100644 --- a/src/execution/__tests__/executor-test.js +++ b/src/execution/__tests__/executor-test.js @@ -329,7 +329,7 @@ describe('Execute: Handles basic execution tasks', () => { name: 'TestUnion', types: [a], resolveType() { - return a; + return 'A'; }, }); const testType = new GraphQLObjectType({ From e72282be2f99980508be0d67f61f13e3496983ce Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Sat, 20 Jun 2020 10:36:14 +0100 Subject: [PATCH 5/7] Rename test types to Some* --- src/execution/__tests__/executor-test.js | 37 ++++++++++++++---------- 1 file changed, 21 insertions(+), 16 deletions(-) diff --git a/src/execution/__tests__/executor-test.js b/src/execution/__tests__/executor-test.js index 4e3c320db2..4750e31eba 100644 --- a/src/execution/__tests__/executor-test.js +++ b/src/execution/__tests__/executor-test.js @@ -314,8 +314,8 @@ describe('Execute: Handles basic execution tasks', () => { it('populates path correctly with complex types', () => { let path; - const a = new GraphQLObjectType({ - name: 'A', + const someObject = new GraphQLObjectType({ + name: 'SomeObject', fields: { test: { type: GraphQLString, @@ -325,41 +325,46 @@ describe('Execute: Handles basic execution tasks', () => { }, }, }); - const testUnion = new GraphQLUnionType({ - name: 'TestUnion', - types: [a], + const someUnion = new GraphQLUnionType({ + name: 'SomeUnion', + types: [someObject], resolveType() { - return 'A'; + return 'SomeObject'; }, }); const testType = new GraphQLObjectType({ - name: 'Test', + name: 'SomeQuery', fields: { test: { type: new GraphQLNonNull( - new GraphQLList(new GraphQLNonNull(testUnion)), + new GraphQLList(new GraphQLNonNull(someUnion)), ), - resolve(_val, _args, _ctx, _info) { - return [{ type: 'A' }]; - }, }, }, }); const schema = new GraphQLSchema({ query: testType }); + const rootValue = { test: [{}] }; + const document = parse(` + query { + l1: test { + ... on SomeObject { + l2: test + } + } + } + `); - const document = parse('query { l1: test { ... on A { l2: test } } }'); - - execute({ schema, document }); + execute({ schema, document, rootValue }); expect(path).to.deep.equal({ key: 'l2', - parentType: 'A', + parentType: 'SomeObject', prev: { key: 0, parentType: undefined, prev: { key: 'l1', - parentType: 'Test', + parentType: 'SomeQuery', prev: undefined, }, }, From b7d026549389efa6c82f7955a993b88878ad1b18 Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Sat, 20 Jun 2020 10:39:08 +0100 Subject: [PATCH 6/7] Rename Path.parentType -> Path.typename --- src/execution/__tests__/executor-test.js | 8 ++++---- src/jsutils/Path.d.ts | 4 ++-- src/jsutils/Path.js | 6 +++--- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/execution/__tests__/executor-test.js b/src/execution/__tests__/executor-test.js index 4750e31eba..89b337b20b 100644 --- a/src/execution/__tests__/executor-test.js +++ b/src/execution/__tests__/executor-test.js @@ -307,7 +307,7 @@ describe('Execute: Handles basic execution tasks', () => { const field = operation.selectionSet.selections[0]; expect(resolvedInfo).to.deep.include({ fieldNodes: [field], - path: { prev: undefined, key: 'result', parentType: 'Test' }, + path: { prev: undefined, key: 'result', typename: 'Test' }, variableValues: { var: 'abc' }, }); }); @@ -358,13 +358,13 @@ describe('Execute: Handles basic execution tasks', () => { expect(path).to.deep.equal({ key: 'l2', - parentType: 'SomeObject', + typename: 'SomeObject', prev: { key: 0, - parentType: undefined, + typename: undefined, prev: { key: 'l1', - parentType: 'SomeQuery', + typename: 'SomeQuery', prev: undefined, }, }, diff --git a/src/jsutils/Path.d.ts b/src/jsutils/Path.d.ts index bebca6c3d8..9a2233dd60 100644 --- a/src/jsutils/Path.d.ts +++ b/src/jsutils/Path.d.ts @@ -1,7 +1,7 @@ export interface Path { prev: Path | undefined; key: string | number; - parentType: string | undefined; + typename: string | undefined; } /** @@ -10,7 +10,7 @@ export interface Path { export function addPath( prev: Path | undefined, key: string | number, - parentType: string | undefined, + typename: string | undefined, ): Path; /** diff --git a/src/jsutils/Path.js b/src/jsutils/Path.js index b1d1f41bd3..3ba629d5e6 100644 --- a/src/jsutils/Path.js +++ b/src/jsutils/Path.js @@ -3,7 +3,7 @@ export type Path = {| +prev: Path | void, +key: string | number, - +parentType: string | void, + +typename: string | void, |}; /** @@ -12,9 +12,9 @@ export type Path = {| export function addPath( prev: $ReadOnly | void, key: string | number, - parentType: string | void, + typename: string | void, ): Path { - return { prev, key, parentType }; + return { prev, key, typename }; } /** From 02827d0eda0b0f675d24ac8df963f2dca642f01e Mon Sep 17 00:00:00 2001 From: Benjie Gillam Date: Thu, 25 Jun 2020 18:03:41 +0100 Subject: [PATCH 7/7] Update src/execution/__tests__/executor-test.js Co-authored-by: Ivan Goncharov --- src/execution/__tests__/executor-test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/execution/__tests__/executor-test.js b/src/execution/__tests__/executor-test.js index 89b337b20b..448d608368 100644 --- a/src/execution/__tests__/executor-test.js +++ b/src/execution/__tests__/executor-test.js @@ -354,7 +354,7 @@ describe('Execute: Handles basic execution tasks', () => { } `); - execute({ schema, document, rootValue }); + executeSync({ schema, document, rootValue }); expect(path).to.deep.equal({ key: 'l2',