From d3e151dfca027fc2d728820a86435259bc49c916 Mon Sep 17 00:00:00 2001 From: Alec Aivazis Date: Tue, 10 Sep 2024 15:43:04 -0700 Subject: [PATCH 01/15] add dedupe directive --- packages/houdini/src/codegen/transforms/schema.ts | 8 ++++++++ packages/houdini/src/lib/config.ts | 4 ++++ 2 files changed, 12 insertions(+) diff --git a/packages/houdini/src/codegen/transforms/schema.ts b/packages/houdini/src/codegen/transforms/schema.ts index 369cc5cbc..6387fdb67 100644 --- a/packages/houdini/src/codegen/transforms/schema.ts +++ b/packages/houdini/src/codegen/transforms/schema.ts @@ -52,6 +52,14 @@ directive @${config.paginateDirective}(${config.listOrPaginateNameArg}: String, """ directive @${config.listPrependDirective} on FRAGMENT_SPREAD +""" + @${ + config.dedupeDirective + } is used to prevent an operation from running twice. If the operation is sent + while there is already one pending and the cancelFirst arg is true, the first one will be canceled instead of the second. +""" +directive @${config.dedupeDirective}(cancelFirst: Boolean) on QUERY | MUTATION + """ @${config.optimisticKeyDirective} is used to identify a field as an optimistic key """ diff --git a/packages/houdini/src/lib/config.ts b/packages/houdini/src/lib/config.ts index 6dd0886f9..d31fc8533 100644 --- a/packages/houdini/src/lib/config.ts +++ b/packages/houdini/src/lib/config.ts @@ -629,6 +629,10 @@ export class Config { return 'list' } + get dedupeDirective() { + return 'list' + } + get optimisticKeyDirective() { return 'optimisticKey' } From f3e43d97b05b10c47de71cc034280b7673264089 Mon Sep 17 00:00:00 2001 From: Alec Aivazis Date: Tue, 10 Sep 2024 15:46:55 -0700 Subject: [PATCH 02/15] import e2e tests --- e2e/_api/graphql.mjs | 8 +++- e2e/_api/schema.graphql | 1 + e2e/kit/src/lib/utils/routes.ts | 1 + .../query/dedupe-pagination-fetch/+page.gql | 9 ++++ .../dedupe-pagination-fetch/+page.svelte | 16 ++++++++ .../query/dedupe-pagination-fetch/spec.ts | 41 +++++++++++++++++++ 6 files changed, 75 insertions(+), 1 deletion(-) create mode 100644 e2e/kit/src/routes/pagination/query/dedupe-pagination-fetch/+page.gql create mode 100644 e2e/kit/src/routes/pagination/query/dedupe-pagination-fetch/+page.svelte create mode 100644 e2e/kit/src/routes/pagination/query/dedupe-pagination-fetch/spec.ts diff --git a/e2e/_api/graphql.mjs b/e2e/_api/graphql.mjs index 6aadc6d1a..48d7c0b8b 100644 --- a/e2e/_api/graphql.mjs +++ b/e2e/_api/graphql.mjs @@ -122,6 +122,7 @@ export const typeDefs = /* GraphQL */ ` before: String first: Int last: Int + delay: Int snapshot: String! ): UserConnection! usersList(limit: Int = 4, offset: Int, snapshot: String!): [User!]! @@ -450,7 +451,12 @@ export const resolvers = { } throw new GraphQLError('No authorization found', { code: 403 }) }, - usersConnection(_, args) { + usersConnection: async (_, args) => { + // simulate network delay + if (args.delay) { + await sleep(args.delay) + } + return connectionFromArray(getUserSnapshot(args.snapshot), args) }, user: async (_, args) => { diff --git a/e2e/_api/schema.graphql b/e2e/_api/schema.graphql index 21bc432e9..67a5974c3 100644 --- a/e2e/_api/schema.graphql +++ b/e2e/_api/schema.graphql @@ -110,6 +110,7 @@ type Query { before: String first: Int last: Int + delay: Int snapshot: String! ): UserConnection! usersList(limit: Int = 4, offset: Int, snapshot: String!): [User!]! diff --git a/e2e/kit/src/lib/utils/routes.ts b/e2e/kit/src/lib/utils/routes.ts index 2d7f66bf0..6f1bf3a87 100644 --- a/e2e/kit/src/lib/utils/routes.ts +++ b/e2e/kit/src/lib/utils/routes.ts @@ -92,6 +92,7 @@ export const routes = { Pagination_query_bidirectional_cursor: '/pagination/query/bidirectional-cursor', Pagination_query_bidirectional_cursor_single_page: '/pagination/query/bidirectional-cursor-single-page', + Pagination_query_dedupe_pagination_fetch: 'pagination/query/dedupe-pagination-fetch', Pagination_query_offset: '/pagination/query/offset', Pagination_query_offset_single_page: '/pagination/query/offset-single-page', Pagination_query_offset_variable: '/pagination/query/offset-variable', diff --git a/e2e/kit/src/routes/pagination/query/dedupe-pagination-fetch/+page.gql b/e2e/kit/src/routes/pagination/query/dedupe-pagination-fetch/+page.gql new file mode 100644 index 000000000..0f9cbbef8 --- /dev/null +++ b/e2e/kit/src/routes/pagination/query/dedupe-pagination-fetch/+page.gql @@ -0,0 +1,9 @@ +query DedupePaginationFetch { + usersConnection(first: 2, delay: 500, snapshot: "dedupe-pagination-fetch") @paginate { + edges { + node { + name + } + } + } +} diff --git a/e2e/kit/src/routes/pagination/query/dedupe-pagination-fetch/+page.svelte b/e2e/kit/src/routes/pagination/query/dedupe-pagination-fetch/+page.svelte new file mode 100644 index 000000000..33b267cc4 --- /dev/null +++ b/e2e/kit/src/routes/pagination/query/dedupe-pagination-fetch/+page.svelte @@ -0,0 +1,16 @@ + + +
+ {$DedupePaginationFetch.data?.usersConnection.edges.map(({ node }) => node?.name).join(', ')} +
+ +
+ {JSON.stringify($DedupePaginationFetch.pageInfo)} +
+ + diff --git a/e2e/kit/src/routes/pagination/query/dedupe-pagination-fetch/spec.ts b/e2e/kit/src/routes/pagination/query/dedupe-pagination-fetch/spec.ts new file mode 100644 index 000000000..b0a11cc5b --- /dev/null +++ b/e2e/kit/src/routes/pagination/query/dedupe-pagination-fetch/spec.ts @@ -0,0 +1,41 @@ +import { sleep } from '@kitql/helpers'; +import test, { expect, type Response } from '@playwright/test'; +import { routes } from '../../../../lib/utils/routes.js'; +import { expect_1_gql, expect_to_be, goto } from '../../../../lib/utils/testsHelper.js'; + +test('pagination before previous request was finished', async ({ page }) => { + await goto(page, routes.Pagination_query_dedupe_pagination_fetch); + + await expect_to_be(page, 'Bruce Willis, Samuel Jackson'); + + // Adapted from `expect_n_gql` in lib/utils/testsHelper.ts + let nbResponses = 0; + async function fnRes(response: Response) { + if (response.url().endsWith(routes.GraphQL)) { + nbResponses++; + } + } + + page.on('response', fnRes); + + // Click the "next page" button twice + await page.click('button[id=next]'); + await page.click('button[id=next]'); + + // Give the query some time to execute + await sleep(1000); + + // Check that only one gql request happened. + expect(nbResponses).toBe(1); + + page.removeListener('response', fnRes); + + await expect_to_be(page, 'Bruce Willis, Samuel Jackson, Morgan Freeman, Tom Hanks'); + + // Fetching the 3rd page still works ok. + await expect_1_gql(page, 'button[id=next]'); + await expect_to_be( + page, + 'Bruce Willis, Samuel Jackson, Morgan Freeman, Tom Hanks, Will Smith, Harrison Ford' + ); +}); From 5e14e51ecae1f14aef8b3773b1a0666dcc7366af Mon Sep 17 00:00:00 2001 From: Alec Aivazis Date: Tue, 10 Sep 2024 16:02:59 -0700 Subject: [PATCH 03/15] persist dedupe argument --- .../src/codegen/generators/artifacts/index.ts | 26 ++++ .../artifacts/tests/artifacts.test.ts | 144 ++++++++++++++++++ .../generators/definitions/schema.test.ts | 24 +++ .../src/codegen/transforms/paginate.test.ts | 4 +- .../src/codegen/transforms/paginate.ts | 9 ++ packages/houdini/src/lib/config.ts | 2 +- packages/houdini/src/runtime/lib/types.ts | 2 + 7 files changed, 208 insertions(+), 3 deletions(-) diff --git a/packages/houdini/src/codegen/generators/artifacts/index.ts b/packages/houdini/src/codegen/generators/artifacts/index.ts index 2d2af2873..821a34ab7 100644 --- a/packages/houdini/src/codegen/generators/artifacts/index.ts +++ b/packages/houdini/src/codegen/generators/artifacts/index.ts @@ -187,6 +187,9 @@ export default function artifactGenerator(stats: { let selectionSet: graphql.SelectionSetNode let originalSelectionSet: graphql.SelectionSetNode | null = null + // extract the deduplication behavior + let dedupe: QueryArtifact['dedupe'] + const fragmentDefinitions = doc.document.definitions .filter( (definition): definition is graphql.FragmentDefinitionNode => @@ -222,6 +225,22 @@ export default function artifactGenerator(stats: { }) } + const dedupeDirective = operation.directives?.find( + (directive) => directive.name.value === config.dedupeDirective + ) + if (dedupeDirective) { + const cancelFirstArg = dedupeDirective.arguments?.find( + (arg) => arg.name.value === 'cancelFirst' + ) + + dedupe = + cancelFirstArg && + cancelFirstArg.value.kind === 'BooleanValue' && + cancelFirstArg.value + ? 'last' + : 'first' + } + // use this selection set selectionSet = operation.selectionSet if (originalParsed.definitions[0].kind === 'OperationDefinition') { @@ -320,6 +339,13 @@ export default function artifactGenerator(stats: { const hash_value = hashPluginBaseRaw({ config, document: { ...doc, artifact } }) artifact.hash = hash_value + if ( + artifact.kind === 'HoudiniQuery' || + (artifact.kind === 'HoudiniMutation' && dedupe) + ) { + artifact.dedupe = dedupe + } + // apply the visibility mask to the artifact so that only // fields in the direct selection are visible applyMask( 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 2ad663daf..46e804239 100644 --- a/packages/houdini/src/codegen/generators/artifacts/tests/artifacts.test.ts +++ b/packages/houdini/src/codegen/generators/artifacts/tests/artifacts.test.ts @@ -7707,3 +7707,147 @@ describe('default arguments', function () { `) }) }) + +test('persists dedupe which', async function () { + // the config to use in tests + const config = testConfig() + // the documents to test + const docs: Document[] = [ + mockCollectedDoc(` + query FindUser @dedupe{ + usersByOffset { + 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": "63be02f78e12d6dd155da0aac94892e700a5be1eeb66dfc2305740ce2464dd3b", + + "raw": \`query FindUser { + usersByOffset { + name + id + } + } + \`, + + "rootType": "Query", + "stripVariables": [], + + "selection": { + "fields": { + "usersByOffset": { + "type": "User", + "keyRaw": "usersByOffset", + + "selection": { + "fields": { + "name": { + "type": "String", + "keyRaw": "name", + "visible": true + }, + + "id": { + "type": "ID", + "keyRaw": "id", + "visible": true + } + } + }, + + "visible": true + } + } + }, + + "pluginData": {}, + "dedupe": "first", + "policy": "CacheOrNetwork", + "partial": false + }; + + "HoudiniHash=752d5f5b068733a0ab1039b96b5f9d13a45a872329bca86998b1971c4ce0816b"; + `) +}) + +test('persists dedupe first', async function () { + // the config to use in tests + const config = testConfig() + // the documents to test + const docs: Document[] = [ + mockCollectedDoc(` + query FindUser @dedupe(cancelFirst: true) { + usersByOffset { + 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": "63be02f78e12d6dd155da0aac94892e700a5be1eeb66dfc2305740ce2464dd3b", + + "raw": \`query FindUser { + usersByOffset { + name + id + } + } + \`, + + "rootType": "Query", + "stripVariables": [], + + "selection": { + "fields": { + "usersByOffset": { + "type": "User", + "keyRaw": "usersByOffset", + + "selection": { + "fields": { + "name": { + "type": "String", + "keyRaw": "name", + "visible": true + }, + + "id": { + "type": "ID", + "keyRaw": "id", + "visible": true + } + } + }, + + "visible": true + } + } + }, + + "pluginData": {}, + "dedupe": "last", + "policy": "CacheOrNetwork", + "partial": false + }; + + "HoudiniHash=3dfb64916aa4359cf85f08b3544bbc7382fd818935c5a0e92f324a2d2519c227"; + `) +}) diff --git a/packages/houdini/src/codegen/generators/definitions/schema.test.ts b/packages/houdini/src/codegen/generators/definitions/schema.test.ts index 528c219dc..ffea0c9f4 100644 --- a/packages/houdini/src/codegen/generators/definitions/schema.test.ts +++ b/packages/houdini/src/codegen/generators/definitions/schema.test.ts @@ -36,6 +36,12 @@ test('adds internal documents to schema', async function () { """@prepend is used to tell the runtime to add the result to the end of the list""" directive @prepend on FRAGMENT_SPREAD + """ + @dedupe is used to prevent an operation from running twice. If the operation is sent + while there is already one pending and the cancelFirst arg is true, the first one will be canceled instead of the second. + """ + directive @dedupe(cancelFirst: Boolean) on QUERY | MUTATION + """@optimisticKey is used to identify a field as an optimistic key""" directive @optimisticKey on FIELD @@ -128,6 +134,12 @@ test('list operations are included', async function () { """@prepend is used to tell the runtime to add the result to the end of the list""" directive @prepend on FRAGMENT_SPREAD + """ + @dedupe is used to prevent an operation from running twice. If the operation is sent + while there is already one pending and the cancelFirst arg is true, the first one will be canceled instead of the second. + """ + directive @dedupe(cancelFirst: Boolean) on QUERY | MUTATION + """@optimisticKey is used to identify a field as an optimistic key""" directive @optimisticKey on FIELD @@ -239,6 +251,12 @@ test('list operations are included but delete directive should not be in when we """@prepend is used to tell the runtime to add the result to the end of the list""" directive @prepend on FRAGMENT_SPREAD + """ + @dedupe is used to prevent an operation from running twice. If the operation is sent + while there is already one pending and the cancelFirst arg is true, the first one will be canceled instead of the second. + """ + directive @dedupe(cancelFirst: Boolean) on QUERY | MUTATION + """@optimisticKey is used to identify a field as an optimistic key""" directive @optimisticKey on FIELD @@ -363,6 +381,12 @@ test("writing twice doesn't duplicate definitions", async function () { """@prepend is used to tell the runtime to add the result to the end of the list""" directive @prepend on FRAGMENT_SPREAD + """ + @dedupe is used to prevent an operation from running twice. If the operation is sent + while there is already one pending and the cancelFirst arg is true, the first one will be canceled instead of the second. + """ + directive @dedupe(cancelFirst: Boolean) on QUERY | MUTATION + """@optimisticKey is used to identify a field as an optimistic key""" directive @optimisticKey on FIELD diff --git a/packages/houdini/src/codegen/transforms/paginate.test.ts b/packages/houdini/src/codegen/transforms/paginate.test.ts index c257f7149..e29578132 100644 --- a/packages/houdini/src/codegen/transforms/paginate.test.ts +++ b/packages/houdini/src/codegen/transforms/paginate.test.ts @@ -347,7 +347,7 @@ test('adds required fragment args to pagination query', async function () { // 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!) { + query PaginatedFragment_Pagination_Query($snapshot: String!, $first: Int = 2, $after: String, $last: Int, $before: String, $id: ID!) @dedupe { node(id: $id) { __typename id @@ -460,7 +460,7 @@ test('embeds pagination query as a separate document', async function () { // load the contents of the file expect(docs[1]?.document).toMatchInlineSnapshot(` - query UserFriends_Pagination_Query($first: Int = 10, $after: String) { + query UserFriends_Pagination_Query($first: Int = 10, $after: String) @dedupe { ...UserFriends_jrGTj @with(first: $first, after: $after) @mask_disable } diff --git a/packages/houdini/src/codegen/transforms/paginate.ts b/packages/houdini/src/codegen/transforms/paginate.ts index eb2e73090..2bb29cd43 100644 --- a/packages/houdini/src/codegen/transforms/paginate.ts +++ b/packages/houdini/src/codegen/transforms/paginate.ts @@ -496,6 +496,15 @@ export default async function paginate(config: Config, documents: Document[]): P ) ) ), + directives: [ + { + kind: graphql.Kind.DIRECTIVE, + name: { + kind: graphql.Kind.NAME, + value: config.dedupeDirective, + }, + }, + ], selectionSet: { kind: graphql.Kind.SELECTION_SET, selections: !nodeQuery diff --git a/packages/houdini/src/lib/config.ts b/packages/houdini/src/lib/config.ts index d31fc8533..c150c194a 100644 --- a/packages/houdini/src/lib/config.ts +++ b/packages/houdini/src/lib/config.ts @@ -630,7 +630,7 @@ export class Config { } get dedupeDirective() { - return 'list' + return 'dedupe' } get optimisticKeyDirective() { diff --git a/packages/houdini/src/runtime/lib/types.ts b/packages/houdini/src/runtime/lib/types.ts index f7b831db3..2c0611467 100644 --- a/packages/houdini/src/runtime/lib/types.ts +++ b/packages/houdini/src/runtime/lib/types.ts @@ -77,10 +77,12 @@ export type QueryArtifact = BaseCompiledDocument<'HoudiniQuery'> & { policy?: CachePolicies partial?: boolean enableLoadingState?: 'global' | 'local' + dedupe?: 'first' | 'last' } export type MutationArtifact = BaseCompiledDocument<'HoudiniMutation'> & { optimisticKeys?: boolean + dedupe?: 'first' | 'last' } export type FragmentArtifact = BaseCompiledDocument<'HoudiniFragment'> & { From 132be7a881fb9f91594829daff03e296634ca6b6 Mon Sep 17 00:00:00 2001 From: Alec Aivazis Date: Tue, 10 Sep 2024 17:03:49 -0700 Subject: [PATCH 04/15] wire up abort controllers --- packages/houdini/src/runtime/client/documentStore.ts | 9 +++++++++ packages/houdini/src/runtime/client/plugins/fetch.ts | 2 ++ 2 files changed, 11 insertions(+) diff --git a/packages/houdini/src/runtime/client/documentStore.ts b/packages/houdini/src/runtime/client/documentStore.ts index 13d30d61e..330d62d29 100644 --- a/packages/houdini/src/runtime/client/documentStore.ts +++ b/packages/houdini/src/runtime/client/documentStore.ts @@ -131,9 +131,11 @@ export class DocumentStore< cacheParams, setup = false, silenceEcho = false, + abortController = new AbortController(), }: SendParams = {}) { // start off with the initial context let context = new ClientPluginContextWrapper({ + abortController, config: this.#configFile!, name: this.artifact.name, text: this.artifact.raw, @@ -358,6 +360,11 @@ export class DocumentStore< } try { + // if the abort controller has been triggered, dont go further + if (draft.abortController.signal.aborted) { + throw new DOMException('aborted') + } + // @ts-expect-error // invoke the target with the correct handlers const result = target(draft, handlers) @@ -635,6 +642,7 @@ export type ClientPluginContext = { metadata?: App.Metadata | null session?: App.Session | null fetchParams?: RequestInit + abortController: AbortController cacheParams?: { layer?: Layer notifySubscribers?: SubscriptionSpec[] @@ -696,4 +704,5 @@ export type SendParams = { cacheParams?: ClientPluginContext['cacheParams'] setup?: boolean silenceEcho?: boolean + abortController?: AbortController } diff --git a/packages/houdini/src/runtime/client/plugins/fetch.ts b/packages/houdini/src/runtime/client/plugins/fetch.ts index ceaeb7dcd..23a49c08f 100644 --- a/packages/houdini/src/runtime/client/plugins/fetch.ts +++ b/packages/houdini/src/runtime/client/plugins/fetch.ts @@ -20,6 +20,7 @@ export const fetch = (target?: RequestHandler | string): ClientPlugin => { text: ctx.text, hash: ctx.hash, variables: { ...marshalVariables(ctx) }, + signal: ctx.abortController.signal, } // before we move onto the next plugin, we need to strip the variables as they go through @@ -124,6 +125,7 @@ export type FetchParams = { text: string hash: string variables: { [key: string]: any } + signal: AbortSignal } function handleMultipart( From a72e880a5918a58a5257db9ecba0e6984285417f Mon Sep 17 00:00:00 2001 From: Alec Aivazis Date: Wed, 11 Sep 2024 09:35:21 -0700 Subject: [PATCH 05/15] test pass! --- .../src/runtime/stores/pagination/query.ts | 22 +++++++++- .../src/codegen/transforms/paginate.ts | 19 +++++---- .../src/runtime/client/documentStore.ts | 42 ++++++++++++++++++- .../houdini/src/runtime/lib/pagination.ts | 34 ++++++++++----- packages/houdini/src/runtime/lib/types.ts | 4 +- 5 files changed, 96 insertions(+), 25 deletions(-) diff --git a/packages/houdini-svelte/src/runtime/stores/pagination/query.ts b/packages/houdini-svelte/src/runtime/stores/pagination/query.ts index 06b85b25b..2c600a31d 100644 --- a/packages/houdini-svelte/src/runtime/stores/pagination/query.ts +++ b/packages/houdini-svelte/src/runtime/stores/pagination/query.ts @@ -88,12 +88,30 @@ export class QueryStoreCursor< args?: Parameters>['loadPreviousPage']>[0] ) { const handlers = await this.#handlers() - return await handlers.loadPreviousPage(args) + try { + return await handlers.loadPreviousPage(args) + } catch (e) { + const err = e as Error + // if the error is an abort error then we don't want to throw + if (err.name === 'AbortError') { + } else { + throw err + } + } } async loadNextPage(args?: Parameters['loadNextPage']>[0]) { const handlers = await this.#handlers() - return await handlers.loadNextPage(args) + try { + return await handlers.loadNextPage(args) + } catch (e) { + const err = e as Error + // if the error is an abort error then we don't want to throw + if (err.name === 'AbortError') { + } else { + throw err + } + } } subscribe( diff --git a/packages/houdini/src/codegen/transforms/paginate.ts b/packages/houdini/src/codegen/transforms/paginate.ts index 2bb29cd43..c73f6018e 100644 --- a/packages/houdini/src/codegen/transforms/paginate.ts +++ b/packages/houdini/src/codegen/transforms/paginate.ts @@ -225,6 +225,16 @@ export default async function paginate(config: Config, documents: Document[]): P return { ...node, variableDefinitions: finalVariables, + directives: [ + ...(node.directives || []), + { + kind: graphql.Kind.DIRECTIVE, + name: { + kind: graphql.Kind.NAME, + value: config.dedupeDirective, + }, + }, + ], } as graphql.OperationDefinitionNode }, // if we are dealing with a fragment definition we'll need to add the arguments directive if it doesn't exist @@ -496,15 +506,6 @@ export default async function paginate(config: Config, documents: Document[]): P ) ) ), - directives: [ - { - kind: graphql.Kind.DIRECTIVE, - name: { - kind: graphql.Kind.NAME, - value: config.dedupeDirective, - }, - }, - ], selectionSet: { kind: graphql.Kind.SELECTION_SET, selections: !nodeQuery diff --git a/packages/houdini/src/runtime/client/documentStore.ts b/packages/houdini/src/runtime/client/documentStore.ts index 330d62d29..9e3b6c37a 100644 --- a/packages/houdini/src/runtime/client/documentStore.ts +++ b/packages/houdini/src/runtime/client/documentStore.ts @@ -24,6 +24,8 @@ const steps = { backwards: ['end', 'afterNetwork'], } as const +let inFlightPageFetches: Record = {} + export class DocumentStore< _Data extends GraphQLObject, _Input extends GraphQLVariables @@ -47,6 +49,10 @@ export class DocumentStore< serverSideFallback?: boolean + controllerKey(variables: any) { + return this.artifact.name + } + constructor({ artifact, plugins, @@ -133,6 +139,29 @@ export class DocumentStore< silenceEcho = false, abortController = new AbortController(), }: SendParams = {}) { + // if the document we are sending is meant to be deduped, then we need to look for an existing + // controller for the document + if ('dedupe' in this.artifact) { + // if there is already a pending request + if (inFlightPageFetches[this.controllerKey(variables)]) { + // we have to abort _something_ + if (this.artifact.dedupe === 'first') { + // cancel the existing one + inFlightPageFetches[this.controllerKey(variables)].abort() + // and register the new one + inFlightPageFetches[this.controllerKey(variables)] = abortController + } + // otherwise we have to abort this one + else { + abortController.abort() + } + } + // register this abort controller as being in flight + else { + inFlightPageFetches[this.controllerKey(variables)] = abortController + } + } + // start off with the initial context let context = new ClientPluginContextWrapper({ abortController, @@ -189,7 +218,14 @@ export class DocumentStore< this.#step('forward', state) }) - return await promise + // fire off the chain + const response = await promise + + // after the whole plugin chain, we need to clean up the in flight tracking + delete inFlightPageFetches[this.controllerKey(variables)] + + // we're done + return response } async cleanup() { @@ -362,7 +398,9 @@ export class DocumentStore< try { // if the abort controller has been triggered, dont go further if (draft.abortController.signal.aborted) { - throw new DOMException('aborted') + const abortError = new Error('aborted') + abortError.name = 'AbortError' + throw abortError } // @ts-expect-error diff --git a/packages/houdini/src/runtime/lib/pagination.ts b/packages/houdini/src/runtime/lib/pagination.ts index 2c9e0938c..00a80da7c 100644 --- a/packages/houdini/src/runtime/lib/pagination.ts +++ b/packages/houdini/src/runtime/lib/pagination.ts @@ -2,7 +2,7 @@ import type { SendParams } from '../client/documentStore' import { getCurrentConfig } from './config' import { deepEquals } from './deepEquals' import { countPage, extractPageInfo, missingPageSizeError } from './pageInfo' -import { CachePolicy } from './types' +import { CachePolicy, DataSource } from './types' import type { CursorHandlers, FetchFn, @@ -44,8 +44,6 @@ export function cursorHandlers<_Data extends GraphQLObject, _Input extends Graph fetch?: typeof globalThis.fetch where: 'start' | 'end' }) => { - const config = getCurrentConfig() - // build up the variables to pass to the query const loadVariables: _Input = { ...getVariables(), @@ -61,7 +59,7 @@ export function cursorHandlers<_Data extends GraphQLObject, _Input extends Graph let isSinglePage = artifact.refetch?.mode === 'SinglePage' // send the query - await (isSinglePage ? parentFetch : parentFetchUpdate)( + return (isSinglePage ? parentFetch : parentFetchUpdate)( { variables: loadVariables, fetch, @@ -78,7 +76,7 @@ export function cursorHandlers<_Data extends GraphQLObject, _Input extends Graph } return { - loadNextPage: async ({ + loadNextPage: ({ first, after, fetch, @@ -93,7 +91,15 @@ export function cursorHandlers<_Data extends GraphQLObject, _Input extends Graph const currentPageInfo = getPageInfo() // if there is no next page, we're done if (!currentPageInfo.hasNextPage) { - return + return Promise.resolve({ + data: getState(), + errors: null, + fetching: false, + partial: false, + stale: false, + source: DataSource.Cache, + variables: getVariables(), + }) } // only specify the page count if we're given one @@ -105,7 +111,7 @@ export function cursorHandlers<_Data extends GraphQLObject, _Input extends Graph } // load the page - return await loadPage({ + return loadPage({ pageSizeVar: 'first', functionName: 'loadNextPage', input, @@ -114,7 +120,7 @@ export function cursorHandlers<_Data extends GraphQLObject, _Input extends Graph where: 'end', }) }, - loadPreviousPage: async ({ + loadPreviousPage: ({ last, before, fetch, @@ -130,7 +136,15 @@ export function cursorHandlers<_Data extends GraphQLObject, _Input extends Graph // if there is no next page, we're done if (!currentPageInfo.hasPreviousPage) { - return + return Promise.resolve({ + data: getState(), + errors: null, + fetching: false, + partial: false, + stale: false, + source: DataSource.Cache, + variables: getVariables(), + }) } // only specify the page count if we're given one @@ -142,7 +156,7 @@ export function cursorHandlers<_Data extends GraphQLObject, _Input extends Graph } // load the page - return await loadPage({ + return loadPage({ pageSizeVar: 'last', functionName: 'loadPreviousPage', input, diff --git a/packages/houdini/src/runtime/lib/types.ts b/packages/houdini/src/runtime/lib/types.ts index 2c0611467..d4ced0ccf 100644 --- a/packages/houdini/src/runtime/lib/types.ts +++ b/packages/houdini/src/runtime/lib/types.ts @@ -315,13 +315,13 @@ export type CursorHandlers<_Data extends GraphQLObject, _Input> = { after?: string fetch?: typeof globalThis.fetch metadata?: {} - }) => Promise + }) => Promise> loadPreviousPage: (args?: { last?: number before?: string fetch?: typeof globalThis.fetch metadata?: {} - }) => Promise + }) => Promise> fetch(args?: FetchParams<_Input> | undefined): Promise> } From 2b8db9688401d4dabfa6ea8e5a3feddbc19778d1 Mon Sep 17 00:00:00 2001 From: Alec Aivazis Date: Wed, 11 Sep 2024 11:07:14 -0700 Subject: [PATCH 06/15] actually dedupe pagination requests --- .../query/dedupe-pagination-fetch/+page.gql | 9 ---- .../dedupe-pagination-fetch/+page.svelte | 16 -------- .../query/dedupe-pagination-fetch/spec.ts | 41 ------------------- .../routes/pagination/query/dedupe/+page.gql | 15 +++++++ .../routes/pagination/query/dedupe/+page.tsx | 21 ++++++++++ .../routes/pagination/query/dedupe/spec.ts | 40 ++++++++++++++++++ e2e/react/src/utils/routes.ts | 2 + .../src/runtime/hooks/useDocumentHandle.ts | 16 +++++++- .../src/codegen/generators/artifacts/index.ts | 4 +- 9 files changed, 94 insertions(+), 70 deletions(-) delete mode 100644 e2e/kit/src/routes/pagination/query/dedupe-pagination-fetch/+page.gql delete mode 100644 e2e/kit/src/routes/pagination/query/dedupe-pagination-fetch/+page.svelte delete mode 100644 e2e/kit/src/routes/pagination/query/dedupe-pagination-fetch/spec.ts create mode 100644 e2e/react/src/routes/pagination/query/dedupe/+page.gql create mode 100644 e2e/react/src/routes/pagination/query/dedupe/+page.tsx create mode 100644 e2e/react/src/routes/pagination/query/dedupe/spec.ts diff --git a/e2e/kit/src/routes/pagination/query/dedupe-pagination-fetch/+page.gql b/e2e/kit/src/routes/pagination/query/dedupe-pagination-fetch/+page.gql deleted file mode 100644 index 0f9cbbef8..000000000 --- a/e2e/kit/src/routes/pagination/query/dedupe-pagination-fetch/+page.gql +++ /dev/null @@ -1,9 +0,0 @@ -query DedupePaginationFetch { - usersConnection(first: 2, delay: 500, snapshot: "dedupe-pagination-fetch") @paginate { - edges { - node { - name - } - } - } -} diff --git a/e2e/kit/src/routes/pagination/query/dedupe-pagination-fetch/+page.svelte b/e2e/kit/src/routes/pagination/query/dedupe-pagination-fetch/+page.svelte deleted file mode 100644 index 33b267cc4..000000000 --- a/e2e/kit/src/routes/pagination/query/dedupe-pagination-fetch/+page.svelte +++ /dev/null @@ -1,16 +0,0 @@ - - -
- {$DedupePaginationFetch.data?.usersConnection.edges.map(({ node }) => node?.name).join(', ')} -
- -
- {JSON.stringify($DedupePaginationFetch.pageInfo)} -
- - diff --git a/e2e/kit/src/routes/pagination/query/dedupe-pagination-fetch/spec.ts b/e2e/kit/src/routes/pagination/query/dedupe-pagination-fetch/spec.ts deleted file mode 100644 index b0a11cc5b..000000000 --- a/e2e/kit/src/routes/pagination/query/dedupe-pagination-fetch/spec.ts +++ /dev/null @@ -1,41 +0,0 @@ -import { sleep } from '@kitql/helpers'; -import test, { expect, type Response } from '@playwright/test'; -import { routes } from '../../../../lib/utils/routes.js'; -import { expect_1_gql, expect_to_be, goto } from '../../../../lib/utils/testsHelper.js'; - -test('pagination before previous request was finished', async ({ page }) => { - await goto(page, routes.Pagination_query_dedupe_pagination_fetch); - - await expect_to_be(page, 'Bruce Willis, Samuel Jackson'); - - // Adapted from `expect_n_gql` in lib/utils/testsHelper.ts - let nbResponses = 0; - async function fnRes(response: Response) { - if (response.url().endsWith(routes.GraphQL)) { - nbResponses++; - } - } - - page.on('response', fnRes); - - // Click the "next page" button twice - await page.click('button[id=next]'); - await page.click('button[id=next]'); - - // Give the query some time to execute - await sleep(1000); - - // Check that only one gql request happened. - expect(nbResponses).toBe(1); - - page.removeListener('response', fnRes); - - await expect_to_be(page, 'Bruce Willis, Samuel Jackson, Morgan Freeman, Tom Hanks'); - - // Fetching the 3rd page still works ok. - await expect_1_gql(page, 'button[id=next]'); - await expect_to_be( - page, - 'Bruce Willis, Samuel Jackson, Morgan Freeman, Tom Hanks, Will Smith, Harrison Ford' - ); -}); diff --git a/e2e/react/src/routes/pagination/query/dedupe/+page.gql b/e2e/react/src/routes/pagination/query/dedupe/+page.gql new file mode 100644 index 000000000..a34c095b0 --- /dev/null +++ b/e2e/react/src/routes/pagination/query/dedupe/+page.gql @@ -0,0 +1,15 @@ +query DedupePaginationFetch { + usersConnection(first: 2, delay: 1000, snapshot: "dedupe-pagination-fetch") @paginate { + pageInfo { + hasNextPage + hasPreviousPage + startCursor + endCursor + } + edges { + node { + name + } + } + } +} diff --git a/e2e/react/src/routes/pagination/query/dedupe/+page.tsx b/e2e/react/src/routes/pagination/query/dedupe/+page.tsx new file mode 100644 index 000000000..98b7f8980 --- /dev/null +++ b/e2e/react/src/routes/pagination/query/dedupe/+page.tsx @@ -0,0 +1,21 @@ +import { PageProps } from './$types' + +export default function ({ DedupePaginationFetch, DedupePaginationFetch$handle }: PageProps) { + return ( +
+
+ {DedupePaginationFetch.usersConnection.edges + .map(({ node }) => node?.name) + .join(', ')} +
+ +
+ {JSON.stringify(DedupePaginationFetch.usersConnection.pageInfo)} +
+ + +
+ ) +} diff --git a/e2e/react/src/routes/pagination/query/dedupe/spec.ts b/e2e/react/src/routes/pagination/query/dedupe/spec.ts new file mode 100644 index 000000000..a4f1c5c99 --- /dev/null +++ b/e2e/react/src/routes/pagination/query/dedupe/spec.ts @@ -0,0 +1,40 @@ +import test, { expect, type Response } from '@playwright/test' +import { routes } from '~/utils/routes' +import { expect_1_gql, expect_to_be, goto } from '~/utils/testsHelper' + +test('pagination before previous request was finished', async ({ page }) => { + await goto(page, routes.pagination_dedupe) + + await expect_to_be(page, 'Bruce Willis, Samuel Jackson') + + // Adapted from `expect_n_gql` in lib/utils/testsHelper.ts + let nbResponses = 0 + async function fnRes(response: Response) { + if (response.url().endsWith(routes.api)) { + nbResponses++ + } + } + + page.on('response', fnRes) + + // Click the "next page" button twice + await page.click('button[id=next]') + await page.click('button[id=next]') + + // Give the query some time to execute + await page.waitForTimeout(1000) + + // Check that only one gql request happened. + expect(nbResponses).toBe(1) + + page.removeListener('response', fnRes) + + await expect_to_be(page, 'Bruce Willis, Samuel Jackson, Morgan Freeman, Tom Hanks') + + // Fetching the 3rd page still works ok. + await expect_1_gql(page, 'button[id=next]') + await expect_to_be( + page, + 'Bruce Willis, Samuel Jackson, Morgan Freeman, Tom Hanks, Will Smith, Harrison Ford' + ) +}) diff --git a/e2e/react/src/utils/routes.ts b/e2e/react/src/utils/routes.ts index 7d31ca0c9..e1c114eec 100644 --- a/e2e/react/src/utils/routes.ts +++ b/e2e/react/src/utils/routes.ts @@ -1,4 +1,5 @@ export const routes = { + api: '/_api', hello: '/hello-world', scalars: '/scalars', componentFields_simple: '/component_fields/simple', @@ -10,6 +11,7 @@ export const routes = { pagination_query_forwards: '/pagination/query/connection-forwards', pagination_query_bidirectional: '/pagination/query/connection-bidirectional', pagination_query_offset: '/pagination/query/offset', + pagination_dedupe: '/pagination/query/dedupe', pagination_query_offset_singlepage: '/pagination/query/offset-singlepage', pagination_query_offset_variable: '/pagination/query/offset-variable/2', optimistic_keys: '/optimistic-keys', diff --git a/packages/houdini-react/src/runtime/hooks/useDocumentHandle.ts b/packages/houdini-react/src/runtime/hooks/useDocumentHandle.ts index 049b8fc15..5a9684dc7 100644 --- a/packages/houdini-react/src/runtime/hooks/useDocumentHandle.ts +++ b/packages/houdini-react/src/runtime/hooks/useDocumentHandle.ts @@ -56,9 +56,21 @@ export function useDocumentHandle< ) => { return async (value: any) => { setLoading(true) - const result = await fn(value) + let result: _Result | null = null + let err: Error | null = null + try { + result = await fn(value) + } catch (e) { + err = e as Error + } setLoading(false) - return result + // ignore abort errors when loading pages + if (err && err.name !== 'AbortError') { + throw err + } + + // we're done + return result || observer.state } } diff --git a/packages/houdini/src/codegen/generators/artifacts/index.ts b/packages/houdini/src/codegen/generators/artifacts/index.ts index 821a34ab7..17ee695a8 100644 --- a/packages/houdini/src/codegen/generators/artifacts/index.ts +++ b/packages/houdini/src/codegen/generators/artifacts/index.ts @@ -237,8 +237,8 @@ export default function artifactGenerator(stats: { cancelFirstArg && cancelFirstArg.value.kind === 'BooleanValue' && cancelFirstArg.value - ? 'last' - : 'first' + ? 'first' + : 'last' } // use this selection set From 2e95507af1833a7e0b728e12a35f662ab2459401 Mon Sep 17 00:00:00 2001 From: Alec Aivazis Date: Wed, 11 Sep 2024 14:10:55 -0700 Subject: [PATCH 07/15] changeset --- .changeset/wild-pandas-sell.md | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 .changeset/wild-pandas-sell.md diff --git a/.changeset/wild-pandas-sell.md b/.changeset/wild-pandas-sell.md new file mode 100644 index 000000000..93fccb5ba --- /dev/null +++ b/.changeset/wild-pandas-sell.md @@ -0,0 +1,7 @@ +--- +'houdini-svelte': patch +'houdini-react': patch +'houdini': patch +--- + +Add @dedupe directive From 69159509257a89114f9887006bdcae6d146a20c7 Mon Sep 17 00:00:00 2001 From: Alec Aivazis Date: Wed, 11 Sep 2024 14:33:08 -0700 Subject: [PATCH 08/15] actually cancel the request when deduping first --- packages/houdini/src/runtime/client/plugins/fetch.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/houdini/src/runtime/client/plugins/fetch.ts b/packages/houdini/src/runtime/client/plugins/fetch.ts index 23a49c08f..e1b92ab34 100644 --- a/packages/houdini/src/runtime/client/plugins/fetch.ts +++ b/packages/houdini/src/runtime/client/plugins/fetch.ts @@ -20,7 +20,6 @@ export const fetch = (target?: RequestHandler | string): ClientPlugin => { text: ctx.text, hash: ctx.hash, variables: { ...marshalVariables(ctx) }, - signal: ctx.abortController.signal, } // before we move onto the next plugin, we need to strip the variables as they go through @@ -46,7 +45,10 @@ export const fetch = (target?: RequestHandler | string): ClientPlugin => { // figure out if we need to do something special for multipart uploads const newArgs = handleMultipart(fetchParams, args) ?? args // use the new args if they exist, otherwise the old ones are good - return fetch(url, newArgs) + return fetch(url, { + ...newArgs, + signal: ctx.abortController.signal, + }) }, metadata: ctx.metadata, session: ctx.session || {}, @@ -125,7 +127,6 @@ export type FetchParams = { text: string hash: string variables: { [key: string]: any } - signal: AbortSignal } function handleMultipart( From 736e0f6dd536b70011281efd049675716cc885e5 Mon Sep 17 00:00:00 2001 From: Alec Aivazis Date: Wed, 11 Sep 2024 14:47:39 -0700 Subject: [PATCH 09/15] update snapshots --- .../artifacts/tests/artifacts.test.ts | 7 +++-- .../artifacts/tests/pagination.test.ts | 1 + .../src/codegen/transforms/paginate.test.ts | 26 +++++++++---------- 3 files changed, 18 insertions(+), 16 deletions(-) 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 46e804239..66006179d 100644 --- a/packages/houdini/src/codegen/generators/artifacts/tests/artifacts.test.ts +++ b/packages/houdini/src/codegen/generators/artifacts/tests/artifacts.test.ts @@ -846,6 +846,7 @@ test('paginate over unions', async function () { }, "pluginData": {}, + "dedupe": "last", "input": { "fields": { @@ -4931,6 +4932,7 @@ describe('mutation artifacts', function () { }, "pluginData": {}, + "dedupe": "last", "input": { "fields": { @@ -5183,6 +5185,7 @@ describe('mutation artifacts', function () { }, "pluginData": {}, + "dedupe": "last", "input": { "fields": { @@ -7771,7 +7774,7 @@ test('persists dedupe which', async function () { }, "pluginData": {}, - "dedupe": "first", + "dedupe": "last", "policy": "CacheOrNetwork", "partial": false }; @@ -7843,7 +7846,7 @@ test('persists dedupe first', async function () { }, "pluginData": {}, - "dedupe": "last", + "dedupe": "first", "policy": "CacheOrNetwork", "partial": false }; 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 067c95156..5533d2994 100644 --- a/packages/houdini/src/codegen/generators/artifacts/tests/pagination.test.ts +++ b/packages/houdini/src/codegen/generators/artifacts/tests/pagination.test.ts @@ -781,6 +781,7 @@ test('cursor as scalar gets the right pagination query argument types', async fu }, "pluginData": {}, + "dedupe": "last", "input": { "fields": { diff --git a/packages/houdini/src/codegen/transforms/paginate.test.ts b/packages/houdini/src/codegen/transforms/paginate.test.ts index e29578132..4095b16f8 100644 --- a/packages/houdini/src/codegen/transforms/paginate.test.ts +++ b/packages/houdini/src/codegen/transforms/paginate.test.ts @@ -347,7 +347,7 @@ test('adds required fragment args to pagination query', async function () { // 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!) @dedupe { + query PaginatedFragment_Pagination_Query($snapshot: String!, $first: Int = 2, $after: String, $last: Int, $before: String, $id: ID!) { node(id: $id) { __typename id @@ -460,7 +460,7 @@ test('embeds pagination query as a separate document', async function () { // load the contents of the file expect(docs[1]?.document).toMatchInlineSnapshot(` - query UserFriends_Pagination_Query($first: Int = 10, $after: String) @dedupe { + query UserFriends_Pagination_Query($first: Int = 10, $after: String) { ...UserFriends_jrGTj @with(first: $first, after: $after) @mask_disable } @@ -1061,7 +1061,7 @@ test('query with forwards cursor paginate', async function () { // load the contents of the file expect(docs[0]?.document).toMatchInlineSnapshot(` - query Users($first: Int = 10, $after: String) { + query Users($first: Int = 10, $after: String) @dedupe { usersByForwardsCursor(first: $first, after: $after) @paginate { edges { node { @@ -1108,7 +1108,7 @@ test('query with custom first args', async function () { // load the contents of the file expect(docs[0]?.document).toMatchInlineSnapshot(` - query Users($limit: Int!, $after: String) { + query Users($limit: Int!, $after: String) @dedupe { usersByForwardsCursor(first: $limit, after: $after) @paginate { edges { node { @@ -1155,7 +1155,7 @@ test('query with backwards cursor paginate', async function () { // load the contents of the file expect(docs[0]?.document).toMatchInlineSnapshot(` - query Users { + query Users @dedupe { usersByBackwardsCursor(last: 10) @paginate { edges { node { @@ -1198,12 +1198,11 @@ test('query with offset paginate', async function () { // load the contents of the file expect(docs[0]?.document).toMatchInlineSnapshot(` - query Users($limit: Int = 10, $offset: Int) { + query Users($limit: Int = 10, $offset: Int) @dedupe { usersByOffset(limit: $limit, offset: $offset) @paginate { id } } - `) }) @@ -1230,7 +1229,7 @@ test('query with backwards cursor on full paginate', async function () { // load the contents of the file expect(docs[0]?.document).toMatchInlineSnapshot(` - query Users($first: Int, $after: String, $last: Int = 10, $before: String) { + query Users($first: Int, $after: String, $last: Int = 10, $before: String) @dedupe { usersByCursor(last: $last, first: $first, after: $after, before: $before) @paginate { edges { node { @@ -1277,7 +1276,7 @@ test('query with forwards cursor on full paginate', async function () { // load the contents of the file expect(docs[0]?.document).toMatchInlineSnapshot(` - query Users($first: Int = 10, $after: String, $last: Int, $before: String) { + query Users($first: Int = 10, $after: String, $last: Int, $before: String) @dedupe { usersByCursor(first: $first, after: $after, last: $last, before: $before) @paginate { edges { node { @@ -1324,7 +1323,7 @@ test("don't generate unsupported directions", async function () { // load the contents of the file expect(docs[0]?.document).toMatchInlineSnapshot(` - query Users($first: Int = 10, $after: String) { + query Users($first: Int = 10, $after: String) @dedupe { usersByForwardsCursor(first: $first, after: $after) @paginate { edges { node { @@ -1371,7 +1370,7 @@ test("forwards cursor paginated query doesn't overlap variables", async function // load the contents of the file expect(docs[0]?.document).toMatchInlineSnapshot(` - query Users($first: Int!, $after: String, $last: Int, $before: String) { + query Users($first: Int!, $after: String, $last: Int, $before: String) @dedupe { usersByCursor(first: $first, after: $after, last: $last, before: $before) @paginate { edges { node { @@ -1418,7 +1417,7 @@ test("backwards cursor paginated query doesn't overlap variables", async functio // load the contents of the file expect(docs[0]?.document).toMatchInlineSnapshot(` - query Users($last: Int!, $first: Int, $after: String, $before: String) { + query Users($last: Int!, $first: Int, $after: String, $before: String) @dedupe { usersByCursor(last: $last, first: $first, after: $after, before: $before) @paginate { edges { node { @@ -1461,12 +1460,11 @@ test("offset paginated query doesn't overlap variables", async function () { // load the contents of the file expect(docs[0]?.document).toMatchInlineSnapshot(` - query Users($limit: Int! = 10, $offset: Int) { + query Users($limit: Int! = 10, $offset: Int) @dedupe { usersByOffset(limit: $limit, offset: $offset) @paginate { id } } - `) }) From 46a56ee0027f6ef105a16c5d8bca3dbc69a68e9e Mon Sep 17 00:00:00 2001 From: Alec Aivazis Date: Wed, 11 Sep 2024 14:51:18 -0700 Subject: [PATCH 10/15] remove unused import --- packages/houdini/src/runtime/lib/pagination.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/houdini/src/runtime/lib/pagination.ts b/packages/houdini/src/runtime/lib/pagination.ts index 00a80da7c..107b63f96 100644 --- a/packages/houdini/src/runtime/lib/pagination.ts +++ b/packages/houdini/src/runtime/lib/pagination.ts @@ -1,5 +1,4 @@ import type { SendParams } from '../client/documentStore' -import { getCurrentConfig } from './config' import { deepEquals } from './deepEquals' import { countPage, extractPageInfo, missingPageSizeError } from './pageInfo' import { CachePolicy, DataSource } from './types' From 87d2f7029b6e3a9e9820f2125ae2ce2b978ed986 Mon Sep 17 00:00:00 2001 From: Alec Aivazis Date: Wed, 11 Sep 2024 15:18:47 -0700 Subject: [PATCH 11/15] document the directive --- site/src/routes/api/graphql-magic/+page.svx | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/site/src/routes/api/graphql-magic/+page.svx b/site/src/routes/api/graphql-magic/+page.svx index f76938b49..cdcbf3434 100644 --- a/site/src/routes/api/graphql-magic/+page.svx +++ b/site/src/routes/api/graphql-magic/+page.svx @@ -60,6 +60,12 @@ mutation UncompleteItem($id: ID!) { } ``` +### `@dedupe(cancelFirst: Booleam)` + +`@dedupe` lets you control wether or not multiple copies of the same operation (query or mutation) are allowed to run at the same time. +If you pass `true` for the `cancelFirst` argument then the first copy of the operation will be canceled (including any in-flight requests). +If you pass `false` (or pass nothing at all) then the second request won't trigger if there is already one pending. + ### `@when_not` `@when` provides a conditional under which the [list operation](#list-operations) should not be executed. It takes arguments that match the arguments of the field tagged with `@list` or `@paginate`. For more information, check out the [mutation docs](/api/mutation#lists) From 74f2ed2b76acc7856b3203f1e80dff31f01248af Mon Sep 17 00:00:00 2001 From: Alec Aivazis Date: Wed, 11 Sep 2024 15:55:22 -0700 Subject: [PATCH 12/15] Update packages/houdini/src/codegen/transforms/schema.ts Co-authored-by: Seppe Dekeyser --- packages/houdini/src/codegen/transforms/schema.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/houdini/src/codegen/transforms/schema.ts b/packages/houdini/src/codegen/transforms/schema.ts index 6387fdb67..6ee2b32f3 100644 --- a/packages/houdini/src/codegen/transforms/schema.ts +++ b/packages/houdini/src/codegen/transforms/schema.ts @@ -55,8 +55,8 @@ directive @${config.listPrependDirective} on FRAGMENT_SPREAD """ @${ config.dedupeDirective - } is used to prevent an operation from running twice. If the operation is sent - while there is already one pending and the cancelFirst arg is true, the first one will be canceled instead of the second. + } is used to prevent an operation from running more than once at the same time. + If the cancelFirst arg is set to true, the response already in flight will be canceled instead of the second one. """ directive @${config.dedupeDirective}(cancelFirst: Boolean) on QUERY | MUTATION From 29943feb548806fd166997f0c8bb504be428aa22 Mon Sep 17 00:00:00 2001 From: Alec Aivazis Date: Wed, 11 Sep 2024 15:56:11 -0700 Subject: [PATCH 13/15] rename variable --- packages/houdini/src/runtime/client/documentStore.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/houdini/src/runtime/client/documentStore.ts b/packages/houdini/src/runtime/client/documentStore.ts index 9e3b6c37a..4414ec9de 100644 --- a/packages/houdini/src/runtime/client/documentStore.ts +++ b/packages/houdini/src/runtime/client/documentStore.ts @@ -24,7 +24,7 @@ const steps = { backwards: ['end', 'afterNetwork'], } as const -let inFlightPageFetches: Record = {} +let inflightRequests: Record = {} export class DocumentStore< _Data extends GraphQLObject, @@ -143,13 +143,13 @@ export class DocumentStore< // controller for the document if ('dedupe' in this.artifact) { // if there is already a pending request - if (inFlightPageFetches[this.controllerKey(variables)]) { + if (inflightRequests[this.controllerKey(variables)]) { // we have to abort _something_ if (this.artifact.dedupe === 'first') { // cancel the existing one - inFlightPageFetches[this.controllerKey(variables)].abort() + inflightRequests[this.controllerKey(variables)].abort() // and register the new one - inFlightPageFetches[this.controllerKey(variables)] = abortController + inflightRequests[this.controllerKey(variables)] = abortController } // otherwise we have to abort this one else { @@ -158,7 +158,7 @@ export class DocumentStore< } // register this abort controller as being in flight else { - inFlightPageFetches[this.controllerKey(variables)] = abortController + inflightRequests[this.controllerKey(variables)] = abortController } } @@ -222,7 +222,7 @@ export class DocumentStore< const response = await promise // after the whole plugin chain, we need to clean up the in flight tracking - delete inFlightPageFetches[this.controllerKey(variables)] + delete inflightRequests[this.controllerKey(variables)] // we're done return response From cdbce0f1169358a7e68aecf6ad200da723744ef6 Mon Sep 17 00:00:00 2001 From: Alec Aivazis Date: Wed, 11 Sep 2024 15:57:51 -0700 Subject: [PATCH 14/15] remove route in kit route config --- e2e/kit/src/lib/utils/routes.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/e2e/kit/src/lib/utils/routes.ts b/e2e/kit/src/lib/utils/routes.ts index 6f1bf3a87..2d7f66bf0 100644 --- a/e2e/kit/src/lib/utils/routes.ts +++ b/e2e/kit/src/lib/utils/routes.ts @@ -92,7 +92,6 @@ export const routes = { Pagination_query_bidirectional_cursor: '/pagination/query/bidirectional-cursor', Pagination_query_bidirectional_cursor_single_page: '/pagination/query/bidirectional-cursor-single-page', - Pagination_query_dedupe_pagination_fetch: 'pagination/query/dedupe-pagination-fetch', Pagination_query_offset: '/pagination/query/offset', Pagination_query_offset_single_page: '/pagination/query/offset-single-page', Pagination_query_offset_variable: '/pagination/query/offset-variable', From c4a959a3788de437af0e6757da34dea20f979f69 Mon Sep 17 00:00:00 2001 From: Alec Aivazis Date: Wed, 11 Sep 2024 16:03:30 -0700 Subject: [PATCH 15/15] update snapshot --- .../generators/definitions/schema.test.ts | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/houdini/src/codegen/generators/definitions/schema.test.ts b/packages/houdini/src/codegen/generators/definitions/schema.test.ts index ffea0c9f4..d2a2242ad 100644 --- a/packages/houdini/src/codegen/generators/definitions/schema.test.ts +++ b/packages/houdini/src/codegen/generators/definitions/schema.test.ts @@ -37,8 +37,8 @@ test('adds internal documents to schema', async function () { directive @prepend on FRAGMENT_SPREAD """ - @dedupe is used to prevent an operation from running twice. If the operation is sent - while there is already one pending and the cancelFirst arg is true, the first one will be canceled instead of the second. + @dedupe is used to prevent an operation from running more than once at the same time. + If the cancelFirst arg is set to true, the response already in flight will be canceled instead of the second one. """ directive @dedupe(cancelFirst: Boolean) on QUERY | MUTATION @@ -135,8 +135,8 @@ test('list operations are included', async function () { directive @prepend on FRAGMENT_SPREAD """ - @dedupe is used to prevent an operation from running twice. If the operation is sent - while there is already one pending and the cancelFirst arg is true, the first one will be canceled instead of the second. + @dedupe is used to prevent an operation from running more than once at the same time. + If the cancelFirst arg is set to true, the response already in flight will be canceled instead of the second one. """ directive @dedupe(cancelFirst: Boolean) on QUERY | MUTATION @@ -252,8 +252,8 @@ test('list operations are included but delete directive should not be in when we directive @prepend on FRAGMENT_SPREAD """ - @dedupe is used to prevent an operation from running twice. If the operation is sent - while there is already one pending and the cancelFirst arg is true, the first one will be canceled instead of the second. + @dedupe is used to prevent an operation from running more than once at the same time. + If the cancelFirst arg is set to true, the response already in flight will be canceled instead of the second one. """ directive @dedupe(cancelFirst: Boolean) on QUERY | MUTATION @@ -382,8 +382,8 @@ test("writing twice doesn't duplicate definitions", async function () { directive @prepend on FRAGMENT_SPREAD """ - @dedupe is used to prevent an operation from running twice. If the operation is sent - while there is already one pending and the cancelFirst arg is true, the first one will be canceled instead of the second. + @dedupe is used to prevent an operation from running more than once at the same time. + If the cancelFirst arg is set to true, the response already in flight will be canceled instead of the second one. """ directive @dedupe(cancelFirst: Boolean) on QUERY | MUTATION