swr icon indicating copy to clipboard operation
swr copied to clipboard

Sideeffects on local bound mutate after changing key

Open samedii opened this issue 3 years ago • 3 comments

Bug report

Description / Observed Behavior

If I understand correctly, the mutate method for the old key changes the new object that has a different key. The key has changed while a request is made to the server. When getting the response from the server I want to update the cache with the result from the server but the local bound mutate has changed behaviour.

I am able to get around this by keeping track of the original key and using the global mutate which makes me think that my code might be working correctly and this could be a bug.

I've tried searching for this issue but I may have missed some important keyword that I was unable to figure out.

Expected Behavior

I would expect swr to give me a function that I can use even after the key that was used to create it has changed. It felt like a very unexpected side effect.

Repro Steps / Code Example

The global solution is implemented below. Remove the comment to change to the local bound method.

import { useAuthorizedFetcher } from "./useAuthorizedFetcher";
import useBackend from "./useBackend";
import { useSWRConfig } from "swr";

const useImage = (imageId, options) => {
  const { mutate } = useSWRConfig();
  const authorizedFetcher = useAuthorizedFetcher();
  const image = useBackend(imageId ? `/images/${imageId}` : null, { options });
  // const mutate = (key, value, revalidate) => image.mutate(value, revalidate);

  const api = ((isLoading, data, key, mutate) => {
    const createAnnotation = (annotationType) => {
      if (!isLoading && data.hot_storage) {
        mutate(
          key,
          {
            ...data,
            annotations: [
              ...(data.annotations ?? []),
              {
                id: "added",
                type: annotationType,
              },
            ],
          },
          false
        );
        return authorizedFetcher(`/images/${imageId}/annotations`, {
          method: "POST",
          body: { type: annotationType },
        }).then(
          (data) => mutate(key, data, false)
        );
      } else {
        return Promise.resolve(image);
      }
    };

    // ...

    return {
      createAnnotation,
      // ...
    };
  })(image.isLoading, image.data, image.key, mutate);

  return {
    ...image,
    ...api,
  };
};

export default useImage;

Additional Context

Using SWR version 1.0.1

samedii avatar Dec 24 '21 13:12 samedii

It seems you can patch the issue easily by wrapping SWR and replacing the local mutate

  const { mutate } = useSWRConfig();

  const backend = useSWR(
    key,
    ...
  );

  const replaceLocalMutate = (data, shouldRevalidate) =>
    mutate(key, data, shouldRevalidate);

  return {
    ...backend,
    mutate: replaceLocalMutate,
  };

samedii avatar Dec 24 '21 14:12 samedii

Thanks for reporting this!

If we make the mutate function a snapshotted value, there are also many downsides:

const { mutate } = useSWR(...)

useEffect(() => {
  // do something
  mutate(...)
}, [mutate]) // ← you will have to put `mutate` here

Some might argue that mutate is per hook and it mutates the resource similar to setState, and it makes sense in some cases that people need to mutate the latest resource. Maybe we shouldn't call it "bound mutate". However I understand your point and it's indeed a problem.

BTW I think you can change your code to:

mutate(
  key,
  authorizedFetcher(`/images/${imageId}/annotations`, {
    method: "POST",
    body: { type: annotationType },
  }),
  false
)

by wrapping the promise with mutate(), it does the same thing but also skips revalidations that happened to occur while doing the mutation.

shuding avatar Dec 24 '21 18:12 shuding

I see. I think it makes sense that the bound mutate should be specified to useEffect like you wrote above and anything else would be wrong but I also only know of my use case so I am very biased :)

What use cases do you see for image.mutate mutating the current key's data rather than the original's?

One more detail from my use case: I am actually on a new "page" (using react-router) and the previous page's bound mutate was being called but had been replaced by the new mutate unexpectedly.

BTW I think you can change your code to: ...

Thanks! I had thought they were functionally equivalent.

samedii avatar Dec 28 '21 16:12 samedii