HMS-9683: migrate react-query to v5

[dominikvagner]

Summary

Warning

TODO: The commits need to be squashed before merging into 2 (one for each version jump), they are split now for easier reviewing.

List of changes 📜🔧 :

v3 --> v4:

• The package name was changed, the @tanstack namespace was added, so the import had to updated (in both regular and test files where they were being mocked).
• Query keys now have to an array (previously they could be either a string or an array).
• Invalidate queries function changed parameters, and it now takes query keys only in an object instead of the first argument being a string or an array.
• Behavior of isLoading has changed for disabled queries, due to the removal of idle query state. Now isInitialLoading has to be used to achieve the same result.

v4 --> v5:

• Lots of functions support just a single signature now, one object. Their usage was update to correctly use the new way.
• The isLoading state of mutations has been renamed to isPending, while keeping its function.
  • The isLoading has been changed for queries as well, but unlike the mutations where you should really use pending as the meaning of it makes sense (from the language/English POV).
    Queries got a "new" isLoading flag which is implemented as isPending && isFetching and it's meaning/function should now better indicate the need to show a loading spinner (even for disabled queries) so we don't need to change all uses of isLoading in queries. [1]
• Callbacks on useQuery (they stayed for mutations) have been removed.

  • onSuccess, onError and onSettled have been removed from queries. They haven't been touched for Mutations. Please see this RFC for motivations behind this change and what to do instead.

  • This means that the onError notification from queries had to be removed and are now handled globally by a onError callback on a global queryCache that's been added to the queryClient.
    • The queries now contain the query specific title and id in a meta field, which can be populated by anything.
    • For this to work the notifications provider had to be moved up into AppEntry, where a notifications store has to be defined and used for the notifications in the query cache, then the store is passed down to the provider.

Testing steps

Tests pass. Manual testing seems good.
Notifications work as they did before.
The behavior hasn't changed, click throughout the app, and verify everything behaves as it did before.

[xbhouse]

https://issues.redhat.com/browse/HMS-9683

[sourcery-ai]

Hey - I've found 2 issues, and left some high level feedback:

• In DeleteSnapshotsModal.tsx there is a stray console.log(isTemplateError); left in the component; this should be removed or routed through your logging/notification utilities before merging.
• In useFetchSubscriptionsQuery, queryFn is a memoized promise (getSubscriptions() / getEphemeralSubscriptions() is invoked immediately) and then returned from the React Query queryFn; consider memoizing the function itself (e.g. () => getSubscriptions() / () => getEphemeralSubscriptions()) to avoid creating and capturing a promise outside React Query’s lifecycle.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- In DeleteSnapshotsModal.tsx there is a stray `console.log(isTemplateError);` left in the component; this should be removed or routed through your logging/notification utilities before merging.
- In `useFetchSubscriptionsQuery`, `queryFn` is a memoized promise (`getSubscriptions()` / `getEphemeralSubscriptions()` is invoked immediately) and then returned from the React Query `queryFn`; consider memoizing the function itself (e.g. `() => getSubscriptions()` / `() => getEphemeralSubscriptions()`) to avoid creating and capturing a promise outside React Query’s lifecycle.

## Individual Comments

