Skip to content

Conversation

@djhi
Copy link
Collaborator

@djhi djhi commented Nov 6, 2025

Problem

Our mutation hooks explicitly exclude the mutationFn option.

However, we might want to override it to simplify hooks that reuse everything from a core mutation hook except the actual dataProvider call (ex: useSoftDelete could just pass a custom mutationFn to useDelete).

Besides, the mutationFn is optional in TanStack query so it won’t change anything for our users.

Solution

Update the mutation hook to remove the explicit exclusion.

@djhi djhi added the RFR Ready For Review label Nov 6, 2025
@djhi djhi changed the base branch from master to next November 6, 2025 08:27
@slax57 slax57 requested review from Copilot and slax57 November 6, 2025 08:40
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR modifies the type definitions for mutation hooks (useUpdate, useUpdateMany, useCreate, useDelete, useDeleteMany) by removing the Omit<UseMutationOptions, 'mutationFn'> wrapper, allowing mutationFn to be included in the options type.

Key changes:

  • Type definitions now directly use UseMutationOptions instead of omitting mutationFn
  • Applies consistently across all five mutation hooks

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
packages/ra-core/src/dataProvider/useUpdateMany.ts Removed Omit<..., 'mutationFn'> wrapper from UseUpdateManyOptions type definition
packages/ra-core/src/dataProvider/useUpdate.ts Removed Omit<..., 'mutationFn'> wrapper from UseUpdateOptions type definition
packages/ra-core/src/dataProvider/useDeleteMany.ts Removed Omit<..., 'mutationFn'> wrapper from UseDeleteManyOptions type definition
packages/ra-core/src/dataProvider/useDelete.ts Removed Omit<..., 'mutationFn'> wrapper from UseDeleteOptions type definition
packages/ra-core/src/dataProvider/useCreate.ts Removed Omit<..., 'mutationFn'> wrapper from UseCreateOptions type definition

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +308 to +311
> = UseMutationOptions<
RecordType,
ErrorType,
Partial<UseUpdateMutateParams<RecordType>>
Copy link

Copilot AI Nov 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Allowing mutationFn in the options type creates an inconsistent API. The type now accepts mutationFn, but the runtime implementation (line 113) will silently override any user-provided value since it's defined after ...mutationOptions is spread (line 110). This mismatch between the type system and runtime behavior could confuse API consumers. Consider either: (1) keeping the Omit<..., 'mutationFn'> to prevent users from passing it, or (2) restructuring the code to respect a user-provided mutationFn if that's the intended behavior.

Suggested change
> = UseMutationOptions<
RecordType,
ErrorType,
Partial<UseUpdateMutateParams<RecordType>>
> = Omit<
UseMutationOptions<
RecordType,
ErrorType,
Partial<UseUpdateMutateParams<RecordType>>
>,
'mutationFn'

Copilot uses AI. Check for mistakes.
Comment on lines +290 to +293
> = UseMutationOptions<
Array<RecordType['id']>,
MutationError,
Partial<UseUpdateManyMutateParams<RecordType>>
Copy link

Copilot AI Nov 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Allowing mutationFn in the options type creates an inconsistent API. The type now accepts mutationFn, but the runtime implementation (line 109) will silently override any user-provided value since it's defined after ...mutationOptions is spread (line 106). This mismatch between the type system and runtime behavior could confuse API consumers. Consider either: (1) keeping the Omit<..., 'mutationFn'> to prevent users from passing it, or (2) restructuring the code to respect a user-provided mutationFn if that's the intended behavior.

Suggested change
> = UseMutationOptions<
Array<RecordType['id']>,
MutationError,
Partial<UseUpdateManyMutateParams<RecordType>>
> = Omit<
UseMutationOptions<
Array<RecordType['id']>,
MutationError,
Partial<UseUpdateManyMutateParams<RecordType>>
>,
'mutationFn'

Copilot uses AI. Check for mistakes.
Comment on lines +275 to +278
> = UseMutationOptions<
RecordType,
MutationError,
Partial<UseDeleteMutateParams<RecordType>>
Copy link

Copilot AI Nov 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Allowing mutationFn in the options type creates an inconsistent API. The type now accepts mutationFn, but the runtime implementation will silently override any user-provided value since it's defined after ...mutationOptions is spread. This mismatch between the type system and runtime behavior could confuse API consumers. Consider either: (1) keeping the Omit<..., 'mutationFn'> to prevent users from passing it, or (2) restructuring the code to respect a user-provided mutationFn if that's the intended behavior.

Suggested change
> = UseMutationOptions<
RecordType,
MutationError,
Partial<UseDeleteMutateParams<RecordType>>
> = Omit<
UseMutationOptions<
RecordType,
MutationError,
Partial<UseDeleteMutateParams<RecordType>>
>,
'mutationFn'

Copilot uses AI. Check for mistakes.
Comment on lines +301 to +304
> = UseMutationOptions<
Array<RecordType['id']> | undefined,
MutationError,
Partial<UseDeleteManyMutateParams<RecordType>>
Copy link

Copilot AI Nov 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Allowing mutationFn in the options type creates an inconsistent API. The type now accepts mutationFn, but the runtime implementation will silently override any user-provided value since it's defined after ...mutationOptions is spread. This mismatch between the type system and runtime behavior could confuse API consumers. Consider either: (1) keeping the Omit<..., 'mutationFn'> to prevent users from passing it, or (2) restructuring the code to respect a user-provided mutationFn if that's the intended behavior.

Suggested change
> = UseMutationOptions<
Array<RecordType['id']> | undefined,
MutationError,
Partial<UseDeleteManyMutateParams<RecordType>>
> = Omit<
UseMutationOptions<
Array<RecordType['id']> | undefined,
MutationError,
Partial<UseDeleteManyMutateParams<RecordType>>
>,
'mutationFn'

Copilot uses AI. Check for mistakes.
Comment on lines +261 to +264
> = UseMutationOptions<
ResultRecordType,
MutationError,
Partial<UseCreateMutateParams<RecordType>>
Copy link

Copilot AI Nov 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Allowing mutationFn in the options type creates an inconsistent API. The type now accepts mutationFn, but the runtime implementation (line 116) will silently override any user-provided value since it's defined after ...mutationOptions is spread (line 113). This mismatch between the type system and runtime behavior could confuse API consumers. Consider either: (1) keeping the Omit<..., 'mutationFn'> to prevent users from passing it, or (2) restructuring the code to respect a user-provided mutationFn if that's the intended behavior.

Suggested change
> = UseMutationOptions<
ResultRecordType,
MutationError,
Partial<UseCreateMutateParams<RecordType>>
> = Omit<
UseMutationOptions<
ResultRecordType,
MutationError,
Partial<UseCreateMutateParams<RecordType>>
>,
'mutationFn'

Copilot uses AI. Check for mistakes.
Copy link
Contributor

@slax57 slax57 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually Copilot is right! (for once...)
However attempting to fix the issue uncovered other TS issues that I was unable to solve so far.
We'll need more time to properly fix this PR.
Btw this PR should target master rather that next IMHO.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

RFR Ready For Review

Development

Successfully merging this pull request may close these issues.

3 participants