fix(vue-query): mark the watcher as post flush #7942

RomainLanz · 2024-08-22T11:53:44Z

Hey there! 👋🏻

This PR changes the watcher mode for useQueries from sync to post.
The change is needed to avoid having the watcher run during component unmount.

Note

This change will probably be needed on useBaseQuery, but I haven't tested it yet.

Related VueJS issues:

watcher for a route param is called when a user leaved the route vuejs/core#2291
watch runs while the component is being unmounted vuejs/core#7030
route change should not be changing state until the previous component has been unmounted vuejs/router#1945
route change should not be changing state until the previous component has been unmounted vuejs/vue-router#3393
Leaving the route causes the trigger parameter to change to undefined posva/unplugin-vue-router#86

Currently, using the route params to define some query params triggers a new fetch when leaving a route.

For example, let's say we have the following code that runs when we are on /players/:id.

const route = useRoute();
const paramId = computed(() => +route.params.id);

const queries = useMyViewData(paramId)

When going from /players to /players/1, TanStack Query will trigger a fetch for Player 1 (expected).
When going from /players/1 to /players, TanStack Query will trigger a fetch for Player undefined (unexpected).
When going from /players/1 to /transfers/12345, TanStack Query will trigger a fetch for Player 12345 (unexpected).

This is due to the watcher running when the page changes.

I haven't created an easy and sharable reproduction. Let me know if it is needed.

RomainLanz · 2024-08-23T09:24:15Z

pnpm patch for the one who wants to test it:

diff --git a/build/modern/useQueries.js b/build/modern/useQueries.js
index c172ecd85df4646ab4869411725cc028ccc0e7e1..ec584988a4ecb9f24c073eb94a4b5cbc9e8eb255 100644
--- a/build/modern/useQueries.js
+++ b/build/modern/useQueries.js
@@ -81,7 +81,7 @@ function useQueries({
       );
       state.value = getCombinedResultPersisted();
     },
