fix: default to fetch type in keyed mutator

[linkvt]

Closes #2744

Hi,

as a few other also noticed the typings used in the bound mutator do not default anymore to the data type used in useSWR<ThisOne> and needs to be specified explicitly everytime right now.

Looking at the other types surrounding the KeyedMutator type it seems reasonable to default to the data type specified at useSWR<...>, e.g. as done here:

swr/_internal/src/types.ts

Lines 372 to 381 in e510955

	export interface ScopedMutator {
	/**
	* @typeParam Data - The type of the data related to the key
	* @typeParam MutationData - The type of the data returned by the mutator
	*/
	<Data = any, MutationData = Data>(
	matcher: (key?: Arguments) => boolean,
	data?: MutationData | Promise<MutationData> | MutatorCallback<MutationData>,
	opts?: boolean | MutatorOptions<Data, MutationData>
	): Promise<Array<MutationData | undefined>>

Let me know if and where a test could be added if you want to test the typings.

Thanks, Vincent

[orca-security-us]

Orca Security Scan Summary

Status	Check	Issues by priority
Passed	Secrets	0   0   0   0

[codesandbox-ci]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 4b1bd9a:

Sandbox	Source
SWR-Basic	Configuration
SWR-States	Configuration
SWR-Infinite	Configuration
SWR-SSR	Configuration

[linkvt]

Hello @shuding or @huozhi , would you mind looking at this tiny PR fixing the types of a keyed mutator? I just rebased the code, let me know if something is missing here.
Thanks!

[huozhi]

Hi, could you also add a types test for this case so we can avoid the regression later? Thanks

[promer94]

Thank you!

[huozhi]

@promer94 thanks for adding the test, maybe we should create an issue to follow up on this? it feels like a regression for mutate() API types

[promer94]

The original pr #2708 did not fix this correctly so there is no regression here.

I also think we should not support this because the type implementation would be really complicated and not useful in practice. You should always make sure you data has same structure as cache when using the bounded mutate fn.

I just keep it here as a reference.

[huozhi]

right the type is number, I updated them in #2781

[AmirHosseinKarimi]

Hey guys. Thanks for improving the SWR.
When we can have these changes on the new version?
Is there any routine release plan?

[huozhi]

@AmirHosseinKarimi A few type improvements including this are landed in 2.2.3

[ckohen]

@promer94 Why was this change reverted? It makes the generic completely useless and returning different MutationData to be passed to populateCache impossible with proper types.

This probably needs to be overload based to function properly, otherwise MutationData will just be inferred improperly from the data arg, although it is mostly possible to do without overloads with some rather extensive types:

type KeyedMutator<Data> = <MutationData = Data>(data?: MutationData | Promise<MutationData | undefined> | MutatorCallback<MutationData>, opts?: boolean | Omit<MutatorOptions<Data, MutationData>, 'populateCache'> & (MutationData extends Data ? Pick<MutatorOptions<Data, Data>, 'populateCache'> : { populateCache: Exclude<MutatorOptions<Data, MutationData>['populateCache'], boolean> })) => Promise<Data | MutationData | undefined>;

I have tested the above and it works as expected, except in the case where opts is not passed or is a boolean. This is why I suggested the overload approach, since if options is not passed as an object, typescript doesn't enforce it being passed.

[simenbrekken-visma]

Not sure if I understood the above changes completely, but after upgrading from 2.0.3 to 2.2.3 we had to update all our populateCache methods:

Before (2.0.3)

const { data = [], mutate } = useSWR<ListItem[]>('/api/list', fetcher);

const addListItem = (name: string) => {
    mutate(
        async () => {
            const item: ListItem = await createListItem(name);

            return item;
        },
        {
            populateCache: (result, currentData = []) => [
                ...currentData,
                result,
            ],
        },
    );
};

After (2.2.3)

const { data = [], mutate } = useSWR<ListItem[]>('/api/list', fetcher);

const addListItem = (name: string) => {
    mutate(
        async () => {
            const item: ListItem = await createListItem(name);

            return [item];
        },
        {
            populateCache: (result, currentData = []) => [
                ...currentData,
                ...result,
            ],
        },
    );
};

We may have missed a generic that can be passed to work around this, but having to modify our mutators to return the same type as given to useSWR feels a bit clunky.