Few questions and ideas

[denisborovikov]

Hey, I just found this library, and it has exciting ideas. Thank you! I have a few questions, though.

I tried to reimplement your optimistic update example but moved the implementation into the mutation definition instead of using the hook instance (a similar question was raised in this issue #21). This implementation is more common because I want to have optimistic updates working for every instance of the hook, no matter how many times I use it in the app and don't copy over the implementation for every instance.

This is what I got codesandbox

I found a few issues there:

1. When useDefaultOptions returns an options object, the onError/onSettle context type is inferred incorrectly (infers unknown instead of { previousTodos: Todos } - the return type of the onMutate callback.

If I remove useDefaultOptions and pass options to createMutation directly, the context type is inferred correctly.

createMutation({ mutationFn, onMutate, onError })  // correct context type
createMutation({ mutationFn, useDefaultOptions: () => ({ onMutate, onError }) })  // unknown context type

2. I think useDefaultOptions should receive the options object from the hook as an argument. Otherwise, if both options are used, the hook options override the ones from useDefaultOptions and there's no way to access the options used in the hook instance. Make sure options don't override each other.

Using the optimistic update code as an example, I want to clear the input after a user clicks the add button:

const useAddTodo = createMutation({ mutationFn, useDefaultOptions: () => ({ onMutate, onError }) })
...
const addTodoMutation = useAddTodo({
  // When mutate is called:
  onMutate: async () => {
    setText("");
  },
});

If I do this, the onMutate from useAddTodo replaces the onMutate in createMutation.

If useDefaultOptions would receive options as an argument, it could be fixed like this:

const useAddTodo = createMutation({
  mutationFn, useDefaultOptions: (options) => ({
    onMutate: (newTodo) => {
      // optimistic update logic goes here
      // then you can run onMutate from useAddTodo()
      options?.onMutate?.(newTodo) 
    }
  })
})

3. I like the shorthand setData for setQueryData with queryClient and queryKey built in. What do you think of adding other most used methods? Now your optimistic updates example looks inconsistent because it used a mixed API. It uses todosQuery.setData() but at the same time await queryClient.cancelQueries(todosQuery.queryKey); and queryClient.invalidateQueries(useTodos.getKey()).

It will be nicer if you expose other methods like getData for queryClient.getQueryData, invalidateQuery for queryClient.invalidateQueries etc.

I.e.

const { methods: { getData, setData, invalidateQuery } } = useTodos()

Maybe make it configurable so devs can decide which methods to expose.

4. It's minor but what's the logic of exposing queryKey as a hook return and getKey() as a static method? getKey seems self-sufficient for this purpose. queryKey does the same, but it makes API look inconsistent.

[liaoliao666]

1. Seems that it is unable to infer the correct type for useDefaultOptions's context after i tried.

2. Cannot pass the hook options into useDefaultOptions. Cuz the hook options is the last options u pass in and it will override useDefaultOptions.

3. I don't want to add more built-in methods. It will add more complexity to this library. But setData is different, it let you to have a better typescript experience.

4. queryKey is the key of current query, and getKey is just a static method. If u using getKey do same thing, it will make more code. as bwlown

   const todosQuery = useTodos()
   queryClient.invalidateQueries(useTodos.getKey(todosQuery.variables))

[denisborovikov]

Thanks for the reply!

[liaoliao666]

Now you can using middleware to customize ur hook whatever u neeed.