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(vue-query): avoid use sync watchers that cause frequent requests #6043

Merged
merged 2 commits into from
Sep 27, 2023

Conversation

Mini-ghost
Copy link
Contributor

@Mini-ghost Mini-ghost commented Sep 21, 2023

Linked issue

resolves #5996

Description

Starting from v4.34.3, there is an issue where the queryFn is erroneously triggered under the following conditions:

  1. When the enabled property is set to false, the queryFn should not be called, but the following code still triggers it:
import { computed, ref } from 'vue';
import { useRoute } from 'vue-router';
import { useQuery } from '@tanstack/vue-query';

const route = useRoute();

const postId = computed(() => ({
  id: route.params.id ? Number(route.params.id) : undefined,
}));

const enabled = computed(() => {
  return !!postId.value.id && postId.value.id < 10;
});

const { data } = useQuery({
  queryKey: ['POST', postId],
  queryFn() {
    return Promise.resolve(Date.now());
  },
  enabled,
});

reproduce: https://stackblitz.com/edit/vitejs-vite-q1vdvf?file=src%2Fpages%2FPost.vue

  1. Modifying the queryKey in the following way causes the queryFn to be called multiple times:
import { ref } from 'vue';
import { useQuery } from '@tanstack/vue-query';

const params = ref({
  page: 1,
  perPage: 20,
});

const { data } = useQuery({
  queryKey: ['PAGE', params],
  queryFn() {
    return Promise.resolve(Date.now());
  },
});

const onReset = () => {
  params.value.page += 1;
  params.value.perPage += 1;
};

reproduce: https://stackblitz.com/edit/vitejs-vite-j5dldg?file=src%2FApp.vue

The reason for this behavior is that when the watch configuration is set to { flush: 'sync' }, the watch callback is synchronously invoked every time there is a modification.

You can find the relevant code in the source file here:

watch(
defaultedOptions,
() => {
observer.setOptions(defaultedOptions.value)
updateState(state, observer.getCurrentResult())
},
{ flush: 'sync' },
)

And this piece of code was introduced at that time to address this issue: #5910

So, this PR attempts to avoid using { flush: 'sync' } and instead, it rewraps refetch to ensure that the observer is correctly updated.

@vercel
Copy link

vercel bot commented Sep 21, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
query ⬜️ Ignored (Inspect) Visit Preview Sep 27, 2023 0:53am

@nx-cloud
Copy link

nx-cloud bot commented Sep 21, 2023

☁️ Nx Cloud Report

CI is running/has finished running commands for commit d750bd1. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch


✅ Successfully ran 2 targets

Sent with 💌 from NxCloud.

@Mini-ghost Mini-ghost marked this pull request as ready for review September 22, 2023 14:36
@codesandbox-ci
Copy link

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit d750bd1:

Sandbox Source
@tanstack/query-example-react-basic-typescript Configuration
@tanstack/query-example-solid-basic-typescript Configuration
@tanstack/query-example-svelte-basic Configuration
@tanstack/query-example-vue-basic Configuration

@codecov-commenter
Copy link

Codecov Report

All modified lines are covered by tests ✅

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Files Coverage Δ
packages/vue-query/src/useBaseQuery.ts 96.20% <100.00%> (ø)
packages/vue-query/src/useInfiniteQuery.ts 100.00% <100.00%> (ø)
packages/vue-query/src/useIsFetching.ts 92.85% <100.00%> (ø)
packages/vue-query/src/useIsMutating.ts 92.85% <100.00%> (ø)
packages/vue-query/src/useMutation.ts 93.47% <100.00%> (ø)
packages/vue-query/src/useQuery.ts 100.00% <100.00%> (ø)

... and 9 files with indirect coverage changes

📢 Thoughts on this report? Let us know!.

@DamianOsipiuk DamianOsipiuk merged commit da79651 into TanStack:main Sep 27, 2023
@DamianOsipiuk
Copy link
Contributor

@Mini-ghost could you get this to rc branch as well?

@Mini-ghost
Copy link
Contributor Author

@DamianOsipiuk Sure! I will give it a try in the next few days.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Sometimes vue-query doesn't respect enabled
3 participants