constant-blue

Batching invalidation updates

Hey there ! Given the following code :

const mutation = useMutation({
  mutationFn: async () => {},
  onSuccess: async () => {
    await Promise.all([
      queryClient.invalidateQueries({
        // This query has hardcoded 1s execution time
        queryKey: letterQueryOption().queryKey,
      }),
      queryClient.invalidateQueries({
        // This query has hardcoded 2s execution time
        queryKey: numberQueryOption().queryKey,
      }),
    ]);
  },
});

const mutation = useMutation({
  mutationFn: async () => {},
  onSuccess: async () => {
    await Promise.all([
      queryClient.invalidateQueries({
        // This query has hardcoded 1s execution time
        queryKey: letterQueryOption().queryKey,
      }),
      queryClient.invalidateQueries({
        // This query has hardcoded 2s execution time
        queryKey: numberQueryOption().queryKey,
      }),
    ]);
  },
});

` The 1st revalidating query takes 1s to refetch, the 2nd takes 2 seconds At the moment, when executing the mutation, The UI updates 2 times : after 1s, and after 2s. Is there a way to "batch" the things wrapped in Promise.all to only render the UI after the longest invalidation is done ? For some context : I recently migrated my Remix app to React Query, and "page stability" is the only thing that's missing from what I had with Remix 🙂 (Also, it's going to be one thing I'll be talking about at React Paris, so I'd like to make sure I'm not saying stupid things ahah!)

36 Replies

rival-black•9mo ago

It's not the invalidation that takes that long, it's the query refetch right? Mostly in that case, the simplist option would be to to put the long and short fetch inside the queryFn to begin with:

const letterAndQueryOption = queryOptions({
  queryKey: ['letter-number']
  queryFn: async () => {
    return Promise.all([
      // takes 1s
      fetchLetter(),
      // takes 2s
      fetchNumber(),
    ]);
  }
});

const letterAndQueryOption = queryOptions({
  queryKey: ['letter-number']
  queryFn: async () => {
    return Promise.all([
      // takes 1s
      fetchLetter(),
      // takes 2s
      fetchNumber(),
    ]);
  }
});

Beyond that, I'm assuming that you have issues with getting: UI(old letter data, old number data) -> UI(new letter data, old number data) -> UI(new letter data, new number data). I've a few ideas in my head that could help you, but I'd like to see the comparison between the behavior you see in Remix loading vs what your trying to do with query. (This problem is usually tackled at the api layer with graphql or https://trpc.io/docs/client/links/httpBatchLink)

rival-black•9mo ago

Oh you could try this: https://tanstack.com/query/latest/docs/reference/notifyManager#notifymanagerbatch

NotifyManager | TanStack Query Docs

The notifyManager handles scheduling and batching callbacks in Tanstack Query. It exposes the following methods: notifyManager.batch batch can be used to batch all updates scheduled inside the passed...

rival-black•9mo ago

Await the promise.all inside the callback to batch and see if that works

constant-blueOP•9mo ago

No louck with notifyManager as well, not sure I'm using it correctly Indeed I am talking about the background refetch. Maybe there could be some workaround using refetchType: 'none' in invalidateQueries, and then manually doing something else manually ?

constant-blueOP•9mo ago

It's the only thing I miss from remix, but hard to get right. In Remix, when an action is called, the router finds all matching routes and fetchers, and calls them in //. Then, when all are in a done state, it "commits" the batched updates at once, and its also integrated with the navigation so the experience is really good - no "flickering" of refreshing caches

rival-black•9mo ago

Yeah, I can't get anything working on that. In your position I'd be roasting query as well lol. Good luck with the talk.

constant-blueOP•9mo ago

Oh overall i'm very happy with the migration ! I got way more control over data fetching which is what I wanted. Just this specific issue that's not as good but it's fine

flat-fuchsia•9mo ago

I think predicate solves the problem for you

await queryClient.invalidateQueries({
  predicate: ({ queryKey }) =>
    queryKey === letterQueryOption().queryKey || queryKey === numberQueryOption().queryKey
})

await queryClient.invalidateQueries({
  predicate: ({ queryKey }) =>
    queryKey === letterQueryOption().queryKey || queryKey === numberQueryOption().queryKey
})

Oh never mind I bet it doesn’t because your problem is the observer updates after the fetches complete. Nicer than two invalidates with Promise.all though I guess

rising-crimson•9mo ago

I don't know of a straight forward of doing this but I guess you could achieve this by setting letterQuery notifyOnChangeProps to an empty array, this way component won't be notified of the change in that query but when numberQuery is done refetching, component will be notified and will rerender. queries should be in the same component and you can only do this if you're sure numberQuery takes longer.

rival-black•9mo ago

This might be worth creating an issue honestly.

flat-fuchsia•9mo ago

Idk this seems more like a feature request than a bug. "Don't update subscribers until every queryFn completes"

rival-black•9mo ago

Something like we update the cache but don't fire the observers until the whole group is finished.

flat-fuchsia•9mo ago

Right. I could see that being useful in some scenarios. A "synchronized" interface or in a rare scenario of expensive renders, trying to prevent a render that's about to get rendered again when fetch n+1 finishes or something

rival-black•9mo ago

There's notifyOnChangeProps already so something simiar might be possible I might read the code for a bit to see what coudl be done, at the very least there shold be a test suite for this.

constant-blueOP•9mo ago

Sorry you all, I forgot to check discord last week. I think of it as the "atomic" part of a database transaction. I thought maybe it could be something like this

useQuery({
  queryFn: () => {...},
  onSuccess: atomic(async () => {
    await queryClient.invalidateQueries(...)
    await queryClient.invalidateQueries(...)
  })
})

useQuery({
  queryFn: () => {...},
  onSuccess: atomic(async () => {
    await queryClient.invalidateQueries(...)
    await queryClient.invalidateQueries(...)
  })
})

Probably updating the cache but not firing the observers is a good approach, as a users that seems like it could work I tried to explain the use case at the conference but I don't know if it was clear - I think Dominik saw my talk, so maybe he has opinions as well ^^ I'll create a feature request on github when I get home if that how it should be done, thanks you all! Thinking of it, there may be tricky edge cases like : what would happen if some other code also invalidates the query, without beeing in the same "atomic" group - would this triggers subscribers ?

rival-black•9mo ago

Let me know when your talk and the issue is live. Also I missed React paris because I'm going to React amsterdam instead so if you're there as well I'll say hi.

constant-blueOP•9mo ago

Hey @DogPawHat , my talk is online : https://www.youtube.com/watch?v=i1cVfJz4Vlw&t=648s&ab_channel=BeJS It was my first time speaking in front of people so not super smooth but the idea is here ^^ I had the feedback that the way I use remix + react query aligns very much with tanstack router / start, so maybe I'll also migrate to that later. One step at the time

BeJS

YouTube

Using Remix, the wrong ? way - Antoine Chalifour

Modern frameworks are great, and they come with many features and opinions. But what happens when you started a project with one of those frameworks, but you no longer agree with some of their opinions ? Let's explore the tradeoffs of Remix: - how actions and loaders work, data revalidation lifecycle - how form submission work with remix - h...

constant-blueOP•9mo ago

Also, I started a discussion on github : https://github.com/TanStack/query/discussions/8863

GitHub

"Atomic" cache invalidation for more UI stability · TanStack query...

Hi everyone! I recently migrated parts of my Remix app from loaders + actions to React query. I'm pretty happy with the migration since I got more control of caching and I can invalidate only s...

foreign-sapphire•9mo ago

I think we can't really stop data from being written to the cache, doing something with observers seems like the better way (like not informing them) @Antoine have you tried setting notifyManager.setScheduler to something that waits until all promises have been settled ? I'm guessing maybe this?

const synced = async (getPromise: () => Promise<unknown>) => {
  const promise = getPromise()
  notifyManager.setScheduler(async (cb) => {
    await promise
    cb()
    notifyManager.setScheduler(undefined)
  })
}

const synced = async (getPromise: () => Promise<unknown>) => {
  const promise = getPromise()
  notifyManager.setScheduler(async (cb) => {
    await promise
    cb()
    notifyManager.setScheduler(undefined)
  })
}

note that we don't currently allow setting the scheduler to undefined, but I think we should, and it should indicate that it goes back to the default scheduler .... or maybe we should even get the previous scheduler (also not exposed at the moment) probably needs a try/catch around awaiting the promise. let me know if that works 🙂

constant-blueOP•9mo ago

Just tried, it works ! The only change I made was to reset to the default scheduler

const synced = async (getPromise: () => Promise<unknown>) => {
  const promise = getPromise();
  notifyManager.setScheduler(async (cb) => {
    await promise;
    cb();
    notifyManager.setScheduler((cb) => setTimeout(cb, 0));
  });
};

const synced = async (getPromise: () => Promise<unknown>) => {
  const promise = getPromise();
  notifyManager.setScheduler(async (cb) => {
    await promise;
    cb();
    notifyManager.setScheduler((cb) => setTimeout(cb, 0));
  });
};

I wonder if it means I can also better integrate it we react router since they now expose promises for fetchers and the navigate function something like

synced(() => {
  await Promise.all([
    invalidateQueries(),
    navigate('/other-page')
  ])
})

synced(() => {
  await Promise.all([
    invalidateQueries(),
    navigate('/other-page')
  ])
})

To update the UI after the router is done updating

rival-black•9mo ago

Well dang. Guess all we need then is new docs? Maybe the helper above as a more user friendly abstraction?

foreign-sapphire•9mo ago

this is great! you need to gracefully catch errors around await promise though; otherwise, you won't reset the scheduler

notifyManager.setScheduler((cb) => setTimeout(cb, 0));

yeah we might change the default to scheduleMIcroTask, so I'd really want to either export it as defaultScheduler or let you pass undefined to reset back to the default. which one would you prefer?

Maybe the helper above as a more user friendly abstraction?

I think mentioning it in the docs would be enough. This is pretty advanced stuff yet still pretty buildable in user-land

constant-blueOP•9mo ago

For me as a user it's a little bit confusing why I am calling a setter (on what I assume is a singleton ?) for invalidating something in my app, and then I must not forget to reset the scheduler Maybe setSchedulerOnce would be nice, but it could easily be implemented in userland (with setScheduler(custom) + setScheduler(defaultScheduler)

flat-fuchsia•9mo ago

Tons of options but resetDefaultScheduler() could work as setScheduler(undefined) with more clarity

foreign-sapphire•9mo ago

I think exporting the default is in-line with us also exporting defaults like defaultShouldDehydrateQuery etc: https://github.com/TanStack/query/pull/8872

GitHub

feat: export defaultScheduleFn by TkDodo · Pull Request #8872 · T...

so that users can set a different scheduler and fall back to the default, too

foreign-sapphire•9mo ago

I also don't really understand why we have a setNotifyFunction, setBatchNotifyFunction and setScheduler 🤯. What's the differences 😅 🙈

constant-blueOP•9mo ago

FYI I did a little more testing, and in scenarios where setQueryData is called directly (not invalidateQueries), we need to wrap the synced call in notifyManager.batch Like so

 const manualSetQueryData = async () => {
    return notifyManager.batch(async () => {
      await synced(async () => {
        number += 1;
        letter += 1;

        queryClient.setQueryData(["numbers"], number);
        await sleep(500);
        queryClient.setQueryData(["letters"], number);
        return sleep(1_500);
      });
    });
  };

 const manualSetQueryData = async () => {
    return notifyManager.batch(async () => {
      await synced(async () => {
        number += 1;
        letter += 1;

        queryClient.setQueryData(["numbers"], number);
        await sleep(500);
        queryClient.setQueryData(["letters"], number);
        return sleep(1_500);
      });
    });
  };

It's me again 👋 I was writing an answer on the github discussion when I found an edge case. When several mutation are called in //, they "override" the current scheduler (which is fine), but the "reseting the scheduler" part is sometimes executed while some promise is being awaited, but thus the scheduler is back to the default before the promise is resolved (race condition) It's tricky, not sure it's easy to implement user-land. Or maybe we also need singleton variables to keep track of the latest "synced" scheduler and ignore calls to reset if not latest

rival-black•9mo ago

Could you set up a Stackblitz with the solution as is (alongside a route that doesn't use synced as a control)? That way I can take a look and we'll also have a reference if new docs have to be added.

constant-blueOP•8mo ago

Will do it today during my break

constant-blueOP•8mo ago

There it is @DogPawHat https://stackblitz.com/edit/react-aq4fjkks?file=src%2Findex.js

Antoine Chalifour

StackBlitz

Tanstack query synced - StackBlitz

A create-react-app project based on react and react-dom.

rival-black•8mo ago

~~I'm in the climbing gym but I had a thought that I'll test when I get home: calling cancelQueries before invalidation~~ this doesn't work yeah, if you override the function call the observers get notified about the cache updates and it breaks again. I'll play around with it tomorrow.

rival-black•8mo ago

https://stackblitz.com/edit/vitejs-vite-cftahzzq?file=src%2FApp.tsx ok I've got something that might work

Ciarán Curley

StackBlitz

syncedWithLock - StackBlitz

Next generation frontend tooling. It's fast!

rival-black•8mo ago

Example code:

const useSyncedMutation = () => {
  const schedulerRef = useRef<'unlocked' | 'locked'>('unlocked');
  const queryClient = useQueryClient();

  const lockScheduler = () => {
    schedulerRef.current = 'locked';
    return () => {
      schedulerRef.current = 'unlocked';
    };
  };

  const synced = async (getPromise: () => Promise<unknown>) => {
    if (schedulerRef.current === 'locked') {
      return;
    }

    const unlock = lockScheduler();
    const promise = getPromise();
    return new Promise((resolve, reject) => {
      notifyManager.setScheduler(async (cb) => {
        try {
          await promise;
          resolve(undefined);
        } catch (e) {
          reject(e);
        } finally {
          cb();
          unlock();
          notifyManager.setScheduler(defaultScheduler);
        }
      });
    });
  };

  return useMutation({
    mutationFn: () => Promise.resolve(),
    onSuccess: async () => {
      await synced(async () => {
        return Promise.allSettled([
          queryClient.invalidateQueries({
            queryKey: numbersQueryOptions().queryKey,
          }),
          queryClient.invalidateQueries({
            queryKey: lettersQueryOptions().queryKey,
          }),
        ]);
      });
    },
  });
};

const useSyncedMutation = () => {
  const schedulerRef = useRef<'unlocked' | 'locked'>('unlocked');
  const queryClient = useQueryClient();

  const lockScheduler = () => {
    schedulerRef.current = 'locked';
    return () => {
      schedulerRef.current = 'unlocked';
    };
  };

  const synced = async (getPromise: () => Promise<unknown>) => {
    if (schedulerRef.current === 'locked') {
      return;
    }

    const unlock = lockScheduler();
    const promise = getPromise();
    return new Promise((resolve, reject) => {
      notifyManager.setScheduler(async (cb) => {
        try {
          await promise;
          resolve(undefined);
        } catch (e) {
          reject(e);
        } finally {
          cb();
          unlock();
          notifyManager.setScheduler(defaultScheduler);
        }
      });
    });
  };

  return useMutation({
    mutationFn: () => Promise.resolve(),
    onSuccess: async () => {
      await synced(async () => {
        return Promise.allSettled([
          queryClient.invalidateQueries({
            queryKey: numbersQueryOptions().queryKey,
          }),
          queryClient.invalidateQueries({
            queryKey: lettersQueryOptions().queryKey,
          }),
        ]);
      });
    },
  });
};

This only works inside the React lifecycle obviously; other frameworks will have to do something different.

constant-blueOP•8mo ago

I think if you return when locked you're never calling the getPromise so never calling the invalidation right ? I think we need to maintain some kind of "queue" and only reset the default scheduler when the queue is empty when some synced invalidation is called

rival-black•8mo ago

Yeah as is the first call remains locked until the promise resolves. I guess you want further calls to "bump" the promise and not unlock the scheduler? ...well I've come up with this monstrosity:

const useSyncedMutation = () => {
  const schedulerRef = useRef<'unlocked' | 'locked'>('unlocked');
  const promiseSetRef = useRef(new Set<Symbol>());
  const resolveSchedulerUnlockRef = useRef<(_: unknown) => void | null>(null);
  const rejectSchedulerLockRef = useRef<(_: unknown) => void | null>(null);
  const queryClient = useQueryClient();

  const lock = () => {
    schedulerRef.current = 'locked';
    let promise = new Promise((res, rej) => {
      resolveSchedulerUnlockRef.current = res;
      rejectSchedulerLockRef.current = rej;
    });

    notifyManager.setScheduler(async (cb) => {
      await promise;
      cb();
      schedulerRef.current = 'unlocked';
      resolveSchedulerUnlockRef.current = null;
      rejectSchedulerLockRef.current = null;
      notifyManager.setScheduler(defaultScheduler);
    });
  };

  const synced = async (getPromise: () => Promise<unknown>) => {
    if (schedulerRef.current === 'unlocked') {
      lock();
    }

    const promiseId = Symbol('promise');
    promiseSetRef.current.add(promiseId);

    try {
      await getPromise();
    } catch (e) {
      (rejectSchedulerLockRef.current as (_: unknown) => void)(e);
    } finally {
      promiseSetRef.current.delete(promiseId);
      if (promiseSetRef.current.size === 0) {
        (resolveSchedulerUnlockRef.current as (_: unknown) => void)(undefined);
      }
    }
  };

  return useMutation({
    mutationFn: () => Promise.resolve(),
    onSuccess: async () => {
      await synced(async () => {
        return Promise.allSettled([
          queryClient.invalidateQueries({
            queryKey: numbersQueryOptions().queryKey,
          }),
          queryClient.invalidateQueries({
            queryKey: lettersQueryOptions().queryKey,
          }),
        ]);
      });
    },
  });
};

const useSyncedMutation = () => {
  const schedulerRef = useRef<'unlocked' | 'locked'>('unlocked');
  const promiseSetRef = useRef(new Set<Symbol>());
  const resolveSchedulerUnlockRef = useRef<(_: unknown) => void | null>(null);
  const rejectSchedulerLockRef = useRef<(_: unknown) => void | null>(null);
  const queryClient = useQueryClient();

  const lock = () => {
    schedulerRef.current = 'locked';
    let promise = new Promise((res, rej) => {
      resolveSchedulerUnlockRef.current = res;
      rejectSchedulerLockRef.current = rej;
    });

    notifyManager.setScheduler(async (cb) => {
      await promise;
      cb();
      schedulerRef.current = 'unlocked';
      resolveSchedulerUnlockRef.current = null;
      rejectSchedulerLockRef.current = null;
      notifyManager.setScheduler(defaultScheduler);
    });
  };

  const synced = async (getPromise: () => Promise<unknown>) => {
    if (schedulerRef.current === 'unlocked') {
      lock();
    }

    const promiseId = Symbol('promise');
    promiseSetRef.current.add(promiseId);

    try {
      await getPromise();
    } catch (e) {
      (rejectSchedulerLockRef.current as (_: unknown) => void)(e);
    } finally {
      promiseSetRef.current.delete(promiseId);
      if (promiseSetRef.current.size === 0) {
        (resolveSchedulerUnlockRef.current as (_: unknown) => void)(undefined);
      }
    }
  };

  return useMutation({
    mutationFn: () => Promise.resolve(),
    onSuccess: async () => {
      await synced(async () => {
        return Promise.allSettled([
          queryClient.invalidateQueries({
            queryKey: numbersQueryOptions().queryKey,
          }),
          queryClient.invalidateQueries({
            queryKey: lettersQueryOptions().queryKey,
          }),
        ]);
      });
    },
  });
};

It's "I think I should be using xstate levels of bad" ngl and if one of the promises throws an error, I've no idea what will happen

rival-black•8mo ago

Just took the two hooks above and through them in a GitHub comment https://github.com/TanStack/query/discussions/8863

GitHub

"Atomic" cache invalidation for more UI stability · TanStack query...

Hi everyone! I recently migrated parts of my Remix app from loaders + actions to React query. I'm pretty happy with the migration since I got more control of caching and I can invalidate only s...

Gaming

Programming

Batching invalidation updates

Did you find this page helpful?