Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(query-core): change QueryMeta and MutationMeta to accept generic types #5412

Closed

Conversation

vojvodics
Copy link

@vojvodics vojvodics commented May 15, 2023

Referring to #4253 and https://tkdodo.eu/blog/breaking-react-querys-api-on-purpose.

Assuming I have a setup like this:

interface MyMeta {
  errorMessage: string | ((error: unknown, variables: unknown, context: unknown) => string);
}

declare module '@tanstack/react-query' {
  interface QueryMeta extends MyMeta {}
}

const queryClient = new QueryClient({
  mutationCache: new MutationCache({
    onError: (error, variables, context, mutation) => {
      if (query.meta.errorMessage) {
		const message = 
			typeof query.meta.errorMessage === 'function' 
				? query.meta.errorMessage(error, variables, context) 
				: query.meta.errorMessage;
        toast.error(message)
      }
    },
  }),
})

export function useDeleteTodo() {
  return useMutation({
    mutationFn: deleteTodo,
    meta: {
	  // I have to specify the type
      errorMessage: (_error, todo: Todo) => `Failed to delete ${todo.name}`
    },
  })
}

This PR enables you to infer the types correctly in meta field:

interface MyMeta<
  TData = unknown,
  TError = unknown,
  TVariables = void,
  TContext = unknown
> {
  errorMessage: string | ((error: TError, variables: TVariables, context: TContext) => string);
  // I can also infer the type of data correctly
  successMessage: string | ((data: TData, variables: TVariables, context: TContext) => string);
}

declare module '@tanstack/react-query' {
  interface QueryMeta<
    TData = unknown,
    TError = unknown,
    TVariables = void,
    TContext = unknown
  > extends MyMeta<TData, TError, TVariables, TContext> {}
}

const queryClient = new QueryClient({
  mutationCache: new MutationCache({
    onError: (error, variables, context, mutation) => {
      if (query.meta.errorMessage) {
		const message = 
			typeof query.meta.errorMessage === 'function' 
				? query.meta.errorMessage(error, variables, context) 
				: query.meta.errorMessage;
        toast.error(message)
      }
    },
  }),
})

export function useDeleteTodo() {
  return useMutation({
    mutationFn: deleteTodo,
    meta: {
	  // todo is inferred correctly now
      errorMessage: (_error, todo) => `Failed to delete ${todo.name}`
    },
  })
}

Besides inference this will catch type errors when refactoring - if we want to refactor deleteTodo from function deleteTodo(todo: Todo): Promise<void> to function deleteTodo(options: {todo: Todo, projectId: string}): Promise<void>, we'll get a type error, however, without inference we could easily miss this.

Is there any way to ignore the errors of unused types?
Is there a docs section that I should update?

@vercel
Copy link

vercel bot commented May 15, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
query ⬜️ Ignored (Inspect) May 16, 2023 9:24am

@TkDodo
Copy link
Collaborator

TkDodo commented May 16, 2023

please have a look at how we solved this differently in v5:

https://tanstack.com/query/v5/docs/react/typescript#registering-a-global-error

I realise the docs are for error only, but we have the same thing for meta:

export interface Register {
// defaultError: Error
// queryMeta: Record<string, unknown>
// mutationMeta: Record<string, unknown>
}

@vojvodics
Copy link
Author

vojvodics commented May 16, 2023

Interesting... Although I still don't see how that implementation will help me infer types in meta functions 😕

In v5 I'd override it like this:

interface MyMeta {
  errorMessage: string | ((error: unknown, variables: unknown, context: unknown) => string);
}

declare module '@tanstack/react-query' {
  interface Register {
    mutationMeta: MyMeta;
  }
}

But I still won't be able to infer the values, so I think the Register needs to be generic (for this to work):

interface MyMeta<
  TData = unknown,
  TError = unknown,
  TVariables = void,
  TContext = unknown
> {
  errorMessage: string | ((error: TError, variables: TVariables, context: TContext) => string);
  // I can also infer the type of data correctly
  successMessage: string | ((data: TData, variables: TVariables, context: TContext) => string);
}

declare module '@tanstack/react-query' {
  interface Register<
    TData = unknown,
    TError = unknown,
    TVariables = void,
    TContext = unknown
  > {
    mutationMeta: MyMeta<TData, TError, TVariables, TContext>;
  }
}

I can create a PR for the v5 branch if you'd like this to be a v5 feature only

I can also create a reproduction repo if it helps to highlight the problem this solves

@TkDodo
Copy link
Collaborator

TkDodo commented May 20, 2023

meta is available in situations where you can't really determine what type it is, e.g. when iterating over all Queries. For example:

queryClient.invaliadateQueries({ predicate: query => ... })

here, query.meta cannot be inferred to anything specific I think. We also don't infer query.queryKey here.

but I can see that the function use-case is pretty interesting. If you can make a PR for v5 (alpha branch), I can take another look!

@TkDodo TkDodo closed this May 20, 2023
@vojvodics vojvodics deleted the refactor/generic-meta-types branch May 24, 2023 07:22
@bryanltobing
Copy link
Contributor

bryanltobing commented Oct 3, 2023

i try to solve it using declaration merging but this doesn't seem to work

declare module '@tanstack/react-query' {
  interface MutationMeta {
    customField1: string;
    customField2: number;
  }
}

// mutation.meta?.customField1 <- doesn't type safe

updates: nevermind, it does work

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants