HoudiniGraphql / houdini

The disappearing GraphQL framework
http://www.houdinigraphql.com
MIT License
913 stars 99 forks source link

Only dedupe pagination queries if the variables match #1378

Closed AlecAivazis closed 5 hours ago

AlecAivazis commented 4 weeks ago

This PR adds a new argument to the @dedupe directive: match which takes 3 values: Variables, Operation, and None. This gives the user more fine-grained control over how the deduping happens. Most importantly, for paginated queries, @dedupe comes with match set to Variables.

To help everyone out, please make sure your PR does the following:

changeset-bot[bot] commented 4 weeks ago

🦋 Changeset detected

Latest commit: d401396419522605e610d1301a0d945ce7b9a86f

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 8 packages | Name | Type | | ----------------------------------- | ----- | | houdini | Patch | | houdini-adapter-auto | Patch | | houdini-adapter-cloudflare | Patch | | houdini-adapter-node | Patch | | houdini-adapter-static | Patch | | houdini-react | Patch | | houdini-svelte | Patch | | houdini-plugin-svelte-global-stores | Patch |

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

netlify[bot] commented 4 weeks ago

Deploy Preview for houdini-docs-next ready!

Name Link
Latest commit 573fc7f07b0cf26a8edcb80825917f62c054b496
Latest deploy log https://app.netlify.com/sites/houdini-docs-next/deploys/6740c8e10ab6fe000889e460
Deploy Preview https://deploy-preview-1378--houdini-docs-next.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

netlify[bot] commented 4 weeks ago

Deploy Preview for houdinigraphql ready!

Name Link
Latest commit 573fc7f07b0cf26a8edcb80825917f62c054b496
Latest deploy log https://app.netlify.com/sites/houdinigraphql/deploys/6740c8e197f17b000809c162
Deploy Preview https://deploy-preview-1378--houdinigraphql.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

half2me commented 2 weeks ago

Can we also either have the option to opt in/out of dedupe behaviour? Or at least disable it for SSR? Currently all my tests are failing when running in parallel mode, because when multiple requests hit my endpoint that uses a @loading directive it throws a 500 with Aborted.

AlecAivazis commented 2 weeks ago

@half2me it should only be automatically deduping pagination queries. are you seeing something different?

half2me commented 2 weeks ago

@half2me it should only be automatically deduping pagination queries. are you seeing something different?

Yes I am. Since I upgraded to this, a lot of tests started failing with the server returning status 500 Aborted error, for any page where I'm using pagination.

This definitely has something to do with multiple requests hitting the same page, since if I disable parallel workers for my tests, and run them just one at a time, they all work fine. Its only when simultaneous requests hit the same page that they break.,

AlecAivazis commented 2 weeks ago

@SeppahBaws i think i fixed what was wrong

@half2me i added a new config value supressPaginationDeduplication that you can set to prevent the deduplication

half2me commented 2 weeks ago

Amazing, I will rerun the tests and let you know. @AlecAivazis is there an easy way I can pin the houdini package in my package.json to point to this branch?

SeppahBaws commented 2 weeks ago

Ah awesome, I'll give this a shot once it's calmed down a bit more at work...

@half2me you can clone and build the repo locally, and then link it locally to your repo with npm/pnpm link: https://docs.npmjs.com/cli/v6/commands/npm-link https://pnpm.io/cli/link

(on npm you can also specify the package version as "file:../path/to/houdini/packages/houdini" - I did find that this messes up in pnpm though)