Fix mutateCallback types

[avocadowastaken]

1. Promise<Data | undefined> because mutate implementation handles undefined values.

mutate implementation can pass undefined values to the mutateCallback:

swr/src/use-swr.ts

Lines 138 to 144 in 9e734aa

	if (_data && typeof _data === 'function') {
	// `_data` is a function, call it passing current cache value
	try {
	data = await _data(cache.get(key))
	} catch (err) {
	error = err
	}

[shuding]

Actually mutate doesn't accept undefined as the data. The implementation is for specific cases like mutate(key) (no data specified, only revalidate).

Generally we don't want you to do this:

mutate(key, () => undefined)

Because in SWR undefined is like a special placeholder which means "no data available", so you can't set undefined as the data.

[avocadowastaken]

Actually mutate doesn't accept undefined as the data. The implementation is for specific cases like mutate(key) (no data specified, only revalidate).

Generally we don't want you to do this:

mutate(key, () => undefined)

Because in SWR undefined is like a special placeholder which means "no data available", so you can't set undefined as the data.

Thanks for clearing things out.

---

The main problem that we've faced is that sometimes when we're calling mutate with callback and currentValue comes as undefined, so mutateCallback should look like that:

export type mutateCallback<Data = any> = (
-  currentValue: Data
+  currentValue: undefined | Data
) => Promise<Data> | Data

But this leads to another problem when we have only part of the value to update:

function updateStatus(guid: string, status: string): void {
  mutate(`/user/${guid}`, (user) => ({ ...user, status }));
}

It will create { status: string } object when we do not have currentValue, so we want to check if currentValue is undefined

function updateStatus(guid: string, status: string): void {
  mutate(`/user/${guid}`, (user) => !user ? user : ({ ...user, status }));
}

Here TS will fail because it expects the result of the mutateCallback to be Promise<Data> | Data, so we have to change mutateCallback types to:

  export type mutateCallback<Data = any> = (
-  currentValue: Data
- ) => Promise<Data> | Data
+  currentValue: undefined | Data
+ ) => Promise<Data | undefined> | Data | undefined

[avocadowastaken]

@shuding hey, sorry to disturb, but can you check one more time please, and if it can't be merged I'll just close this PR.

[shuding]

@umidbekkarimov Sorry for the delay!

Now I understand your use case and I think that makes sense. We can have it. 👍
The only request is can we add a test case for it, since it's a new behavior? It's fine if you are busy and we can take this PR and open an issue (for the test case) for now.

Thanks for the contribution!

[avocadowastaken]

@umidbekkarimov Sorry for the delay!

Now I understand your use case and I think that makes sense. We can have it. 👍
The only request is can we add a test case for it, since it's a new behavior? It's fine if you are busy and we can take this PR and open an issue (for the test case) for now.

Thanks for the contribution!

Done. I could not add checks in TS because #741 was merged to next, but I add repro in the ts playground: https://www.typescriptlang.org/play?#code/KYDwDg9gTgLgBDAnmYcC2BXGBDGwDC2ANkQEbYDGA1gDwAiu2cAvHNgHaIB8LcAFACg4cChihRg7GADViGYAC44GdgBNgAMwCW7YKrgAfOAxwCAlCx4AFKBDRaAzsBor123fqMnsPI6806eobGjAKgkLAIyKgSDpDsTgCSUsBQGpTO3gA0cACi4tA8rADeQnCqjAD8St5lqbZQ1XkFUGUSAG7EWhV4SnwWzNa29k40pBAQRMAcXGWYOL38ZcI92E3ewTZ2jpmMvuhYuATEZJS03lxZy3AOABYQGESqAErAnUTdR03jk9PsZQMhttRhs-GoAh5ZsJHLIPj0dABzJQ-KYcAQAXwEAgA9Ni4AABGAOAC04WAFBgpJaAgoEAS8Fi8ScSkZdKSKTSGRo7AwaFIqRy+QaRTgxUxAlZCWAADp5kc+HxROJJDI5MBAaKykqJFJYfJpTAIAAxLQgIK4uDpLREBxlLHCC0AQQoFGAYCJynB7j0ZS0Gn4AEJtSq9erNcJhBIYGJ2J63IFVABuMriyPAaNQWPB3Vq5PosxAA

[shuding]

Thanks! This is already looking great. When we move to next strict mode will work 👍

[Zertz]

First, thanks for improving the types, this will definitely help make apps more robust!

I'm not sure where to ask this and I understand swr is 0.x but are type breaks like this considered a breaking change or more like a bug?

[shuding]

@Zertz This PR is a bug fix and should be available in the latest release already (if not please let me know).

But regarding TS strict mode, I'd consider it a pretty breaking change because we need to revamped the core code base and the tests. The goal is to also have better type names in the 1.0 version.