Skip to content
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

fix: avoid redundant refetchQueries call for mutation with no-cache policy #11515

Merged
merged 7 commits into from Jan 31, 2024

Conversation

vladar
Copy link
Contributor

@vladar vladar commented Jan 23, 2024

This is a quick small fix for the bug outlined in #10238

In the nutshell, mutation.refetchQueries below is an empty array, not boolean or undefined. So this check errorneously evaluates to true even when the refetchQueries option is not set for the mutation.

The fix in this PR is a bit defensive - in case if it could be actually something other than array.

if (
cacheWrites.length > 0 ||
mutation.refetchQueries ||
mutation.update ||
mutation.onQueryUpdated ||
mutation.removeOptimistic
) {

Fixes #10238

Copy link

netlify bot commented Jan 23, 2024

‼️ Deploy request for apollo-client-docs rejected.

Name Link
🔨 Latest commit 35e9f36

Copy link

changeset-bot bot commented Jan 23, 2024

🦋 Changeset detected

Latest commit: 35e9f36

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

This PR includes changesets to release 1 package
Name Type
@apollo/client 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

Copy link
Member

@phryneas phryneas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, this is a very good catch, thank you for that!

I've made some suggestions that would keep the change in bundle size down a bit - what do you think?

src/core/QueryManager.ts Outdated Show resolved Hide resolved
src/core/QueryManager.ts Outdated Show resolved Hide resolved
vladar and others added 2 commits January 24, 2024 14:01
Co-authored-by: Lenz Weber-Tronic <mail@lenzw.de>
Co-authored-by: Lenz Weber-Tronic <mail@lenzw.de>
@vladar
Copy link
Contributor Author

vladar commented Jan 24, 2024

Hi, this is a very good catch, thank you for that!

I've made some suggestions that would keep the change in bundle size down a bit - what do you think?

Updated, thanks. But looks like it doesn't help with size check.

@phryneas
Copy link
Member

Don't worry about the size check - I'll fix that up before merging :)

This looks good to me. We're currently in a code freeze, so this will probably be released in a 3.9.1 after we've released our 3.9 minor.

@vladar vladar requested a review from a team as a code owner January 31, 2024 16:05
Copy link
Member

@jerelmiller jerelmiller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉 Now with 3.9 out, we'll get this into 3.9.1 🙂

@jerelmiller jerelmiller merged commit c9bf93b into apollographql:main Jan 31, 2024
29 checks passed
@github-actions github-actions bot mentioned this pull request Jan 31, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Mutations with fetchPolicy: 'no-cache' still trigger refetchQueries
3 participants