From 21ef04bffce6e22a49e0294e1618a2a9f879f43d Mon Sep 17 00:00:00 2001 From: Seppe Dekeyser Date: Mon, 5 Feb 2024 21:45:03 +0100 Subject: [PATCH] Allow for required arguments on paginated fragments (#1253) --- .changeset/neat-cameras-pay.md | 6 + e2e/_api/graphql.mjs | 9 +- e2e/_api/schema.graphql | 1 + e2e/kit/src/lib/utils/routes.ts | 1 + .../fragment/required-arguments/+page.svelte | 42 +++ .../fragment/required-arguments/spec.ts | 48 +++ .../src/plugin/artifactData.test.ts | 3 +- .../src/runtime/stores/query.ts | 11 +- .../codegen/generators/artifacts/inputs.ts | 12 +- .../artifacts/tests/artifacts.test.ts | 332 +++++++++++++++++- .../artifacts/tests/pagination.test.ts | 30 +- .../generators/typescript/typescript.test.ts | 10 +- .../transforms/fragmentVariables.test.ts | 13 +- .../codegen/transforms/fragmentVariables.ts | 9 +- .../src/codegen/transforms/paginate.test.ts | 90 ++++- .../src/codegen/transforms/paginate.ts | 98 ++++-- .../src/codegen/validators/typeCheck.test.ts | 4 +- .../src/codegen/validators/typeCheck.ts | 21 -- packages/houdini/src/runtime/cache/cache.ts | 21 +- .../src/runtime/cache/tests/variables.test.ts | 4 +- packages/houdini/src/runtime/lib/types.ts | 1 + packages/houdini/src/test/index.ts | 1 + 22 files changed, 658 insertions(+), 109 deletions(-) create mode 100644 .changeset/neat-cameras-pay.md create mode 100644 e2e/kit/src/routes/pagination/fragment/required-arguments/+page.svelte create mode 100644 e2e/kit/src/routes/pagination/fragment/required-arguments/spec.ts diff --git a/.changeset/neat-cameras-pay.md b/.changeset/neat-cameras-pay.md new file mode 100644 index 000000000..877120ae5 --- /dev/null +++ b/.changeset/neat-cameras-pay.md @@ -0,0 +1,6 @@ +--- +'houdini-svelte': patch +'houdini': patch +--- + +Add support for required arguments in paginated fragments diff --git a/e2e/_api/graphql.mjs b/e2e/_api/graphql.mjs index 9f61c87cb..09cfe7a67 100644 --- a/e2e/_api/graphql.mjs +++ b/e2e/_api/graphql.mjs @@ -260,6 +260,9 @@ export const resolvers = { usersConnection: (user, args) => { return connectionFromArray(getUserSnapshot(user.snapshot), args) }, + usersConnectionSnapshot: (user, args) => { + return connectionFromArray(getUserSnapshot(args.snapshot), args) + }, userSearch: (_, args) => { const allUsers = [...getUserSnapshot(args.snapshot)] @@ -270,11 +273,11 @@ export const resolvers = { enumValue: () => 'Value1', testField: (user, args) => { if (args.someParam) { - return "Hello world"; + return 'Hello world' } - return null; - } + return null + }, }, Mutation: { diff --git a/e2e/_api/schema.graphql b/e2e/_api/schema.graphql index f64389b86..c0da52bd7 100644 --- a/e2e/_api/schema.graphql +++ b/e2e/_api/schema.graphql @@ -105,6 +105,7 @@ type User implements Node { friendsConnection(after: String, before: String, first: Int, last: Int): UserConnection! "This is the same list as what's used globally. its here to tests fragments" usersConnection(after: String, before: String, first: Int, last: Int): UserConnection! + usersConnectionSnapshot(after: String, before: String, first: Int, last: Int, snapshot: String!): UserConnection! "This is the same list as what's used globally. its here to tests fragments" userSearch(filter: UserNameFilter!, snapshot: String!): [User!]! friendsList(limit: Int, offset: Int): [User!]! diff --git a/e2e/kit/src/lib/utils/routes.ts b/e2e/kit/src/lib/utils/routes.ts index 2d14678b9..66b59185a 100644 --- a/e2e/kit/src/lib/utils/routes.ts +++ b/e2e/kit/src/lib/utils/routes.ts @@ -98,6 +98,7 @@ export const routes = { Pagination_fragment_backwards_cursor: '/pagination/fragment/backwards-cursor', Pagination_fragment_bidirectional_cursor: '/pagination/fragment/bidirectional-cursor', Pagination_fragment_offset: '/pagination/fragment/offset', + Pagination_fragment_required_arguments: '/pagination/fragment/required-arguments', nested_argument_fragments: '/nested-argument-fragments', nested_argument_fragments_masking: '/nested-argument-fragments-masking', diff --git a/e2e/kit/src/routes/pagination/fragment/required-arguments/+page.svelte b/e2e/kit/src/routes/pagination/fragment/required-arguments/+page.svelte new file mode 100644 index 000000000..7e3e5979e --- /dev/null +++ b/e2e/kit/src/routes/pagination/fragment/required-arguments/+page.svelte @@ -0,0 +1,42 @@ + + +
+ {$fragmentResult.data?.usersConnectionSnapshot.edges.map(({ node }) => node?.name).join(', ')} +
+ +
+ {JSON.stringify($fragmentResult.pageInfo)} +
+ + diff --git a/e2e/kit/src/routes/pagination/fragment/required-arguments/spec.ts b/e2e/kit/src/routes/pagination/fragment/required-arguments/spec.ts new file mode 100644 index 000000000..68ea37ca4 --- /dev/null +++ b/e2e/kit/src/routes/pagination/fragment/required-arguments/spec.ts @@ -0,0 +1,48 @@ +import { test } from '@playwright/test'; +import { routes } from '../../../../lib/utils/routes.js'; +import { + expectToContain, + expect_0_gql, + expect_1_gql, + expect_to_be, + goto +} from '../../../../lib/utils/testsHelper.js'; + +test.describe('forwards cursor paginatedFragment with required arguments', () => { + test('loadNextPage', async ({ page }) => { + await goto(page, routes.Pagination_fragment_required_arguments); + await expect_to_be(page, 'Bruce Willis, Samuel Jackson'); + + // wait for the api response + await expect_1_gql(page, 'button[id=next]'); + + // make sure we got the new content + await expect_to_be(page, 'Bruce Willis, Samuel Jackson, Morgan Freeman, Tom Hanks'); + }); + + test('page info tracks connection state', async ({ page }) => { + await goto(page, routes.Pagination_fragment_required_arguments); + + const data = [ + 'Bruce Willis, Samuel Jackson, Morgan Freeman, Tom Hanks', + 'Bruce Willis, Samuel Jackson, Morgan Freeman, Tom Hanks, Will Smith, Harrison Ford', + 'Bruce Willis, Samuel Jackson, Morgan Freeman, Tom Hanks, Will Smith, Harrison Ford, Eddie Murphy, Clint Eastwood' + ]; + + // load the next 3 pages + for (let i = 0; i < 3; i++) { + // wait for the request to resolve + await expect_1_gql(page, 'button[id=next]'); + + // check the page info + await expect_to_be(page, data[i]); + } + + // make sure we have all of the data loaded + await expect_to_be(page, data[2]); + + await expectToContain(page, `"hasNextPage":false`); + + await expect_0_gql(page, 'button[id=next]'); + }); +}); diff --git a/packages/houdini-svelte/src/plugin/artifactData.test.ts b/packages/houdini-svelte/src/plugin/artifactData.test.ts index 1a4ca6946..327d9371b 100644 --- a/packages/houdini-svelte/src/plugin/artifactData.test.ts +++ b/packages/houdini-svelte/src/plugin/artifactData.test.ts @@ -131,7 +131,8 @@ describe('load', () => { "id": "ID" }, - "types": {} + "types": {}, + "defaults": {} }, "policy": "CacheOrNetwork", diff --git a/packages/houdini-svelte/src/runtime/stores/query.ts b/packages/houdini-svelte/src/runtime/stores/query.ts index 117ef9023..34267b311 100644 --- a/packages/houdini-svelte/src/runtime/stores/query.ts +++ b/packages/houdini-svelte/src/runtime/stores/query.ts @@ -161,13 +161,20 @@ This will result in duplicate queries. If you are trying to ensure there is alwa // we might not want to actually wait for the fetch to resolve const fakeAwait = clientStarted && isBrowser && !need_to_block + // spreading the default variables frist so that if the user provides one of these params themselves, + // those params get overwritten with the correct value + const usedVariables = { + ...this.artifact.input?.defaults, + ...params.variables, + } + // we want to try to load cached data before we potentially fake the await // this makes sure that the UI feels snappy as we click between cached pages // (no loaders) if (policy !== CachePolicy.NetworkOnly && fakeAwait) { await this.observer.send({ fetch: context.fetch, - variables: params.variables, + variables: usedVariables, metadata: params.metadata, session: context.session, policy: CachePolicy.CacheOnly, @@ -181,7 +188,7 @@ This will result in duplicate queries. If you are trying to ensure there is alwa // since CacheOrNetwork behaves the same as CacheAndNetwork const request = this.observer.send({ fetch: context.fetch, - variables: params.variables, + variables: usedVariables, metadata: params.metadata, session: context.session, policy: policy, diff --git a/packages/houdini/src/codegen/generators/artifacts/inputs.ts b/packages/houdini/src/codegen/generators/artifacts/inputs.ts index ba2aa93be..142fa156b 100644 --- a/packages/houdini/src/codegen/generators/artifacts/inputs.ts +++ b/packages/houdini/src/codegen/generators/artifacts/inputs.ts @@ -1,12 +1,10 @@ import * as graphql from 'graphql' -import * as recast from 'recast' import { unwrapType } from '../../../lib' import type { Config } from '../../../lib/config' +import { variableValue } from '../../../runtime/cache/cache' import type { InputObject } from '../../../runtime/lib/types' -const AST = recast.types.builders - export function inputObject( config: Config, inputs: readonly graphql.VariableDefinitionNode[] @@ -27,6 +25,14 @@ export function inputObject( } }, {}), types: {}, + defaults: inputs.reduce((fields, input) => { + return { + ...fields, + [input.variable.name.value]: input.defaultValue + ? variableValue(input.defaultValue, {}) + : undefined, + } + }, {}), } // walk through every type referenced and add it to the list diff --git a/packages/houdini/src/codegen/generators/artifacts/tests/artifacts.test.ts b/packages/houdini/src/codegen/generators/artifacts/tests/artifacts.test.ts index 3da79cc5d..170f8dc89 100644 --- a/packages/houdini/src/codegen/generators/artifacts/tests/artifacts.test.ts +++ b/packages/houdini/src/codegen/generators/artifacts/tests/artifacts.test.ts @@ -400,7 +400,8 @@ test('variables only used by internal directives are scrubbed', async function ( "parentID": "ID" }, - "types": {} + "types": {}, + "defaults": {} }, "policy": "CacheOrNetwork", @@ -592,7 +593,8 @@ test('interface to interface inline fragment', async function () { "id": "ID" }, - "types": {} + "types": {}, + "defaults": {} }, "policy": "CacheOrNetwork", @@ -842,7 +844,11 @@ test('paginate over unions', async function () { "before": "String" }, - "types": {} + "types": {}, + + "defaults": { + "first": 10 + } }, "policy": "CacheOrNetwork", @@ -3876,7 +3882,8 @@ describe('mutation artifacts', function () { "value": "String" }, - "types": {} + "types": {}, + "defaults": {} }, "policy": "CacheOrNetwork", @@ -4329,7 +4336,11 @@ describe('mutation artifacts', function () { "before": "String" }, - "types": {} + "types": {}, + + "defaults": { + "first": 10 + } }, "policy": "CacheOrNetwork", @@ -4574,7 +4585,11 @@ describe('mutation artifacts', function () { "before": "String" }, - "types": {} + "types": {}, + + "defaults": { + "first": 10 + } }, "policy": "CacheOrNetwork", @@ -4696,7 +4711,8 @@ describe('mutation artifacts', function () { "value": "String" }, - "types": {} + "types": {}, + "defaults": {} }, "policy": "CacheOrNetwork", @@ -4819,7 +4835,8 @@ describe('mutation artifacts', function () { "value": "String" }, - "types": {} + "types": {}, + "defaults": {} }, "policy": "CacheOrNetwork", @@ -5039,7 +5056,9 @@ test('operation inputs', async function () { "recursive": "UserFilter", "enum": "MyEnum" } - } + }, + + "defaults": {} }, "policy": "CacheOrNetwork", @@ -6023,7 +6042,8 @@ test('client nullability', async function () { "id": "ID" }, - "types": {} + "types": {}, + "defaults": {} }, "policy": "CacheOrNetwork", @@ -6672,3 +6692,295 @@ test('nested abstract fragments', async function () { "HoudiniHash=72091e952cf94a18193cee6cda1caf256f6439e4e202b319d876e9b3365ac319"; `) }) + +describe('default arguments', function () { + test('adds default values to the artifact', async function () { + // the config to use in tests + const config = testConfig() + // the documents to test + const docs: Document[] = [ + mockCollectedDoc(` + query UserFriends($count: Int = 10, $search: String = "bob") { + user { + friendsByOffset(offset: $count, filter: $search) { + name + } + } + } + `), + ] + + // execute the generator + await runPipeline(config, docs) + + // load the contents of the file + expect(docs[0]).toMatchInlineSnapshot(` + export default { + "name": "UserFriends", + "kind": "HoudiniQuery", + "hash": "50713a85f40c418e37c1eb92eef9dc136b8916e78b4126a902bde2956a642db3", + + "raw": \`query UserFriends($count: Int = 10, $search: String = "bob") { + user { + friendsByOffset(offset: $count, filter: $search) { + name + id + } + id + } + } + \`, + + "rootType": "Query", + + "selection": { + "fields": { + "user": { + "type": "User", + "keyRaw": "user", + + "selection": { + "fields": { + "friendsByOffset": { + "type": "User", + "keyRaw": "friendsByOffset(filter: $search, offset: $count)", + + "selection": { + "fields": { + "name": { + "type": "String", + "keyRaw": "name", + "visible": true + }, + + "id": { + "type": "ID", + "keyRaw": "id", + "visible": true + } + } + }, + + "visible": true + }, + + "id": { + "type": "ID", + "keyRaw": "id", + "visible": true + } + } + }, + + "visible": true + } + } + }, + + "pluginData": {}, + + "input": { + "fields": { + "count": "Int", + "search": "String" + }, + + "types": {}, + + "defaults": { + "count": 10, + "search": "bob" + } + }, + + "policy": "CacheOrNetwork", + "partial": false + }; + + "HoudiniHash=d1f65a0e526e297d58858015a806c475bdca0a1b153f3ee839712ec7ee6190ff"; + `) + }) + + test('handles base scalars correctly', async function () { + // the config to use in tests + const config = testConfig() + // the documents to test + const docs: Document[] = [ + mockCollectedDoc(` + query ListUsers($bool: Boolean = true, $int: Int = 5, $float: Float = 3.14, $string: String = "hello world") { + users(boolValue: $bool, intValue: $int, floatValue: $float, stringValue: $string) { + name + } + } + `), + ] + + // execute the generator + await runPipeline(config, docs) + + // load the contents of the file + expect(docs[0]).toMatchInlineSnapshot(` + export default { + "name": "ListUsers", + "kind": "HoudiniQuery", + "hash": "8e997ca35d0030fcfbb888a740c20240530c85149e04b931e7d34d489d8be553", + + "raw": \`query ListUsers($bool: Boolean = true, $int: Int = 5, $float: Float = 3.14, $string: String = "hello world") { + users( + boolValue: $bool + intValue: $int + floatValue: $float + stringValue: $string + ) { + name + id + } + } + \`, + + "rootType": "Query", + + "selection": { + "fields": { + "users": { + "type": "User", + "keyRaw": "users(boolValue: $bool, floatValue: $float, intValue: $int, stringValue: $string)", + + "selection": { + "fields": { + "name": { + "type": "String", + "keyRaw": "name", + "visible": true + }, + + "id": { + "type": "ID", + "keyRaw": "id", + "visible": true + } + } + }, + + "visible": true + } + } + }, + + "pluginData": {}, + + "input": { + "fields": { + "bool": "Boolean", + "int": "Int", + "float": "Float", + "string": "String" + }, + + "types": {}, + + "defaults": { + "bool": true, + "int": 5, + "float": 3.14, + "string": "hello world" + } + }, + + "policy": "CacheOrNetwork", + "partial": false + }; + + "HoudiniHash=f8edbf4199a63a56d214cd5c845d90310052da75c45f3f3f5abf5f5cdb707a3e"; + `) + }) + + test('handles complex default arguments', async function () { + // the config to use in tests + const config = testConfig() + // the documents to test + const docs: Document[] = [ + mockCollectedDoc(` + query FindUser($filter: UserFilter = { name: "bob" }) { + usersByOffset(offset: 5, filter: $filter) { + name + } + } + `), + ] + + // execute the generator + await runPipeline(config, docs) + + // load the contents of the file + expect(docs[0]).toMatchInlineSnapshot(` + export default { + "name": "FindUser", + "kind": "HoudiniQuery", + "hash": "178720d2fc874e6b58c920f655d292a59a6de314ed70ea9eee335b1ad3fb1755", + + "raw": \`query FindUser($filter: UserFilter = {name: "bob"}) { + usersByOffset(offset: 5, filter: $filter) { + name + id + } + } + \`, + + "rootType": "Query", + + "selection": { + "fields": { + "usersByOffset": { + "type": "User", + "keyRaw": "usersByOffset(filter: $filter, offset: 5)", + + "selection": { + "fields": { + "name": { + "type": "String", + "keyRaw": "name", + "visible": true + }, + + "id": { + "type": "ID", + "keyRaw": "id", + "visible": true + } + } + }, + + "visible": true + } + } + }, + + "pluginData": {}, + + "input": { + "fields": { + "filter": "UserFilter" + }, + + "types": { + "UserFilter": { + "name": "String" + } + }, + + "defaults": { + "filter": { + "name": "bob" + } + } + }, + + "policy": "CacheOrNetwork", + "partial": false + }; + + "HoudiniHash=4b2886d3afb40660837727c266b5667c81698e3bbf240ec47270a262842f61d8"; + `) + }) +}) diff --git a/packages/houdini/src/codegen/generators/artifacts/tests/pagination.test.ts b/packages/houdini/src/codegen/generators/artifacts/tests/pagination.test.ts index c03aef6ff..6ac09d97c 100644 --- a/packages/houdini/src/codegen/generators/artifacts/tests/pagination.test.ts +++ b/packages/houdini/src/codegen/generators/artifacts/tests/pagination.test.ts @@ -199,7 +199,11 @@ test('pagination arguments stripped from key', async function () { "before": "String" }, - "types": {} + "types": {}, + + "defaults": { + "first": 10 + } } }; @@ -406,7 +410,11 @@ test('pagination arguments stays in key as it s a SinglePage Mode', async functi "before": "String" }, - "types": {} + "types": {}, + + "defaults": { + "first": 10 + } } }; @@ -506,7 +514,11 @@ test('offset based pagination marks appropriate field', async function () { "offset": "Int" }, - "types": {} + "types": {}, + + "defaults": { + "limit": 10 + } } }; @@ -768,7 +780,11 @@ test('cursor as scalar gets the right pagination query argument types', async fu "before": "Cursor" }, - "types": {} + "types": {}, + + "defaults": { + "first": 10 + } }, "policy": "CacheOrNetwork", @@ -1132,7 +1148,11 @@ test("sibling aliases don't get marked", async function () { "before": "String" }, - "types": {} + "types": {}, + + "defaults": { + "first": 10 + } } }; diff --git a/packages/houdini/src/codegen/generators/typescript/typescript.test.ts b/packages/houdini/src/codegen/generators/typescript/typescript.test.ts index 5d006cb80..d97b589ba 100644 --- a/packages/houdini/src/codegen/generators/typescript/typescript.test.ts +++ b/packages/houdini/src/codegen/generators/typescript/typescript.test.ts @@ -262,6 +262,7 @@ describe('typescript', function () { "name": "ID"; }; "types": {}; + "defaults": {}; }; }; `) @@ -351,6 +352,7 @@ describe('typescript', function () { "name": "ID"; }; "types": {}; + "defaults": {}; }; }; `) @@ -862,6 +864,7 @@ describe('typescript', function () { "enum": "MyEnum"; }; "types": {}; + "defaults": {}; }; "policy": "CacheOrNetwork"; "partial": false; @@ -1124,6 +1127,7 @@ describe('typescript', function () { "enum": "MyEnum"; }; }; + "defaults": {}; }; }; `) @@ -1351,6 +1355,7 @@ describe('typescript', function () { "enum": "MyEnum"; }; }; + "defaults": {}; }; "policy": "CacheOrNetwork"; "partial": false; @@ -2283,6 +2288,7 @@ describe('typescript', function () { "date": "DateTime"; }; "types": {}; + "defaults": {}; }; "policy": "CacheOrNetwork"; "partial": false; @@ -2686,6 +2692,7 @@ describe('typescript', function () { "enum": "MyEnum"; }; }; + "defaults": {}; }; }; `) @@ -2801,7 +2808,8 @@ describe('typescript', function () { "id": "ID" }, - "types": {} + "types": {}, + "defaults": {} }, "policy": "CacheOrNetwork", diff --git a/packages/houdini/src/codegen/transforms/fragmentVariables.test.ts b/packages/houdini/src/codegen/transforms/fragmentVariables.test.ts index 95d892b54..e2224431a 100644 --- a/packages/houdini/src/codegen/transforms/fragmentVariables.test.ts +++ b/packages/houdini/src/codegen/transforms/fragmentVariables.test.ts @@ -518,7 +518,8 @@ test('thread query variables to inner fragments', async function () { "name": "String" }, - "types": {} + "types": {}, + "defaults": {} }, "policy": "CacheOrNetwork", @@ -1022,7 +1023,12 @@ test('persists fragment variables in artifact', async function () { "cool": "Boolean" }, - "types": {} + "types": {}, + + "defaults": { + "name": "Hello", + "cool": true + } } }; @@ -1196,7 +1202,8 @@ test('variables referenced deeply in objects', async function () { "name": "String" }, - "types": {} + "types": {}, + "defaults": {} } }; diff --git a/packages/houdini/src/codegen/transforms/fragmentVariables.ts b/packages/houdini/src/codegen/transforms/fragmentVariables.ts index 3bd02cb30..221b0ff7f 100644 --- a/packages/houdini/src/codegen/transforms/fragmentVariables.ts +++ b/packages/houdini/src/codegen/transforms/fragmentVariables.ts @@ -122,7 +122,7 @@ function inlineFragmentArgs({ * ``` * * It replaced every VariableNode with its explicit value. If there is not argument provided, - * error if the argument is required or delete the node if the argument is optional. + * delete the node if the argument is optional. * * @param node any ValueNode * @returns the node where all variable nodes get replaced with their explicit values. And null if the argument is optional and not set @@ -168,12 +168,9 @@ function inlineFragmentArgs({ return newValue } - // if the argument is required + // if the argument is required, make sure we keep it if (definitionArgs[node.name.value] && definitionArgs[node.name.value].required) { - throw new HoudiniError({ - filepath, - message: 'Missing value for required arg: ' + node.name.value, - }) + return node } // The argument is optional and not set in parent => delete the node. diff --git a/packages/houdini/src/codegen/transforms/paginate.test.ts b/packages/houdini/src/codegen/transforms/paginate.test.ts index 73b23f324..2dd381cf3 100644 --- a/packages/houdini/src/codegen/transforms/paginate.test.ts +++ b/packages/houdini/src/codegen/transforms/paginate.test.ts @@ -323,6 +323,71 @@ test('paginate adds backwards cursor args to the fragment', async function () { `) }) +test('adds required fragment args to pagination query', async function () { + const docs = [ + mockCollectedDoc( + ` + fragment PaginatedFragment on User @arguments(snapshot: { type: "String!" }) { + friendsByCursorSnapshot(first: 2, snapshot: $snapshot) @paginate { + edges { + node { + id + name + } + } + } + } + ` + ), + ] + + // run the pipeline + const config = testConfig() + await runPipeline(config, docs) + + // load the contents of the file + expect(docs[1]?.document).toMatchInlineSnapshot(` + query PaginatedFragment_Pagination_Query($snapshot: String!, $first: Int = 2, $after: String, $last: Int, $before: String, $id: ID!) { + node(id: $id) { + __typename + id + ...PaginatedFragment_2XQ2zF @with(snapshot: $snapshot, first: $first, after: $after, last: $last, before: $before) @mask_disable + } + } + + fragment PaginatedFragment_2XQ2zF on User @arguments(snapshot: {type: "String!"}, first: {type: "Int", default: 2}, after: {type: "String"}, last: {type: "Int"}, before: {type: "String"}) { + friendsByCursorSnapshot( + first: $first + snapshot: $snapshot + after: $after + last: $last + before: $before + ) @paginate { + edges { + node { + id + name + } + } + edges { + cursor + node { + __typename + } + } + pageInfo { + hasPreviousPage + hasNextPage + startCursor + endCursor + } + } + id + __typename + } + `) +}) + test('sets before with default value', async function () { const docs = [ mockCollectedDoc( @@ -677,7 +742,11 @@ test('embeds node pagination query as a separate document', async function () { "id": "ID" }, - "types": {} + "types": {}, + + "defaults": { + "first": 10 + } }, "policy": "CacheOrNetwork", @@ -948,7 +1017,11 @@ test('embeds custom pagination query as a separate document', async function () "aka": "String" }, - "types": {} + "types": {}, + + "defaults": { + "first": 10 + } }, "policy": "CacheOrNetwork", @@ -1732,7 +1805,12 @@ test('generated query has same refetch spec', async function () { "before": "String" }, - "types": {} + "types": {}, + + "defaults": { + "first": 10, + "after": "1234" + } }, "policy": "CacheOrNetwork", @@ -1991,7 +2069,11 @@ test('default defaultPaginateMode to SinglePage', async function () { "before": "String" }, - "types": {} + "types": {}, + + "defaults": { + "first": 10 + } } }; diff --git a/packages/houdini/src/codegen/transforms/paginate.ts b/packages/houdini/src/codegen/transforms/paginate.ts index a686bdb92..eb2e73090 100644 --- a/packages/houdini/src/codegen/transforms/paginate.ts +++ b/packages/houdini/src/codegen/transforms/paginate.ts @@ -3,6 +3,10 @@ import * as graphql from 'graphql' import type { Config, Document } from '../../lib' import { HoudiniError, parentTypeFromAncestors, unwrapType, wrapType } from '../../lib' import { ArtifactKind, type PaginateModes } from '../../runtime/lib/types' +import { + fragmentArguments as collectFragmentArguments, + type FragmentArgument, +} from '../transforms/fragmentVariables' // the paginate transform is responsible for preparing a fragment marked for pagination // to be embedded in the query that will be used to fetch additional data. That means it @@ -163,6 +167,8 @@ export default async function paginate(config: Config, documents: Document[]): P let paginateMode: PaginateModes = config.defaultPaginateMode + const requiredArgs: FragmentArgument[] = [] + doc.document = graphql.visit(doc.document, { // if we are dealing with a query, we'll need to add the variables to the definition OperationDefinition(node) { @@ -236,6 +242,13 @@ export default async function paginate(config: Config, documents: Document[]): P (directive) => directive.name.value === config.argumentsDirective ) + // keep track of all the required arguments on the fragment + requiredArgs.push( + ...collectFragmentArguments(config, doc.filename, node).filter( + (arg) => arg.required + ) + ) + // if there isn't an arguments directive, add it and we'll add arguments to it when // we run into it again if (!argDirective) { @@ -365,9 +378,9 @@ export default async function paginate(config: Config, documents: Document[]): P kind: graphql.Kind.NAME, value: config.withDirective, }, - ['arguments']: paginationArgs.map(({ name }) => - variableAsArgument(name) - ), + ['arguments']: requiredArgs + .map((arg) => variableAsArgument(arg.name)) + .concat(paginationArgs.map(({ name }) => variableAsArgument(name))), }, { kind: graphql.Kind.DIRECTIVE, @@ -419,18 +432,12 @@ export default async function paginate(config: Config, documents: Document[]): P value: refetchQueryName, }, operation: 'query', - variableDefinitions: paginationArgs + variableDefinitions: requiredArgs .map( (arg) => ({ kind: graphql.Kind.VARIABLE_DEFINITION, - type: { - kind: graphql.Kind.NAMED_TYPE, - name: { - kind: graphql.Kind.NAME, - value: arg.type, - }, - }, + type: arg.type, variable: { kind: graphql.Kind.VARIABLE, name: { @@ -438,33 +445,56 @@ export default async function paginate(config: Config, documents: Document[]): P value: arg.name, }, }, - defaultValue: !flags[arg.name].defaultValue - ? undefined - : { - kind: (arg.type + 'Value') as - | 'IntValue' - | 'StringValue', - value: flags[arg.name].defaultValue, - }, } as graphql.VariableDefinitionNode) ) .concat( - !nodeQuery - ? [] - : keys.map( - (key) => - ({ - kind: graphql.Kind.VARIABLE_DEFINITION, - type: key.type, - variable: { - kind: graphql.Kind.VARIABLE, - name: { - kind: graphql.Kind.NAME, - value: key.name, - }, + paginationArgs + .map( + (arg) => + ({ + kind: graphql.Kind.VARIABLE_DEFINITION, + type: { + kind: graphql.Kind.NAMED_TYPE, + name: { + kind: graphql.Kind.NAME, + value: arg.type, + }, + }, + variable: { + kind: graphql.Kind.VARIABLE, + name: { + kind: graphql.Kind.NAME, + value: arg.name, }, - } as graphql.VariableDefinitionNode) - ) + }, + defaultValue: !flags[arg.name].defaultValue + ? undefined + : { + kind: (arg.type + 'Value') as + | 'IntValue' + | 'StringValue', + value: flags[arg.name].defaultValue, + }, + } as graphql.VariableDefinitionNode) + ) + .concat( + !nodeQuery + ? [] + : keys.map( + (key) => + ({ + kind: graphql.Kind.VARIABLE_DEFINITION, + type: key.type, + variable: { + kind: graphql.Kind.VARIABLE, + name: { + kind: graphql.Kind.NAME, + value: key.name, + }, + }, + } as graphql.VariableDefinitionNode) + ) + ) ), selectionSet: { kind: graphql.Kind.SELECTION_SET, diff --git a/packages/houdini/src/codegen/validators/typeCheck.test.ts b/packages/houdini/src/codegen/validators/typeCheck.test.ts index cda93c9a1..0bea46608 100755 --- a/packages/houdini/src/codegen/validators/typeCheck.test.ts +++ b/packages/houdini/src/codegen/validators/typeCheck.test.ts @@ -779,8 +779,8 @@ const table: Row[] = [ ], }, { - title: "@paginate can't show up in a document with required args", - pass: false, + title: '@paginate can show up in a document with required args', + pass: true, documents: [ ` fragment UserPaginatedA on User @arguments(foo: { type: "String!" }) { diff --git a/packages/houdini/src/codegen/validators/typeCheck.ts b/packages/houdini/src/codegen/validators/typeCheck.ts index 88878e467..af9ff9381 100755 --- a/packages/houdini/src/codegen/validators/typeCheck.ts +++ b/packages/houdini/src/codegen/validators/typeCheck.ts @@ -860,27 +860,6 @@ function paginateArgs(config: Config, filepath: string) { // make sure we fail if we see another paginated field alreadyPaginated = true - // find the definition containing the directive - const { definition } = definitionFromAncestors(ancestors) - - // look at the fragment arguments - const definitionArgs = collectFragmentArguments( - config, - filepath, - definition as graphql.FragmentDefinitionNode - ) - - // a fragment marked for pagination can't have required args - const hasRequiredArgs = definitionArgs.find((arg) => arg.required) - if (hasRequiredArgs) { - ctx.reportError( - new graphql.GraphQLError( - '@paginate cannot appear on a document with required args' - ) - ) - return - } - // look at the field the directive is applied to const targetFieldType = parentTypeFromAncestors( config.schema, diff --git a/packages/houdini/src/runtime/cache/cache.ts b/packages/houdini/src/runtime/cache/cache.ts index 59dcf9084..fb0f762d7 100644 --- a/packages/houdini/src/runtime/cache/cache.ts +++ b/packages/houdini/src/runtime/cache/cache.ts @@ -950,10 +950,7 @@ class CacheInternal { key, { parent, - variables: evaluateFragmentVariables( - value.arguments, - variables ?? {} - ), + variables: evaluateVariables(value.arguments, variables ?? {}), }, ]) ), @@ -1005,7 +1002,7 @@ class CacheInternal { }) if (includeDirective) { // if the `if` argument evaluates to false, skip the field - if (!evaluateFragmentVariables(includeDirective.arguments, variables ?? {})['if']) { + if (!evaluateVariables(includeDirective.arguments, variables ?? {})['if']) { continue } } @@ -1014,7 +1011,7 @@ class CacheInternal { }) if (skipDirective) { // if the `if` argument evaluates to false, skip the field - if (evaluateFragmentVariables(skipDirective.arguments, variables ?? {})['if']) { + if (evaluateVariables(skipDirective.arguments, variables ?? {})['if']) { continue } } @@ -1489,9 +1486,9 @@ class CacheInternal { } } -export function evaluateFragmentVariables(variables: ValueMap, args: GraphQLObject) { +export function evaluateVariables(variables: ValueMap, args: GraphQLObject) { return Object.fromEntries( - Object.entries(variables).map(([key, value]) => [key, fragmentVariableValue(value, args)]) + Object.entries(variables).map(([key, value]) => [key, variableValue(value, args)]) ) } @@ -1502,7 +1499,7 @@ function wrapInLists(target: T, count: number = 0): T | NestedList { return wrapInLists([target], count - 1) } -function fragmentVariableValue(value: ValueNode, args: GraphQLObject): GraphQLValue { +export function variableValue(value: ValueNode, args: GraphQLObject): GraphQLValue { if (value.kind === 'StringValue') { return value.value } @@ -1525,14 +1522,14 @@ function fragmentVariableValue(value: ValueNode, args: GraphQLObject): GraphQLVa return args[value.name.value] } if (value.kind === 'ListValue') { - return value.values.map((value) => fragmentVariableValue(value, args)) + return value.values.map((value) => variableValue(value, args)) } if (value.kind === 'ObjectValue') { return value.fields.reduce( (obj, field) => ({ ...obj, - [field.name.value]: fragmentVariableValue(field.value, args), + [field.name.value]: variableValue(field.value, args), }), {} ) @@ -1571,7 +1568,7 @@ export function defaultComponentField({ // look up the component in the store const componentFn = cache._internal_unstable.componentCache[component.key] - const args = evaluateFragmentVariables(component.variables ?? {}, variables ?? {}) + const args = evaluateVariables(component.variables ?? {}, variables ?? {}) // return the instantiated component with the appropriate prop return cache._internal_unstable.createComponent(componentFn, { diff --git a/packages/houdini/src/runtime/cache/tests/variables.test.ts b/packages/houdini/src/runtime/cache/tests/variables.test.ts index 43562d1fc..2a5562005 100644 --- a/packages/houdini/src/runtime/cache/tests/variables.test.ts +++ b/packages/houdini/src/runtime/cache/tests/variables.test.ts @@ -1,7 +1,7 @@ import { test, expect, describe } from 'vitest' import type { GraphQLObject, ValueMap } from '../../lib/types' -import { evaluateFragmentVariables } from '../cache' +import { evaluateVariables } from '../cache' describe('evaluateFragmentVariables', function () { const table: { title: string; input: ValueMap; variables: GraphQLObject; expected: any }[] = [ @@ -122,7 +122,7 @@ describe('evaluateFragmentVariables', function () { for (const row of table) { test(row.title, function () { - expect(evaluateFragmentVariables(row.input, row.variables)).toEqual(row.expected) + expect(evaluateVariables(row.input, row.variables)).toEqual(row.expected) }) } }) diff --git a/packages/houdini/src/runtime/lib/types.ts b/packages/houdini/src/runtime/lib/types.ts index 6785b8dd2..5dfe44221 100644 --- a/packages/houdini/src/runtime/lib/types.ts +++ b/packages/houdini/src/runtime/lib/types.ts @@ -94,6 +94,7 @@ export type RefetchUpdateModes = ValuesOf export type InputObject = { fields: Record types: Record> + defaults: Record } export type BaseCompiledDocument<_Kind extends ArtifactKinds> = { diff --git a/packages/houdini/src/test/index.ts b/packages/houdini/src/test/index.ts index cb34b2387..cfe1663cc 100644 --- a/packages/houdini/src/test/index.ts +++ b/packages/houdini/src/test/index.ts @@ -26,6 +26,7 @@ export function testConfigFile({ plugins, ...config }: Partial = {}) firstName: String! friends: [User!]! friendsByCursor(first: Int, after: String, last: Int, before: String, filter: String): UserConnection! + friendsByCursorSnapshot(snapshot: String!, first: Int, after: String, last: Int, before: String): UserConnection! friendsByCursorScalar(first: Int, after: Cursor, last: Int, before: Cursor, filter: String): UserConnection! friendsByBackwardsCursor(last: Int, before: String, filter: String): UserConnectionScalar! friendsByForwardsCursor(first: Int, after: String, filter: String): UserConnection!