query icon indicating copy to clipboard operation
query copied to clipboard

Status of useMutation not shared across usages/components with custom hook (like useQuery does)

Open bennettdams opened this issue 3 years ago • 14 comments

Describe the bug Please move this to a "feature request" discussion if this is not a bug. At least for me, this took me half a day to figure out, so I classified it as a bug.

Imagine creating a custom hook (let's name it useTodos) which is used in multiple components and uses useQuery and useMutation inside. When using useQuery, its status (and the helpers isLoading, isError, ...) are shared across multiple usages of useTodos. For useMutation, this is not true. Every usage of useTodos will have its own mutation status.

To Reproduce Here's a CodeSandbox that shows the behavior. It shows two components that both use useTodos. For GET requests (via useQuery), the status is shared across the components. For POST requests (via useMutation), the status is different for both components. Only the component who executes the mutation will have the status "error".

react-query

Expected behavior I understand that useQuery has one precise query key to identify "the same hook". Mutations are not like that. In my case, I had a list that showed items (like "Todos") and a child component that was supposed to create a new item (like "Todo"). Naturally, I used useTodos in both components and wanted to show an error in the List component. The problem is isError was never true, only the child component's one was.

function TodoList() {
  // this "isError" here will never be true, only the <Todo> component's "isError" will
  const { todos, isError } = useTodos();

  return (
    <div>
      <p>Todos:</p>
      {isError && <p>Error!</p>}
      <div>
        {todos.map((todo) => (
          <Todo todo={todo} />
        ))}
      </div>
    </div>
  );
}

function Todo({ todo }: { todo: Todo | null }) {
  const { createTodo } = useTodos();

  return (
    <div>
      {todo ? (
        <p>{todo.title}</p>
      ) : (
        <div>
          <button onClick={() => createTodo(...)}>Create</button>
          <input ... />
           ...
        </div>
      )}
    </div>
  );
}

I'm using React Query for a while now and being used to useQuery having a shared status, I wasted time checking my mutation implementation ("Why is the error not thrown?" etc.), but in reality useQuery and useMutation just behave differently and differently than expected.

I would really like to let useMutations share their status across all components. There's already a mutationKey, which could maybe used for this exact use case.

bennettdams avatar May 21 '21 14:05 bennettdams

Yes, I was also surprised by this, but mutationKey is not required, unlike the queryKey, so we can't really create a subscription like for useQuery. Mutations are also imperative so it just doesn't happen very often I guess. What we do have is the useIsMutating hook, similar to the useIsFetching hook, so that you know when a mutation is in-flight, and you can filter these by mutationKey. With that, you can e.g. disable all buttons that would trigger the same mutation. Not sure how that would fix the error situation though - but you can take a look how to create a subscription to the mutationCache by looking at the impl of useIsMutating:

https://github.com/tannerlinsley/react-query/blob/master/src/react/useIsMutating.ts

TkDodo avatar May 21 '21 14:05 TkDodo

@TkDodo Thanks for the feedback and possible workaround. I could also create my own React context and save isError or isLoading there, so all components have the same status - I just thought it'd be nice to let React Query provide this out of the box. Feel free to move this issue to the discussions, so we keep it there as a "feature request".

bennettdams avatar May 25 '21 09:05 bennettdams

I have similar "Idea" under the discussions section - https://github.com/tannerlinsley/react-query/discussions/2294

VolodymyrBaydalka avatar May 25 '21 12:05 VolodymyrBaydalka

Feel free to move this issue to the discussions, so we keep it there as a "feature request".

let's keep here, I'd like to analyze how hard it would be to actually use the mutationKey to achieve that functionality, or if that would be a breaking change.

TkDodo avatar May 25 '21 12:05 TkDodo

I always attach mutationKey and then got to the part where i needed to share if it ever succeeded or failed count and same thing. Thanks for bringing up this topic, it was first hit on search engine. I should have searched first rather then assume.

Noitidart avatar Oct 05 '21 18:10 Noitidart

I was very surprised to find this out today when my mutation hook wasn't working as expected. I've also run into issues like the one described here as well as #2294. Losing transient mutation state like isLoading or error when your component changes contexts is unfortunate.

It made me start to wonder why there's such a strong conceptual distinction between queries and mutations, when at the end of the day they're both just abstractions around some promise that returns data, generally an HTTP request.

You could almost use useQuery in place of useMutation today if it exposed the imperative functions like mutate and mutateAsync and if it had a callback like onMutate. The key difference between the two hooks seems to come down to configuration, e.g. queries execute on lifecycle events by default, mutations do not retry by default, etc. I could imagine both of these hooks being thin wrappers around some other hook that provides all the common functionality, such as useRequest. This could even simplify some of the internals for RQ - one cache instead of separate query and mutation cache.

I could easily be overlooking some important details or underestimating the complexity, but I do agree that the state behavior of useMutation is not intuitive and inconsistent with useQuery.

stefan-toubia avatar Jan 13 '22 00:01 stefan-toubia

@stefan-toubia it is something that is on my mind for quite some time. What if mutations were conceptually "just" disabled queries, where mutate or mutateAsync just calls refetch() 🤔 .

Not only would that get rid of a ton of internal code (MutationCache, Mutation, MutationObserver, ...), it would also, at the same time, add a lot of features for mutations, like sharing data, select and other options.

Of course, we could still turn off some of the options and / or set different default values for useMutation.

However, when I went down that road, there is one thing that works inherently different from queries:

useQuery instantly creates an observer, but useMutation only does it after .mutate is called. Further, if you call .mutate multiple times, the previous observer is unsubscribed automatically. That is necessary so that callbacks of .mutate are only called once. However, callbacks on useMutation itself are actually cache level callbacks, which will execute any time the mutation succeeds / fails, even if the observer has already unmounted. That is important, and something that doesn't exist on the query cache itself.

Feel free to take a look at the implementation (best on v4 directly, alpha branch), the road would need to be a merging of the two slightly different functionalities. Would love to have it but I don't have enough time to look into it right now :)

TkDodo avatar Jan 14 '22 13:01 TkDodo

Just thought to provide code for another workaround for this issue, which doesn't require manually keeping state but still allows different components to share the same mutation object. As mentioned, just wrap a react context around the custom hook and the desired functionality will be achieved. https://codesandbox.io/s/silent-frost-mjxlrf?file=/src/App.js

const CustomMutationProvider = ({ children }) => {
  const mutation = useMutation({
    mutationKey: "UpdateData",
    mutationFn: async () => {
      // Fake api call
      await new Promise((resolve, reject) => setTimeout(reject, 2000));
    }
  });
  return (
    <CustomMutationContext.Provider value={mutation}>
      {children}
    </CustomMutationContext.Provider>
  );
};
const CustomMutationContext = React.createContext();
const useCustomMutation = () => React.useContext(CustomMutationContext);

And use like so:

const queryClient = new QueryClient();
export default function App() {
  return (
    <div className="App">
      <QueryClientProvider client={queryClient}>
        <CustomMutationProvider>
          <ComponentA />
          <ComponentB />
        </CustomMutationProvider>
      </QueryClientProvider>
    </div>
  );
}
const ComponentA = () => {
  const { mutate, status } = useCustomMutation();
  return (
    <div>
      <h1>Component A </h1>
      Status: {status}
      <br />
      <button onClick={mutate}>Update</button>
    </div>
  );
};
const ComponentB = () => {
  const { mutate, status } = useCustomMutation();
  return (
    <div>
      <h1>Component B </h1>
      Status: {status}
      <br />
      <button onClick={mutate}>Update</button>
    </div>
  );
};

nextrapi avatar Jul 01 '22 16:07 nextrapi

If I understood the reason correctly, the following worked for me:

// workaround for https://github.com/TanStack/query/issues/2304
// instead of useIsMutating
const [isMutating, setIsMutating] = useState(false);
useEffect(() => {
  return queryClient.getMutationCache().subscribe(({ type, mutation }) => {
    if (
      type === 'updated' &&
      mutation.options.mutationKey?.[0] === 'KEY' // your predicate here
    ) {
      setIsMutating(mutation.state.status === 'loading');
    }
  });
}, []);

welcomemax avatar Aug 03 '22 13:08 welcomemax

I got into this today. My current workaround is

function useShippingZone() {
  const address = useAppSelector(selectAddress);
  const queryKey = ["getShippingZone", address.zipCode];
  const result = useQuery(queryKey, getShippingZone, {
    meta: address as IndexSignature<typeof address>,
  });
}

async function getShippingZone({ meta }: QueryFunctionContext<...>) {
  const address = meta as unknown as Address;
  ...
}

type IndexSignature<O extends object> = {
  [P in keyof O]: O[P] extends object ? IndexSignature<O[P]> : O[P];
};

where you need to pass variables through meta option. (which is very vulnerable in typing, and not really recommended as other developers wouldn't think that's what meta should be used for). But for this case where I anyway save address in a global state, I think it's worth to do.

cdpark0530 avatar Aug 15 '22 10:08 cdpark0530

Based on @TkDodo answers https://github.com/TanStack/query/issues/2304#issuecomment-1013139242, I've implemented my useCustomMutation that uses useQuery to share data between components.

My first use-case that requires that useMutation share data across components was a multi-step form in react-native (ps: in react-native, the stack navigation doesn't unmount the previous screen) where at the first page, we need to get an AuthToken, we do it onPress of a button by the user, so it makes sense to be a mutation at this point, but I faced that if I need to get the token on subsequent screens, it will not be there, so that's why of the implementation.

The useCustomMutation follows the exactly same API of useMutation

export const useCustomMutation = <TData = unknown, TError = unknown, TVariables = unknown, TContext = unknown>(
  mutationKey: MutationKey,
  mutationFn: MutationFunction<TData, TVariables>,
  options?: Omit<UseMutationOptions<TData, TError, TVariables, TContext>, 'mutationKey' | 'mutationFn'>
): UseMutationResult<TData, TError, TVariables, TContext> => {
  const query = useQuery<TData, TError>(
    ['CustomMutation', mutationKey],
    async () => await Promise.resolve(false as unknown as TData),
    { retry: false, cacheTime: Infinity, staleTime: Infinity }
  )
  const queryError = useQuery<TError, TData>(
    ['CustomMutationError', mutationKey],
    async () => await Promise.resolve(false as unknown as TError),
    { retry: false, cacheTime: Infinity, staleTime: Infinity }
  )
  const mutation = useMutation<TData, TError, TVariables, TContext>(
    mutationKey,
    async (...params) => {
      queryClient.setQueryData(['CustomMutationError', mutationKey], false)
      return await mutationFn(...params)
    },
    {
      ...options,
      onSuccess: (data, variables, context) => {
        queryClient.setQueryData(['CustomMutation', mutationKey], data)
        if (options?.onSuccess) options.onSuccess(data, variables, context)
      },
      onError: (err, variables, context) => {
        queryClient.setQueryData(['CustomMutationError', mutationKey], err)
        if (options?.onError) options.onError(err, variables, context)
      },
    }
  )
  const isLoading = useIsMutating(mutationKey)

  // We need typecasting here due the ADT about the mutation result, and as we're using a data not related to the mutation result
  // The typescript can't infer the type correctly.
  // eslint-disable-next-line @typescript-eslint/consistent-type-assertions
  return {
    ...mutation,
    data: query.data,
    isLoading: !!isLoading,
    error: queryError.data,
    isError: !!queryError.data,
  } as UseMutationResult<TData, TError, TVariables, TContext>
}

You can find it also on a gist: https://gist.github.com/antoniel/1f8d59f7e3f73487e5ba02a293fb39ec

antoniel avatar Sep 02 '22 01:09 antoniel

TGhanks @antoniel for sharing, I think that onSuccess etc will only fire if the component remains mounted. I think you'll have to move the success/error into the queryFn.

Noitidart avatar Sep 02 '22 04:09 Noitidart

Any updates? Using the context API is solves the problem but it is still a bad solution.

leotuna avatar Oct 13 '22 13:10 leotuna

@antoniel 's solution worked for me as well. I used the useQueryClient hook to get the queryClient.

SzabKel avatar Oct 19 '22 09:10 SzabKel

Are there any updates?

JasoonX avatar Jan 04 '23 11:01 JasoonX

Any updates?

leotuna avatar Feb 05 '23 15:02 leotuna

Updates ?

andrecrimb avatar Feb 10 '23 15:02 andrecrimb

So I don't waste any more time on this, can someone tell me if this is possible, please?

I want to extract all the queries and mutations for a particular API endpoint into their own file so I can import them where needed e.g. get (query), get_all (query), create (mutation), update (mutation) and delete (mutation) for say the user's endpoint, so they are grouped together in their own file.

This works fine for queries and I can query endpoints anywhere in my code by importing a wrapped anonymous function that returns a useQuery function:

// ./api/UserData.tsx

...

export const getAllUsers = () => {
  const authHeader = useAuthHeader();

  return useQuery<UserModel[], Error>({ 
    queryKey: ['users'],
    queryFn: async () => {
      const response = await fetch("http://localhost:8000/users", { 
        method: 'GET',
        headers: {
          "Content-Type": "application/json",
          "Authorization": authHeader()
        }
      });

      const data = await response.json();

      if (!response.ok) {
        throw new Error(`Network response was not ok - ${response.status}: ${data.detail}`);
      }

      return data;
    },
    onError: async (error:Error) => {
      console.log(error);
    }
  });
}

...
// ./api/index.ts

import { getAllUsers, ... } from "./UserData";

export {
    getAllUsers,
    ...
};

and then I just import the function in the file I need it in

// ./folder/file.tsx

import { getAllUsers } from "~/api"
...
const userData = getAllUsers();
...

but I'm struggling to do the same with mutations as it doesn't seem to import properly as the mutate function doesn't exist when I import the createUser function:

export const createUser = () => {
  const authHeader = useAuthHeader();
  const queryClient = useQueryClient();

  return useMutation({
    mutationFn: async (newUser:UserModel) => {
      const response = await fetch("http://localhost:8000/users", { 
        method: 'POST',
        headers: {
          "Content-Type": "application/json",
          "Authorization": authHeader()
        },
        body: JSON.stringify(newUser)
      });

      const returnedData = await response.json();

      if (! response.ok) {
        throw new Error(`Network response was not ok - ${response.status}: ${returnedData.detail}`);
      }

      return returnedData;
    },
    onSuccess: (data:UserModel) => {
      console.log(data)
      queryClient.invalidateQueries(["users"]);
    },
    onError: async (error:Error) => {
      console.log(error)
    }
  });
}

I get: Property 'mutate' does not exist on type '() => void' when I do:

createUser.mutate(formData);

It does work though when the useMutation function is in the same file as the mutate call 🤔

But, after reading through this issue I'm guessing this isn't currently possible, am I correct?

thanks in advance

Chunkford avatar Feb 12 '23 10:02 Chunkford

Any updates?

Well, yes. There will likely be something in v5, but it will not mean that useMutation will share state, because every invocation of useMutation.mutate creates a new, completely separate mutation entry. It's fundamentally different from useQuery.

Instead, we will likely have a new hook useMutationState() where you can pass in filters and potentially a selector. It's like useIsMutating (which in fact is now implemented with useMutationState) but for arbitrary fields of the mutation, like data or error or something derived. It will always return an array, because unlike queries, there can be multiples even for the exact same mutationKey by just calling mutate twice on the same "instance".

changes are somewhere in this PR if you're interested:

  • https://github.com/TanStack/query/pull/4814

@Chunkford I don't think your problem has anything to do with this issue. Please just open a separate discussion (discussion, not issue 😉)

TkDodo avatar Feb 12 '23 12:02 TkDodo

v5 PR was merged

TkDodo avatar Feb 20 '23 17:02 TkDodo