-
Notifications
You must be signed in to change notification settings - Fork 1.4k
fix(react-query): ensure mutationKey is an array #7282
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
react query will require all keys be an array. see: TanStack/query#2919
🦋 Changeset detectedLatest commit: 87ff406 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/theguild/graphql-code-generator/34EkdZ22rJELmy7Mqx7csziyW9ji |
@jonathanstanley thanks !! can you please run ^ this should fix ci and we can merge and release |
@dotansimha I completed those commands as you asked. I'm not entirely certain if this should be considered a PATCH or a MINOR. Although the change is backwards compatible with react-query, it is possible some have hardcoded their systems to expect a key of |
I agree with you, this change might be breaking for people directly on The best solution here might be to add a new plugin option WDYT? |
Thanks @charlypoly . I could see it both ways. A flag to |
@jonathanstanley let's proceed with a minor bump on the plugin |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jonathanstanley some tests are failing because snapshots had not been updated 👀
cf: https://jestjs.io/docs/snapshot-testing#updating-snapshots
can you take it from here? i'm not familiar with this package build/ci processes |
@jonathanstanley The unit-tests were not updated, cf: 87ff406 The test setup is pretty simple to run locally, you just need to do the following (at the root of the project): yarn
yarn build
yarn test packages/plugins/typescript/react-query # will fail
# fix tests
yarn test --updateSnapshot packages/plugins/typescript/react-query
yarn test packages/plugins/typescript/react-query # should pass! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good to go 🚀
fix: react query is requiring all keys to be an array
Description
Related #7281 react query will require all keys be an array. see: TanStack/query#2919
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
The change is simple and backwards compatible because arrays have always been valid keys.
Checklist: