From 49c62e1ff7366c27535712434ead797444a12363 Mon Sep 17 00:00:00 2001 From: Vladimir Razuvaev Date: Fri, 24 Jan 2020 20:06:36 +0200 Subject: [PATCH 1/2] fix(gatsby): add `__typename` and `id` fields in queries to match previous behavior of relay-compiler --- .../__snapshots__/query-compiler.js.snap | 663 ++++++++++++++++++ .../fixtures/query-compiler-schema.graphql | 3 + .../src/query/__tests__/query-compiler.js | 327 +++++++++ packages/gatsby/src/query/query-compiler.js | 78 ++- 4 files changed, 1070 insertions(+), 1 deletion(-) diff --git a/packages/gatsby/src/query/__tests__/__snapshots__/query-compiler.js.snap b/packages/gatsby/src/query/__tests__/__snapshots__/query-compiler.js.snap index 5ebc996d9d692..156982c7f0a13 100644 --- a/packages/gatsby/src/query/__tests__/__snapshots__/query-compiler.js.snap +++ b/packages/gatsby/src/query/__tests__/__snapshots__/query-compiler.js.snap @@ -1,5 +1,662 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP +exports[`Extra fields adds __typename field to abstract types 1`] = ` +Object { + "hash": "hash", + "isHook": false, + "isStaticQuery": false, + "name": "mockFileQuery", + "originalText": " + query mockFileQuery { + allDirectory { + nodes { + contents { + ... on File { + id + } + } + children { + id + } + } + } + } + ", + "path": "mockFile", + "text": "query mockFileQuery { + allDirectory { + nodes { + id + contents { + __typename + ... on File { + id + } + } + children { + __typename + id + } + } + } +} +", +} +`; + +exports[`Extra fields adds __typename field to abstract types in the query of arbitrary depth 1`] = ` +Object { + "hash": "hash", + "isHook": false, + "isStaticQuery": false, + "name": "mockFileQuery", + "originalText": " + query mockFileQuery { + allDirectory { + nodes { + contents { + ... on Directory { + contents { + ... on Directory { + contents { + ... on Directory { + id + } + } + children { + id + } + } + } + } + } + children { + children { + children { + id + } + ... on Directory { + contents { + ... on File { + id + } + } + } + } + } + } + } + } + ", + "path": "mockFile", + "text": "query mockFileQuery { + allDirectory { + nodes { + id + contents { + __typename + ... on Directory { + id + contents { + __typename + ... on Directory { + id + contents { + __typename + ... on Directory { + id + } + } + children { + __typename + id + } + } + } + } + } + children { + __typename + id + children { + __typename + id + children { + __typename + id + } + ... on Directory { + id + contents { + __typename + ... on File { + id + } + } + } + } + } + } + } +} +", +} +`; + +exports[`Extra fields adds __typename field to abstract types within fragments 1`] = ` +Object { + "hash": "hash", + "isHook": false, + "isStaticQuery": false, + "name": "mockFileQuery", + "originalText": " + query mockFileQuery { + allDirectory { + nodes { + ...DirectoryContents + } + } + } + fragment DirectoryContents on Directory { + contents { + ... on File { + id + } + } + children { + id + } + } + ", + "path": "mockFile", + "text": "fragment DirectoryContents on Directory { + id + contents { + __typename + ... on File { + id + } + } + children { + __typename + id + } +} + +query mockFileQuery { + allDirectory { + nodes { + id + ...DirectoryContents + } + } +} +", +} +`; + +exports[`Extra fields adds __typename field to abstract types within inline fragments 1`] = ` +Object { + "hash": "hash", + "isHook": false, + "isStaticQuery": false, + "name": "mockFileQuery", + "originalText": " + query mockFileQuery { + allDirectory { + nodes { + ... on Directory { + contents { + ... on File { + id + } + } + children { + id + } + } + } + } + } + ", + "path": "mockFile", + "text": "query mockFileQuery { + allDirectory { + nodes { + id + ... on Directory { + id + contents { + __typename + ... on File { + id + } + } + children { + __typename + id + } + } + } + } +} +", +} +`; + +exports[`Extra fields adds id field if type has it 1`] = ` +Object { + "hash": "hash", + "isHook": false, + "isStaticQuery": false, + "name": "mockFileQuery", + "originalText": " + query mockFileQuery { + allDirectory { + nodes { + __typename + } + } + } + ", + "path": "mockFile", + "text": "query mockFileQuery { + allDirectory { + nodes { + id + __typename + } + } +} +", +} +`; + +exports[`Extra fields adds id field in the query of arbitrary depth 1`] = ` +Object { + "hash": "hash", + "isHook": false, + "isStaticQuery": false, + "name": "mockFileQuery", + "originalText": " + query mockFileQuery { + allDirectory { + nodes { + contents { + ... on Directory { + contents { + ... on Directory { + contents { + ... on File { + __typename + } + ... on Directory { + __typename + } + } + } + } + } + } + children { + children { + children { + __typename + } + ... on Directory { + contents { + ... on File { + __typename + } + } + } + } + } + } + } + } + ", + "path": "mockFile", + "text": "query mockFileQuery { + allDirectory { + nodes { + id + contents { + __typename + ... on Directory { + id + contents { + __typename + ... on Directory { + id + contents { + __typename + ... on File { + id + __typename + } + ... on Directory { + id + __typename + } + } + } + } + } + } + children { + __typename + id + children { + __typename + id + children { + id + __typename + } + ... on Directory { + id + contents { + __typename + ... on File { + id + __typename + } + } + } + } + } + } + } +} +", +} +`; + +exports[`Extra fields adds id field within fragment if type has it 1`] = ` +Object { + "hash": "hash", + "isHook": false, + "isStaticQuery": false, + "name": "mockFileQuery", + "originalText": " + query mockFileQuery { + allDirectory { + nodes { + ... Directory + } + } + } + fragment Directory on Directory { + __typename + } + + ", + "path": "mockFile", + "text": "fragment Directory on Directory { + id + __typename +} + +query mockFileQuery { + allDirectory { + nodes { + id + ...Directory + } + } +} +", +} +`; + +exports[`Extra fields adds id field within inline fragment if type has it 1`] = ` +Object { + "hash": "hash", + "isHook": false, + "isStaticQuery": false, + "name": "mockFileQuery", + "originalText": " + query mockFileQuery { + allDirectory { + nodes { + ... on Directory { + __typename + } + } + } + } + ", + "path": "mockFile", + "text": "query mockFileQuery { + allDirectory { + nodes { + id + ... on Directory { + id + __typename + } + } + } +} +", +} +`; + +exports[`Extra fields doesn't add __typename field to abstract types twice 1`] = ` +Object { + "hash": "hash", + "isHook": false, + "isStaticQuery": false, + "name": "mockFileQuery", + "originalText": " + query mockFileQuery { + allDirectory { + nodes { + contents { + __typename + ... on File { + id + } + ... FileOrDirectory + } + children { + __typename + ... Node + } + ... on Directory { + contents { + __typename + } + children { + __typename + } + } + ...DirectoryContents + } + } + } + + fragment DirectoryContents on Directory { + contents { + __typename + } + children { + __typename + } + } + + fragment FileOrDirectory on FileOrDirectory { + __typename + } + + fragment Node on Node { + __typename + } + ", + "path": "mockFile", + "text": "fragment FileOrDirectory on FileOrDirectory { + __typename +} + +fragment Node on Node { + id + __typename +} + +fragment DirectoryContents on Directory { + id + contents { + __typename + } + children { + id + __typename + } +} + +query mockFileQuery { + allDirectory { + nodes { + id + contents { + __typename + ... on File { + id + } + ...FileOrDirectory + } + children { + id + __typename + ...Node + } + ... on Directory { + id + contents { + __typename + } + children { + id + __typename + } + } + ...DirectoryContents + } + } +} +", +} +`; + +exports[`Extra fields doesn't add id field twice 1`] = ` +Object { + "hash": "hash", + "isHook": false, + "isStaticQuery": false, + "name": "mockFileQuery", + "originalText": " + query mockFileQuery { + allDirectory { + nodes { + id + contents { + ... on File { + id + } + ... on Directory { + id + } + } + children { + id + ... Node + } + ... on Directory { + id + children { + id + } + } + ...DirectoryContents + } + } + } + + fragment DirectoryContents on Directory { + contents { + ... on File { + id + } + ... on Directory { + id + } + } + children { + id + } + } + + fragment Node on Node { + id + } + ", + "path": "mockFile", + "text": "fragment Node on Node { + __typename + id +} + +fragment DirectoryContents on Directory { + id + contents { + __typename + ... on File { + id + } + ... on Directory { + id + } + } + children { + __typename + id + } +} + +query mockFileQuery { + allDirectory { + nodes { + id + contents { + __typename + ... on File { + id + } + ... on Directory { + id + } + } + children { + __typename + id + ...Node + } + ... on Directory { + id + children { + __typename + id + } + } + ...DirectoryContents + } + } +} +", +} +`; + exports[`actual compiling accepts identical fragment definitions 1`] = ` Map { "mockFile" => Object { @@ -26,6 +683,7 @@ Map { query mockFileQuery { allPostsJson { nodes { + id ...PostsJsonFragment } } @@ -56,6 +714,7 @@ Object { query mockFileQuery { allPostsJson { nodes { + id ...PostsJsonFragment } } @@ -89,6 +748,7 @@ Object { query mockFileQuery { allPostsJson { nodes { + id ...PostsJsonFragment } } @@ -235,12 +895,14 @@ Object { } fragment AnotherPostsJsonFragment on PostsJson { + id text } query mockFileQuery { allPostsJson { nodes { + id ...PostsJsonFragment } } @@ -278,6 +940,7 @@ Object { query mockFileQuery { allPostsJson { nodes { + id ...PostsJsonFragment } } diff --git a/packages/gatsby/src/query/__tests__/fixtures/query-compiler-schema.graphql b/packages/gatsby/src/query/__tests__/fixtures/query-compiler-schema.graphql index 354af6878afc3..661365468d7e1 100644 --- a/packages/gatsby/src/query/__tests__/fixtures/query-compiler-schema.graphql +++ b/packages/gatsby/src/query/__tests__/fixtures/query-compiler-schema.graphql @@ -24,8 +24,11 @@ type Directory implements Node { children: [Node!]! internal: Internal! absolutePath: String + contents: [FileOrDirectory!]! } +union FileOrDirectory = File | Directory + type DirectoryConnection { totalCount: Int! edges: [DirectoryEdge!]! diff --git a/packages/gatsby/src/query/__tests__/query-compiler.js b/packages/gatsby/src/query/__tests__/query-compiler.js index 5e64ef48e1239..ed8cec0566e3d 100644 --- a/packages/gatsby/src/query/__tests__/query-compiler.js +++ b/packages/gatsby/src/query/__tests__/query-compiler.js @@ -775,6 +775,333 @@ describe(`actual compiling`, () => { }) }) +describe(`Extra fields`, () => { + let schema + beforeAll(async () => { + const sdl = await fs.readFile( + path.join(__dirname, `./fixtures/query-compiler-schema.graphql`), + { encoding: `utf-8` } + ) + schema = buildSchema(sdl) + }) + + const transformQuery = queryString => { + const nodes = [createGatsbyDoc(`mockFile`, queryString)] + const errors = [] + const result = processQueries({ + schema, + parsedQueries: nodes, + addError: e => { + errors.push(e) + }, + }) + return [result, errors] + } + + it(`adds __typename field to abstract types`, async () => { + const [result, errors] = transformQuery(` + query mockFileQuery { + allDirectory { + nodes { + contents { + ... on File { + id + } + } + children { + id + } + } + } + } + `) + expect(errors).toEqual([]) + expect(result.get(`mockFile`)).toMatchSnapshot() + }) + + it(`adds __typename field to abstract types within inline fragments`, async () => { + const [result, errors] = transformQuery(` + query mockFileQuery { + allDirectory { + nodes { + ... on Directory { + contents { + ... on File { + id + } + } + children { + id + } + } + } + } + } + `) + expect(errors).toEqual([]) + expect(result.get(`mockFile`)).toMatchSnapshot() + }) + + it(`adds __typename field to abstract types within fragments`, async () => { + const [result, errors] = transformQuery(` + query mockFileQuery { + allDirectory { + nodes { + ...DirectoryContents + } + } + } + fragment DirectoryContents on Directory { + contents { + ... on File { + id + } + } + children { + id + } + } + `) + expect(errors).toEqual([]) + expect(result.get(`mockFile`)).toMatchSnapshot() + }) + + it(`adds __typename field to abstract types in the query of arbitrary depth`, async () => { + const [result, errors] = transformQuery(` + query mockFileQuery { + allDirectory { + nodes { + contents { + ... on Directory { + contents { + ... on Directory { + contents { + ... on Directory { + id + } + } + children { + id + } + } + } + } + } + children { + children { + children { + id + } + ... on Directory { + contents { + ... on File { + id + } + } + } + } + } + } + } + } + `) + expect(errors).toEqual([]) + expect(result.get(`mockFile`)).toMatchSnapshot() + }) + + it(`doesn't add __typename field to abstract types twice`, async () => { + const [result, errors] = transformQuery(` + query mockFileQuery { + allDirectory { + nodes { + contents { + __typename + ... on File { + id + } + ... FileOrDirectory + } + children { + __typename + ... Node + } + ... on Directory { + contents { + __typename + } + children { + __typename + } + } + ...DirectoryContents + } + } + } + + fragment DirectoryContents on Directory { + contents { + __typename + } + children { + __typename + } + } + + fragment FileOrDirectory on FileOrDirectory { + __typename + } + + fragment Node on Node { + __typename + } + `) + expect(errors).toEqual([]) + expect(result.get(`mockFile`)).toMatchSnapshot() + }) + + it(`adds id field if type has it`, () => { + const [result, errors] = transformQuery(` + query mockFileQuery { + allDirectory { + nodes { + __typename + } + } + } + `) + expect(errors).toEqual([]) + expect(result.get(`mockFile`)).toMatchSnapshot() + }) + + it(`adds id field within inline fragment if type has it`, () => { + const [result, errors] = transformQuery(` + query mockFileQuery { + allDirectory { + nodes { + ... on Directory { + __typename + } + } + } + } + `) + expect(errors).toEqual([]) + expect(result.get(`mockFile`)).toMatchSnapshot() + }) + + it(`adds id field within fragment if type has it`, () => { + const [result, errors] = transformQuery(` + query mockFileQuery { + allDirectory { + nodes { + ... Directory + } + } + } + fragment Directory on Directory { + __typename + } + + `) + expect(errors).toEqual([]) + expect(result.get(`mockFile`)).toMatchSnapshot() + }) + + it(`adds id field in the query of arbitrary depth`, () => { + const [result, errors] = transformQuery(` + query mockFileQuery { + allDirectory { + nodes { + contents { + ... on Directory { + contents { + ... on Directory { + contents { + ... on File { + __typename + } + ... on Directory { + __typename + } + } + } + } + } + } + children { + children { + children { + __typename + } + ... on Directory { + contents { + ... on File { + __typename + } + } + } + } + } + } + } + } + `) + expect(errors).toEqual([]) + expect(result.get(`mockFile`)).toMatchSnapshot() + }) + + it(`doesn't add id field twice`, async () => { + const [result, errors] = transformQuery(` + query mockFileQuery { + allDirectory { + nodes { + id + contents { + ... on File { + id + } + ... on Directory { + id + } + } + children { + id + ... Node + } + ... on Directory { + id + children { + id + } + } + ...DirectoryContents + } + } + } + + fragment DirectoryContents on Directory { + contents { + ... on File { + id + } + ... on Directory { + id + } + } + children { + id + } + } + + fragment Node on Node { + id + } + `) + expect(errors).toEqual([]) + expect(result.get(`mockFile`)).toMatchSnapshot() + }) +}) + const createGatsbyDoc = ( filePath, query, diff --git a/packages/gatsby/src/query/query-compiler.js b/packages/gatsby/src/query/query-compiler.js index db53e11a5a743..70061ab5d5ed2 100644 --- a/packages/gatsby/src/query/query-compiler.js +++ b/packages/gatsby/src/query/query-compiler.js @@ -15,6 +15,11 @@ const { validate, print, visit, + visitWithTypeInfo, + TypeInfo, + isAbstractType, + isObjectType, + isInterfaceType, Kind, FragmentsOnCompositeTypesRule, KnownTypeNamesRule, @@ -345,7 +350,7 @@ const processDefinitions = ({ continue } - const document = { + let document = { kind: Kind.DOCUMENT, definitions: Array.from(usedFragments.values()) .map(name => definitionsByName.get(name).def) @@ -385,6 +390,8 @@ const processDefinitions = ({ continue } + document = addExtraFields(document, schema) + const query = { name, text: print(document), @@ -465,3 +472,72 @@ const determineUsedFragmentsForDefinition = ( return { usedFragments, missingFragments } } } + +/** + * Automatically add: + * `__typename` field to abstract types (unions, interfaces) + * `id` field to all object/interface types having an id + * TODO: Remove this in v3.0 as it is a legacy from Relay compiler + */ +const addExtraFields = (document, schema) => { + const typeInfo = new TypeInfo(schema) + const contextStack = [] + + const transformer = visitWithTypeInfo(typeInfo, { + enter: { + [Kind.SELECTION_SET]: node => { + // Entering selection set: + // selection sets can be nested, so keeping their metadata stacked + contextStack.push({ hasTypename: false, hasId: false }) + }, + [Kind.FIELD]: node => { + // Entering a field of the current selection-set: + // mark which fields already exist in this selection set to avoid duplicates + const context = contextStack[contextStack.length - 1] + if (node.name.value === `__typename`) { + context.hasTypename = true + } + if (node.name.value === `id`) { + context.hasId = true + } + }, + }, + leave: { + [Kind.SELECTION_SET]: node => { + // Modify the selection-set AST on leave (add extra fields unless they already exist) + const context = contextStack.pop() + const parentType = typeInfo.getParentType() + const extraFields = [] + + // Adding __typename to unions and interfaces (if required) + if (!context.hasTypename && isAbstractType(parentType)) { + extraFields.push({ + kind: Kind.FIELD, + name: { kind: Kind.NAME, value: `__typename` }, + }) + } + if ( + !context.hasId && + (isObjectType(parentType) || isInterfaceType(parentType)) && + hasIdField(parentType) + ) { + extraFields.push({ + kind: Kind.FIELD, + name: { kind: Kind.NAME, value: `id` }, + }) + } + return extraFields.length > 0 + ? { ...node, selections: [...extraFields, ...node.selections] } + : undefined + }, + }, + }) + + return visit(document, transformer) +} + +const hasIdField = type => { + const idField = type.getFields()[`id`] + const fieldType = idField ? String(idField.type) : `` + return (idField && fieldType === `ID`) || fieldType === `ID!` +} From 5abffd5bb2822f68c05a1fd36a9688e551ff2de2 Mon Sep 17 00:00:00 2001 From: Vladimir Razuvaev Date: Fri, 24 Jan 2020 20:58:05 +0200 Subject: [PATCH 2/2] Minor fix --- packages/gatsby/src/query/query-compiler.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/gatsby/src/query/query-compiler.js b/packages/gatsby/src/query/query-compiler.js index 70061ab5d5ed2..216e8a87936ed 100644 --- a/packages/gatsby/src/query/query-compiler.js +++ b/packages/gatsby/src/query/query-compiler.js @@ -539,5 +539,5 @@ const addExtraFields = (document, schema) => { const hasIdField = type => { const idField = type.getFields()[`id`] const fieldType = idField ? String(idField.type) : `` - return (idField && fieldType === `ID`) || fieldType === `ID!` + return fieldType === `ID` || fieldType === `ID!` }