### Comment 1
<location> `src/AppEntry.tsx:22-25` </location>
<code_context>
 }

 export default function AppEntry({ logger }: AppEntryProps) {
+  const notificationsStore = createNotificationStore();
   const store = React.useMemo(() => {
     restoreStore();
</code_context>

<issue_to_address>
**issue (bug_risk):** notificationsStore is recreated on every render and can diverge from the store used in QueryCache callbacks

Since `notificationsStore` is recreated on every render but `QueryClient` is initialized only once via `useState`, the `queryCache.onError` callback will keep referencing the original store while `NotificationsProvider` may receive a new one. This can cause notifications to dispatch to an unmounted store and never render. Please initialize `notificationsStore` once and share that instance with both `NotificationsProvider` and the `QueryClient` (e.g., define it outside the component or create it via `useState`/`useMemo`).
</issue_to_address>

### Comment 2
<location> `src/Pages/Repositories/ContentListTable/components/SnapshotListModal/DeleteSnapshotsModal/DeleteSnapshotsModal.tsx:101-102` </location>
<code_context>
   } = useGetSnapshotList(uuid, 1, -1, '');

   const { isError: isTemplateError, data: templates } = useFetchTemplatesForSnapshots(uuid, uuids);
+  console.log(isTemplateError);

   useEffect(() => {
</code_context>

<issue_to_address>
**issue:** Debug console.log left in production component

This `console.log(isTemplateError);` will run on every render and add unnecessary noise to production logs and tests. Since error handling already uses `isTemplateError` and the `useEffect` below, this debug statement can be removed.
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[sourcery-ai]

Hey - I've found 2 issues, and left some high level feedback:

• In useFetchSubscriptionsQuery, the queryFn is computed via useMemo as getEphemeralSubscriptions()/getSubscriptions() and then wrapped as queryFn: () => queryFn; this means you're returning a pre-created promise instead of a fresh call and you’ve effectively lost the runtime branch on isEphemeral—consider keeping queryFn as a function (() => isEphemeral ? getEphemeralSubscriptions() : getSubscriptions()).
• Several queries that previously had keepPreviousData: false (e.g. useTagsQuery) now use placeholderData: keepPreviousData, which will always reuse the last result during refetch; double-check that this change in behavior (showing cached data instead of an empty/loading state) is intentional for those specific screens.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- In `useFetchSubscriptionsQuery`, the `queryFn` is computed via `useMemo` as `getEphemeralSubscriptions()`/`getSubscriptions()` and then wrapped as `queryFn: () => queryFn`; this means you're returning a pre-created promise instead of a fresh call and you’ve effectively lost the runtime branch on `isEphemeral`—consider keeping `queryFn` as a function (`() => isEphemeral ? getEphemeralSubscriptions() : getSubscriptions()`).
- Several queries that previously had `keepPreviousData: false` (e.g. `useTagsQuery`) now use `placeholderData: keepPreviousData`, which will always reuse the last result during refetch; double-check that this change in behavior (showing cached data instead of an empty/loading state) is intentional for those specific screens.

## Individual Comments

### Comment 1
<location> `src/services/Subscriptions/SubscriptionQueries.ts:10-15` </location>
<code_context>
 export const useFetchSubscriptionsQuery = () => {
-  const errorNotifier = useErrorNotification();
   const isEphemeral = useIsEphemeralEnv();
   const queryFn = useMemo(
     () => (isEphemeral ? getEphemeralSubscriptions() : getSubscriptions()),
     [isEphemeral],
   );

-  return useQuery<Subscriptions>([SUBSCRIPTION_CHECK_KEY], () => queryFn, {
-    onError: (err) =>
-      errorNotifier(
-        'Error fetching subscriptions',
-        'An error occurred',
-        err,
-        'fetch-subscriptions-error',
-      ),
+  return useQuery({
+    queryKey: [SUBSCRIPTION_CHECK_KEY],
+    queryFn: () => queryFn,
</code_context>

<issue_to_address>
**suggestion:** Avoid eagerly invoking `getSubscriptions` inside `useMemo` and simplify the query function.

Here `queryFn` holds the result of `getEphemeralSubscriptions()`/`getSubscriptions()`, so `useQuery` is actually using `() => queryFn`. While this works, it eagerly calls the API when the hook runs rather than when React Query fires the request, and adds an extra `useMemo` indirection. You can inline the logic into `queryFn` and drop `useMemo`:

```ts
return useQuery({
  queryKey: [SUBSCRIPTION_CHECK_KEY],
  queryFn: () => (isEphemeral ? getEphemeralSubscriptions() : getSubscriptions()),
  meta: {
    title: 'Error fetching subscriptions',
    id: 'fetch-subscriptions-error',
  },
});
```

Suggested implementation:

```typescript
import useIsEphemeralEnv from 'Hooks/useIsEphemeralEnv';
import { useQuery } from '@tanstack/react-query';

```

```typescript
export const useFetchSubscriptionsQuery = () => {
  const isEphemeral = useIsEphemeralEnv();

  return useQuery({
    queryKey: [SUBSCRIPTION_CHECK_KEY],
    queryFn: () =>
      isEphemeral ? getEphemeralSubscriptions() : getSubscriptions(),
    meta: {
      title: 'Error fetching subscriptions',
      id: 'fetch-subscriptions-error',
    },
  });
};

```
</issue_to_address>

### Comment 2
<location> `src/services/Content/ContentQueries.ts:834` </location>
<code_context>
+  });
 };

 export const useBulkDeleteSnapshotsMutate = (
</code_context>

<issue_to_address>
**suggestion (bug_risk):** Align the mutation function signature with the variables passed to `mutate`/`mutateAsync` to avoid future inconsistencies.

`mutationFn` currently closes over `uuids` from hook creation, while `onMutate` receives `checkedSnapshots`. If a caller passes a different set to `mutate`, the optimistic update and the server request can diverge.

Have `mutationFn` take the same argument as `onMutate`, e.g.:

```ts
return useMutation({
  mutationFn: (checkedSnapshots: Set<string>) =>
    deleteSnapshots(repoUuid, checkedSnapshots),
  onMutate: async (checkedSnapshots: Set<string>) => {
    // ... existing optimistic update logic
  },
  // ...
});
```

This keeps a single source of truth and ensures both optimistic updates and the actual mutation use the same input.

Suggested implementation:

```typescript
  return useMutation({
    mutationFn: (checkedSnapshots: Set<string>) =>
      deleteSnapshots(repoUuid, checkedSnapshots),

    onMutate: async (checkedSnapshots: Set<string>) => {
      await queryClient.cancelQueries({

```

1. Ensure that all callers of `useBulkDeleteSnapshotsMutate` call `mutate`/`mutateAsync` with the appropriate `Set<string>` (`checkedSnapshots`) so that both `mutationFn` and `onMutate` receive the same input.
2. The snippet references `repoUuid` and `uuids` but only `repoUuid` is used after this change; if `uuids` is no longer needed anywhere else in the hook, remove it from the hook's parameters and internal state to avoid dead code.
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}