react-apollo-hooks icon indicating copy to clipboard operation
react-apollo-hooks copied to clipboard

return memoized value

Open sijad opened this issue 6 years ago • 8 comments

currently returned value both of useMutation and useQuery calculated every time they called, I'm not really familiar with apollo logics, but I guess this can memoized?, for instance useMutation should be similar like this:

export function useMutation(mutation, baseOptions) {
  const previousOptions = useRef();
  const client = useApolloClient();
  const result = isEqual(previousOptions.current, baseOptions);
  return useMemo(
    () => {
      previousOptions.current = baseOptions;
      return localOptions => client.mutate({
        mutation,
        ...baseOptions,
        ...localOptions,
      });
    },
    [
      mutation,
      result ? previousOptions.current : baseOptions,
    ],
  );
}

sijad avatar Dec 14 '18 09:12 sijad

@sijad Have you tried to profile it if the version with memoization is really faster than the current one?

trojanowski avatar Dec 18 '18 19:12 trojanowski

no, I did not, but I guess with memorized version you can actually prevent re-rendering e.g:

const mutate = useMutation(MUTATION)
const memoizedMutation = useCallback(() => {
  mutate({
    variables,
  });
}, [mutate]);

return (<button onClick={memoizedMutation}>click on me</button>);

with current implimention mutate always have a new value therefore it might re-render more...

sijad avatar Dec 19 '18 18:12 sijad

Checking for mutation itself is not enough, we have to check for changes of options parameters, and this will create unnecessary memoization most of the times.

Anyways, React team does not suggest to pass individual callbacks in props and there is (not ideal) solution for this. I hope they will come up with better idea.

But if you don't like it, you still can use your own hook for memoization , e.g:

function useMemoMutation(mutation, options, inputs) {
   return useCallback(useMutation(mutation, option), inputs);
}

function Foo({ bar, baz }) {
  const mutate = useMemoMutation(Mutate, {
    variables: { bar, baz }
  }, [bar, baz]) // Update only on `bar` or `baz` change
}

avocadowastaken avatar Dec 27 '18 19:12 avocadowastaken

We also can useRef to imitate this in class components:

export function useMutation<TData, TVariables = OperationVariables>(
  mutation: DocumentNode,
  baseOptions?: MutationHookOptions<TData, TVariables>
): MutationFn<TData, TVariables> {
  const client = useApolloClient();

  // Use refs to access variable values inside callback.
  const mutationRef = useRef(mutation);
  const baseOptionsRef = useRef(baseOptions);

  // Update references on each render.
  mutationRef.current = mutation;
  baseOptionsRef.current = baseOptions;

  // Memoize callback.
  const mutationFnRef = useRef<MutationFn<TData, TVariables>>(options =>
    client.mutate({
      mutation: mutationRef.current,
      ...baseOptionsRef.current,
      ...options,
    })
  );

  return mutationFnRef.current;
}

avocadowastaken avatar Jan 10 '19 07:01 avocadowastaken

A downside of not memoizing the value of useMutation or useQuery is that they break the memoization of hooks that are built on top of them if you follow the rules (and eslint plugin) for hooks strictly.

robertvansteen avatar Feb 25 '19 20:02 robertvansteen

I just leave @gaearon's comment here.

in my test objToKey seems slower that deep equal.

sijad avatar Mar 09 '19 19:03 sijad

should not this be closed by https://github.com/apollographql/react-apollo/issues/3260 ?

prztrz avatar Sep 20 '19 11:09 prztrz

@prztrz this library doesn’t use React-apollo’s useMutation (this preceded that!)

If you switch to @apollo/hooks or whatever that package name is, then that fix will be relevant. Cheers!

fbartho avatar Sep 20 '19 16:09 fbartho