fix: not clear promise of revalidate after mutating

[MoonBall]

Fix #828.

See added test case for detail.

[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 7e07e30:

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

[promer94]

It might be batter if we delete the promise conditionally

Suggested change

	const updaters = CACHE_REVALIDATORS[key]
	const updaters = CACHE_REVALIDATORS[key]
	if (updaters && updaters.length > 0) {
	const promises = []
	for (let i = 0; i < updaters.length; ++i) {
	promises.push(
	updaters[i](!!shouldRevalidate, data, error, undefined, i > 0)
	)
	}
	// return new updated value
	return Promise.all(promises).then(() => {
	if (error) throw error
	return cache.get(key)
	})
	} else {
	// clear dedupingInterval of an unmounted key
	delete CONCURRENT_PROMISES[key]
	delete CONCURRENT_PROMISES_TS[key]
	}

[MoonBall]

It might be batter if we delete the promise conditionally

Thanks for your suggestions.

I deleted the promise conditionally at first. I think the conditional statement should be !!shouldRevalidate && (updaters && updaters.length > 0) if deleting it conditionally. This conditional statement is little of complicated and hard to understand.

However, Deleting the promise after mutating is easier to understand. I would like to modify my comment to express it clearly.

[promer94]

I think the root cause of the original issue might not be the dedupingInterval ?

import { mutate } from 'swr'
mutate('key')

When using the global mutate

• If there are mounted hook instanced - calling mutate('key') will revalidate the data and update the cache using the hook's fetcher
• If all the hooks are unmounted - calling mutate('key') will do nothing - you need provide the data or function to update the cache

swr/src/use-swr.ts

Lines 391 to 402 in f787624

	if (
	MUTATION_TS[key] !== undefined &&
	// case 1
	(startAt <= MUTATION_TS[key] ||
	// case 2
	startAt <= MUTATION_END_TS[key] ||
	// case 3
	MUTATION_END_TS[key] === 0)
	) {
	setState({ isValidating: false })
	return false
	}

The dedupingInterval will be ignored by these conditions.

[MoonBall]

You are right. But The mutate('key') should support to clear revalidate promise although not providing the second param.

Some backend post api doesn't return the data, so RD should use useSWR as follow:

fetch('/api/user/edit', { method: 'post', name })
mutate('/api/user')

and then they want to fetch new data if they need.

[MoonBall]

mutate('key') has different behavior according to if there are mounted hook. This API should be better.

[MoonBall]

@promer94 Thanks for your review.

It's better to keep mutate() to revalidate when shouldRevalidate is true. It's a good API design.

I made a new commit, PTAL.

[promer94]

I still have some concerns about clearing the deduplicatedInterval in mutate function. I am not sure this is the best way to solve the problem. We might need more discussions about this.

[promer94]

if hook is unmounted

mutate('key') // This will not clear the deduplicatedInterval

mutate("key", () => undefined) // This will clear the deduplicatedInterval
mutate("key", () => Promise.resolve(undefined)) // This will also clear the deduplicatedInterval

swr/src/use-swr.ts

Line 107 in f787624

if (typeof _data === 'undefined') return trigger(_key, shouldRevalidate)

[MoonBall]

if hook is unmounted, the above three cases will not clear the deduplicatedInterval because updaters && updaters.length is true.

[MoonBall]

I has an irrelevant problem. How did you embed code like this:

[promer94]

https://docs.github.com/cn/github/managing-your-work-on-github/creating-a-permanent-link-to-a-code-snippet

[shuding]

Thank you! I think this is a pretty neat trick to work around that special case. I'll look into it deeper to see if there's any other problems, since this is not a perfect solution.