From 891a8c72b89af39f17b402485cea642946375278 Mon Sep 17 00:00:00 2001 From: Ricardo Canastro Date: Thu, 22 Jun 2023 09:01:07 +0100 Subject: [PATCH] Fix inserts into lists of unions (#1103) Co-authored-by: jycouet Co-authored-by: Alec Aivazis --- .changeset/mean-pears-flash.md | 5 + e2e/_api/graphql.mjs | 30 ++++ e2e/_api/schema.graphql | 15 ++ e2e/kit/houdini.config.js | 3 + e2e/kit/src/lib/utils/routes.ts | 2 + e2e/kit/src/routes/union-insert/+page.gql | 12 ++ e2e/kit/src/routes/union-insert/+page.svelte | 31 ++++ e2e/kit/src/routes/union-insert/spec.ts | 27 ++++ .../codegen/generators/artifacts/selection.ts | 38 ++++- .../artifacts/tests/selection.test.ts | 145 ++++++++++++++++++ 10 files changed, 303 insertions(+), 5 deletions(-) create mode 100644 .changeset/mean-pears-flash.md create mode 100644 e2e/kit/src/routes/union-insert/+page.gql create mode 100644 e2e/kit/src/routes/union-insert/+page.svelte create mode 100644 e2e/kit/src/routes/union-insert/spec.ts diff --git a/.changeset/mean-pears-flash.md b/.changeset/mean-pears-flash.md new file mode 100644 index 000000000..c2f521421 --- /dev/null +++ b/.changeset/mean-pears-flash.md @@ -0,0 +1,5 @@ +--- +'houdini': patch +--- + +fix: insert in list works for list of union diff --git a/e2e/_api/graphql.mjs b/e2e/_api/graphql.mjs index 0fc056e53..653b51a8b 100644 --- a/e2e/_api/graphql.mjs +++ b/e2e/_api/graphql.mjs @@ -84,6 +84,9 @@ let dataRentedBooks = [ { userId: '1', bookId: 1, rate: 9 }, ] +const listA = [] +const listB = [] + const userSnapshots = {} function getUserSnapshot(snapshot) { if (!userSnapshots[snapshot]) { @@ -112,6 +115,23 @@ export const resolvers = { hello: () => { return 'Hello World! // From Houdini!' }, + aOrB: () => { + const toRet = [] + + toRet.push( + ...listA.map((a) => { + return { __typename: 'A', ...a } + }) + ) + + toRet.push( + ...listB.map((b) => { + return { __typename: 'B', ...b } + }) + ) + + return toRet + }, usersList: (_, args) => { return [...getUserSnapshot(args.snapshot)].splice(args.offset || 0, args.limit) }, @@ -429,6 +449,16 @@ export const resolvers = { throw new GraphQLError('RentedBook not found', { code: 403 }) }, + createA: (_, args) => { + const a = { id: listA.length + 1, a: args.a } + listA.push(a) + return a + }, + createB: (_, args) => { + const b = { id: listB.length + 1, b: args.b } + listB.push(b) + return b + }, }, DateTime: new GraphQLScalarType({ diff --git a/e2e/_api/schema.graphql b/e2e/_api/schema.graphql index 2c20874b8..0fb7f47d1 100644 --- a/e2e/_api/schema.graphql +++ b/e2e/_api/schema.graphql @@ -49,6 +49,8 @@ type Mutation { deleteLibrary(library: ID!): Library! deleteBook(book: ID!, delay: Int, force: ForceReturn): Book updateRentedBook(userId: String!, bookId: Int!, rate: Int!): RentedBook + createA(a: String!): A! + createB(b: String!): B! } interface Node { @@ -66,7 +68,10 @@ input UserNameFilter { name: String! } +union UnionAorB = A | B + type Query { + aOrB: [UnionAorB!]! avgYearsBirthDate: Float! node(id: ID!): Node user(id: ID!, snapshot: String!, tmp: Boolean, delay: Int, forceNullDate: Boolean): User! @@ -178,6 +183,16 @@ type RentedBook { rate: Int! } +type A { + id: ID! + a: String! +} + +type B { + id: ID! + b: String! +} + union UserNodesResult = UserNodes | Message1 union UserResult = User | Message1 diff --git a/e2e/kit/houdini.config.js b/e2e/kit/houdini.config.js index bf51c338e..e23c23966 100644 --- a/e2e/kit/houdini.config.js +++ b/e2e/kit/houdini.config.js @@ -26,6 +26,9 @@ const config = { types: { RentedBook: { keys: ['userId', 'bookId'] + }, + UnionAorB: { + keys: [] } }, diff --git a/e2e/kit/src/lib/utils/routes.ts b/e2e/kit/src/lib/utils/routes.ts index 91c37aae6..9ce6b7b63 100644 --- a/e2e/kit/src/lib/utils/routes.ts +++ b/e2e/kit/src/lib/utils/routes.ts @@ -50,6 +50,8 @@ export const routes = { Stores_Pagination_query_forward_cursor: '/stores/pagination/query/forward-cursor', Stores_Directives: '/stores/directives', + union_insert: 'union-insert', + Plugin_query_simple: '/plugin/query/simple', Plugin_query_variable_1: '/plugin/query/variable-1', Plugin_query_variable_2: '/plugin/query/variable-2', diff --git a/e2e/kit/src/routes/union-insert/+page.gql b/e2e/kit/src/routes/union-insert/+page.gql new file mode 100644 index 000000000..9ba683f16 --- /dev/null +++ b/e2e/kit/src/routes/union-insert/+page.gql @@ -0,0 +1,12 @@ +query AorB { + aOrB @list(name: "All_AorB") { + ... on A { + id + a + } + ... on B { + id + b + } + } +} diff --git a/e2e/kit/src/routes/union-insert/+page.svelte b/e2e/kit/src/routes/union-insert/+page.svelte new file mode 100644 index 000000000..e6e2ab0e2 --- /dev/null +++ b/e2e/kit/src/routes/union-insert/+page.svelte @@ -0,0 +1,31 @@ + + + + + +
+ 
{JSON.stringify($AorB?.data, null, 0)}
+
diff --git a/e2e/kit/src/routes/union-insert/spec.ts b/e2e/kit/src/routes/union-insert/spec.ts new file mode 100644 index 000000000..87a20283c --- /dev/null +++ b/e2e/kit/src/routes/union-insert/spec.ts @@ -0,0 +1,27 @@ +import { test } from '@playwright/test'; +import { routes } from '../../lib/utils/routes.js'; +import { expect_1_gql, expect_to_be, goto } from '../../lib/utils/testsHelper.js'; + +test.describe('union-result', () => { + test('Get two stores and not resetting', async ({ page }) => { + await goto(page, routes.union_insert); + + // When we arrive on the page, we expect to see null in the result div + await expect_to_be(page, '{"aOrB":[]}'); + + // we click on the button to getAllUsers + await expect_1_gql(page, 'button[id="addA"]'); + + // expect 1 element in the array + await expect_to_be(page, '{"aOrB":[{"id":"1","a":"MyA","__typename":"A"}]}'); + + // we click on the button to getAllUsers + await expect_1_gql(page, 'button[id="addB"]'); + + // expect 2 elements in the array + await expect_to_be( + page, + '{"aOrB":[{"id":"1","a":"MyA","__typename":"A"},{"id":"1","b":"MyB","__typename":"B"}]}' + ); + }); +}); diff --git a/packages/houdini/src/codegen/generators/artifacts/selection.ts b/packages/houdini/src/codegen/generators/artifacts/selection.ts index 592e8afd3..116c99924 100644 --- a/packages/houdini/src/codegen/generators/artifacts/selection.ts +++ b/packages/houdini/src/codegen/generators/artifacts/selection.ts @@ -121,22 +121,50 @@ function prepareSelection({ // look up the field const type = config.schema.getType(rootType) as graphql.GraphQLObjectType if (!type) { - throw new HoudiniError({ filepath, message: 'Could not find type' }) + throw new HoudiniError({ + filepath, + message: 'Could not find type. Looking for ' + JSON.stringify(rootType), + }) } const attributeName = field.alias?.value || field.name.value - // if we are looking at __typename, its a string (not defined in the schema) - let fieldType: graphql.GraphQLType + let fieldType: graphql.GraphQLType | null = null let nullable = false + + // if we are looking at __typename, its a string (not defined in the schema) if (field.name.value === '__typename') { fieldType = config.schema.getType('String')! - } else { + } + // if the type is something that has definite fields then we can look up the field + // type in the schema + else if ('getFields' in type) { let typeRef = type.getFields()[field.name.value].type fieldType = getRootType(typeRef) nullable = !graphql.isNonNullType(typeRef) } + // if we are looking at an abstract type that doesn't have well-defined fields (ie a union) + // then we are safe to look at any possible type (i think) + else if (graphql.isAbstractType(type)) { + for (const possible of config.schema.getPossibleTypes(type)) { + if (graphql.isObjectType(possible)) { + if (possible.getFields()[field.name.value]) { + fieldType = possible.getFields()[field.name.value].type + nullable = !graphql.isNonNullType(fieldType) + break + } + } + } + } + + // make sure we identified a type + if (!fieldType) { + throw { + message: "Could not identify field's type", + description: `Missing definition for ${field.name.value} in ${type.name}`, + } + } - const typeName = fieldType.toString() + const typeName = (getRootType(fieldType) as graphql.GraphQLObjectType).name // make sure we include the attribute in the path const pathSoFar = path.concat(attributeName) diff --git a/packages/houdini/src/codegen/generators/artifacts/tests/selection.test.ts b/packages/houdini/src/codegen/generators/artifacts/tests/selection.test.ts index e8acbe2ce..b2eb0facf 100644 --- a/packages/houdini/src/codegen/generators/artifacts/tests/selection.test.ts +++ b/packages/houdini/src/codegen/generators/artifacts/tests/selection.test.ts @@ -95,6 +95,151 @@ test('fragments of unions inject correctly', function () { `) }) +test('list of fragment unions', async function () { + // the config to use in tests + const config = testConfig() + const docs = [ + mockCollectedDoc( + `query Entities { + entities @list(name: "list_entities") { + ... on User { + name + } + ... on Cat { + name + } + } + }` + ), + mockCollectedDoc( + `mutation CatMutation { + catMutation { + ...list_entities_insert + } + }` + ), + ] + + // execute the generator + await runPipeline(config, docs) + + expect(docs[0]).toMatchInlineSnapshot(` + export default { + "name": "Entities", + "kind": "HoudiniQuery", + "hash": "d7db6b92dac1893d8640c0f9d3535d1f75cdb1017cf4c8960c0bdaefbe119229", + + "raw": \`query Entities { + entities { + ... on User { + name + id + } + ... on Cat { + name + id + } + __typename + } + } + \`, + + "rootType": "Query", + + "selection": { + "fields": { + "entities": { + "type": "Entity", + "keyRaw": "entities", + + "directives": [{ + "name": "list", + + "arguments": { + "name": { + "kind": "StringValue", + "value": "list_entities" + } + } + }], + + "list": { + "name": "list_entities", + "connection": false, + "type": "Entity" + }, + + "selection": { + "abstractFields": { + "fields": { + "User": { + "name": { + "type": "String", + "keyRaw": "name", + "visible": true + }, + + "id": { + "type": "ID", + "keyRaw": "id", + "visible": true + }, + + "__typename": { + "type": "String", + "keyRaw": "__typename", + "visible": true + } + }, + + "Cat": { + "name": { + "type": "String", + "keyRaw": "name", + "visible": true + }, + + "id": { + "type": "ID", + "keyRaw": "id", + "visible": true + }, + + "__typename": { + "type": "String", + "keyRaw": "__typename", + "visible": true + } + } + }, + + "typeMap": {} + }, + + "fields": { + "__typename": { + "type": "String", + "keyRaw": "__typename", + "visible": true + } + } + }, + + "abstract": true, + "visible": true + } + } + }, + + "pluginData": {}, + "policy": "CacheOrNetwork", + "partial": false + }; + + "HoudiniHash=d7db6b92dac1893d8640c0f9d3535d1f75cdb1017cf4c8960c0bdaefbe119229"; + `) +}) + test('fragments in lists', async function () { // the config to use in tests const config = testConfig()