-    { flush: "sync" }
+    { flush: "post" }
   );
   onScopeDispose(() => {
     unsubscribe();

RomainLanz · 2024-08-28T09:29:29Z

We deployed this patch to production on Monday and found no issues for the moment.

nx-cloud · 2024-08-30T11:25:26Z

☁️ Nx Cloud Report

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

📂 See all runs for this CI Pipeline Execution

✅ Successfully ran 2 targets

Sent with 💌 from NxCloud.

pkg-pr-new · 2024-08-30T11:26:39Z

commit: c6443c4

@tanstack/angular-query-devtools-experimental

pnpm add https://pkg.pr.new/@tanstack/angular-query-devtools-experimental@7942

@tanstack/angular-query-experimental

pnpm add https://pkg.pr.new/@tanstack/angular-query-experimental@7942

@tanstack/eslint-plugin-query

pnpm add https://pkg.pr.new/@tanstack/eslint-plugin-query@7942

@tanstack/query-async-storage-persister

pnpm add https://pkg.pr.new/@tanstack/query-async-storage-persister@7942

@tanstack/query-broadcast-client-experimental

pnpm add https://pkg.pr.new/@tanstack/query-broadcast-client-experimental@7942

@tanstack/query-core

pnpm add https://pkg.pr.new/@tanstack/query-core@7942

@tanstack/query-devtools

pnpm add https://pkg.pr.new/@tanstack/query-devtools@7942

@tanstack/query-persist-client-core

pnpm add https://pkg.pr.new/@tanstack/query-persist-client-core@7942

@tanstack/query-sync-storage-persister

pnpm add https://pkg.pr.new/@tanstack/query-sync-storage-persister@7942

@tanstack/react-query

pnpm add https://pkg.pr.new/@tanstack/react-query@7942

@tanstack/react-query-devtools

pnpm add https://pkg.pr.new/@tanstack/react-query-devtools@7942

@tanstack/react-query-next-experimental

pnpm add https://pkg.pr.new/@tanstack/react-query-next-experimental@7942

@tanstack/react-query-persist-client

pnpm add https://pkg.pr.new/@tanstack/react-query-persist-client@7942

@tanstack/solid-query

pnpm add https://pkg.pr.new/@tanstack/solid-query@7942

@tanstack/solid-query-devtools

pnpm add https://pkg.pr.new/@tanstack/solid-query-devtools@7942

@tanstack/solid-query-persist-client

pnpm add https://pkg.pr.new/@tanstack/solid-query-persist-client@7942

@tanstack/svelte-query

pnpm add https://pkg.pr.new/@tanstack/svelte-query@7942

@tanstack/svelte-query-devtools

pnpm add https://pkg.pr.new/@tanstack/svelte-query-devtools@7942

@tanstack/svelte-query-persist-client

pnpm add https://pkg.pr.new/@tanstack/svelte-query-persist-client@7942

@tanstack/vue-query

pnpm add https://pkg.pr.new/@tanstack/vue-query@7942

@tanstack/vue-query-devtools

pnpm add https://pkg.pr.new/@tanstack/vue-query-devtools@7942

Open in Stackblitz

More templates

codecov · 2024-08-30T11:30:13Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 71.42%. Comparing base (c7fb5fd) to head (c6443c4).
Report is 28 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #7942       +/-   ##
===========================================
+ Coverage   44.64%   71.42%   +26.78%     
===========================================
  Files         187       19      -168     
  Lines        7090      462     -6628     
  Branches     1575      119     -1456     
===========================================
- Hits         3165      330     -2835     
+ Misses       3562      102     -3460     
+ Partials      363       30      -333

Components	Coverage Δ
@tanstack/angular-query-devtools-experimental	`∅ <ø> (∅)`
@tanstack/angular-query-experimental	`∅ <ø> (∅)`
@tanstack/eslint-plugin-query	`∅ <ø> (∅)`
@tanstack/query-async-storage-persister	`∅ <ø> (∅)`
@tanstack/query-broadcast-client-experimental	`∅ <ø> (∅)`
@tanstack/query-codemods	`∅ <ø> (∅)`
@tanstack/query-core	`∅ <ø> (∅)`
@tanstack/query-devtools	`∅ <ø> (∅)`
@tanstack/query-persist-client-core	`∅ <ø> (∅)`
@tanstack/query-sync-storage-persister	`∅ <ø> (∅)`
@tanstack/react-query	`∅ <ø> (∅)`
@tanstack/react-query-devtools	`∅ <ø> (∅)`
@tanstack/react-query-next-experimental	`∅ <ø> (∅)`
@tanstack/react-query-persist-client	`∅ <ø> (∅)`
@tanstack/solid-query	`∅ <ø> (∅)`
@tanstack/solid-query-devtools	`∅ <ø> (∅)`
@tanstack/solid-query-persist-client	`∅ <ø> (∅)`
@tanstack/svelte-query	`∅ <ø> (∅)`
@tanstack/svelte-query-devtools	`∅ <ø> (∅)`
@tanstack/svelte-query-persist-client	`∅ <ø> (∅)`
@tanstack/vue-query	`71.42% <ø> (ø)`
@tanstack/vue-query-devtools	`∅ <ø> (∅)`

DamianOsipiuk · 2024-08-30T17:33:03Z

We could potentially remove sync watcher entirely.
We did it for other hooks some time ago.

#6043

RomainLanz · 2024-09-02T07:34:30Z

I'm not entirely sure about the implications either. If removing this watch statement is the only change, I can create another patch and test how my application behaves.

DamianOsipiuk · 2024-09-02T08:01:55Z

You would also need to override refetch to update observer options before actual refetch is called.

RomainLanz · 2024-09-02T09:49:31Z

Then we could merge that one first and work towards removing all watchers in a follow-up PR/issue.

DamianOsipiuk · 2024-09-03T20:35:08Z

I do not think merging this and immediately changing solution in a follow-up is the way to go.
Better way would be to fix it properly and add automated test that verifies the fix.
For the mean time, you can patch the package locally, as you already did.

RomainLanz · 2024-10-16T09:08:38Z

I might be misunderstanding, but I think we’re talking about different things.

This PR is updating a watcher for the useQueries hook, and there’s no refetch concept involved at this level. The refetch system is handled within the underlying QueriesObserver layer.

RomainLanz · 2024-11-25T10:50:05Z

Could someone review this again?

This issue affects all Vue applications using useQueries. It's a significant problem that shouldn't be disregarded.

DamianOsipiuk · 2024-11-25T11:12:02Z

Could someone review this again?

This issue affects all Vue applications using useQueries. It's a significant problem that shouldn't be disregarded.

As mentioned before i do not think this change should be merged as it will introduce another bug in specific cases where states would not be properly synced up when calling refetch.

There is a similar PR that handled that for other hooks, and this MR should be adjusted to follow similar pattern. #6043

In that MR you could see additional block that patches refetch function returned from observer - https://github.com/TanStack/query/pull/6043/files#diff-996721944b77ab3155cb5f86364575311132aeaaff8f7f715755bfcc9fc09108R124-R128

Mini-ghost · 2024-12-17T12:42:29Z

I have opened a PR(#8443) to address this issue. If there are any areas where I may have overlooked something, please kindly let me know.

Thank you!

fix(vue:queries): mark the watcher as post flush

c6443c4

github-actions bot added the package: vue-query label Aug 22, 2024

RomainLanz changed the title ~~fix(vue:queries): mark the watcher as post flush~~ fix(vue-query): mark the watcher as post flush Aug 29, 2024

Mini-ghost mentioned this pull request Dec 17, 2024

fix(vue-query): avoid use sync for watch that cause unnecessary requests #8443

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(vue-query): mark the watcher as post flush #7942

fix(vue-query): mark the watcher as post flush #7942

RomainLanz commented Aug 22, 2024 •

edited

Loading

RomainLanz commented Aug 23, 2024

RomainLanz commented Aug 28, 2024

nx-cloud bot commented Aug 30, 2024 •

edited

Loading

pkg-pr-new bot commented Aug 30, 2024

codecov bot commented Aug 30, 2024

DamianOsipiuk commented Aug 30, 2024

RomainLanz commented Sep 2, 2024

DamianOsipiuk commented Sep 2, 2024

RomainLanz commented Sep 2, 2024

DamianOsipiuk commented Sep 3, 2024

RomainLanz commented Oct 16, 2024

RomainLanz commented Nov 25, 2024

DamianOsipiuk commented Nov 25, 2024

Mini-ghost commented Dec 17, 2024

fix(vue-query): mark the watcher as post flush #7942

Are you sure you want to change the base?

fix(vue-query): mark the watcher as post flush #7942

Conversation

RomainLanz commented Aug 22, 2024 • edited Loading

RomainLanz commented Aug 23, 2024

RomainLanz commented Aug 28, 2024

nx-cloud bot commented Aug 30, 2024 • edited Loading

☁️ Nx Cloud Report

pkg-pr-new bot commented Aug 30, 2024

codecov bot commented Aug 30, 2024

Codecov Report

DamianOsipiuk commented Aug 30, 2024

RomainLanz commented Sep 2, 2024

DamianOsipiuk commented Sep 2, 2024

RomainLanz commented Sep 2, 2024

DamianOsipiuk commented Sep 3, 2024

RomainLanz commented Oct 16, 2024

RomainLanz commented Nov 25, 2024

DamianOsipiuk commented Nov 25, 2024

Mini-ghost commented Dec 17, 2024

RomainLanz commented Aug 22, 2024 •

edited

Loading

nx-cloud bot commented Aug 30, 2024 •

edited

Loading