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

read cache during render #794

Merged
merged 23 commits into from
Mar 28, 2021
Merged

read cache during render #794

merged 23 commits into from
Mar 28, 2021

Conversation

promer94
Copy link
Collaborator

@promer94 promer94 commented Nov 30, 2020

The problem

In swr, we have stateRef to store the data from the global cache.

  1. In first render, stateRef will read data from cache then render it.
  2. It will start a revalidation after the first mount and register an updater for the mutate function.
  3. During the revalidation, if the fetcher gives new data, the cache and stateRef will update and it will schedule a rerender. So useSWR will return the fresh data
  4. when mutate is called, the cache will update synchronously and updater will schedule a rerender to make useSWR return the fresh data
  5. when key is changed, it will start a revalidation. repeat 3.

So if the fetcher or the mutate function makes useSWR rerender, the stateRef is always sync with the cache.

But if useSWR rerender because of the sibling hooks rerender or the parent component rerender, it may return 'previous' data and stateRef is not sync with the cache

Related issues

Fixes #387: the parents rerender because of the mutate function, the child component will render twice, the first render will give a stale value and the second render which is scheduled by the mutate function.

Fixes #463: the sibling hooks rerender because of the fetcher, so other hook will return the 'stale' value.

Fixes #789: the parents rerender because of the timer. so the previous error is returned from the stateRef.

Repo

react 17 react experimental
#387 https://codesandbox.io/s/swr-387-hspvd https://codesandbox.io/s/swr-387-concurrent-mode-3j8iy
#463 https://codesandbox.io/s/swr-463-9gtkq https://codesandbox.io/s/swr-463-concurrent-mode-8ng8g
#789 https://codesandbox.io/s/swr-789-m21g2 https://codesandbox.io/s/swr-789-concurrent-mode-757nt

Possible solution

what if we do step 1 in every render ?

During the render, we pass the current cache to the dispatch function.

  • if stateRef is same as cache, nothing will happen.
  • if stateRef is not same as cache, the stateRef will be update and react will schedule another render.

    swr/src/use-swr.ts

    Lines 306 to 325 in 2248899

    let dispatch = useCallback((payload: actionType<Data, Error>) => {
    let shouldUpdateState = false
    for (let k in payload) {
    if (stateRef.current[k] === payload[k]) {
    continue
    }
    stateRef.current[k] = payload[k]
    if (stateDependencies.current[k]) {
    shouldUpdateState = true
    }
    }
    if (shouldUpdateState || config.suspense) {
    // if component is unmounted, should skip rerender
    // if component is not mounted, should skip rerender
    if (unmountedRef.current || !initialMountedRef.current) return
    rerender({})
    }
    }, [])

@codesandbox-ci
Copy link

codesandbox-ci bot commented Nov 30, 2020

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 2c02fa5:

Sandbox Source
SWR-Basic Configuration
SWR-States Configuration
SWR-Infinite Configuration
swr #387 PR
swr #387 concurrent mode PR
SWR #463 PR
SWR #463 concurrent mode PR
SWR #789 PR
SWR #789 concurrent mode PR

@shuding
Copy link
Member

shuding commented Nov 30, 2020

In first render, stateRef will read data from cache then render it.

This will possibly break the React hydration, we need to consider that. BTW, thanks for the sandboxes!!

@promer94
Copy link
Collaborator Author

In first render, stateRef will read data from cache then render it.

This will possibly break the React hydration, we need to consider that. BTW, thanks for the sandboxes!!

if we only sync the stateRef with cache when key changes, would it possibly break the React hydration ?

if (initialMountedRef && key !== keyRef.current) {
    stateRef.current = {
      data: initialData,
      error: initialError,
      isValidating: initialIsValidating
    }
  }

@promer94 promer94 marked this pull request as ready for review December 1, 2020 08:10
src/use-swr.ts Outdated Show resolved Hide resolved
@shuding
Copy link
Member

shuding commented Dec 8, 2020

I don't know if we should do that (setting a ref's .current during render) but couldn't think of a better way to handle it...

src/use-swr.ts Outdated Show resolved Hide resolved
@promer94 promer94 marked this pull request as ready for review March 11, 2021 08:22
@promer94 promer94 requested review from shuding and huozhi March 11, 2021 08:22
@shuding shuding removed the request for review from Timer March 11, 2021 11:22
src/use-swr.ts Outdated
Comment on lines 341 to 343
const keys = Object.keys(payload)
if (
keys.includes('data') &&
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
const keys = Object.keys(payload)
if (
keys.includes('data') &&
if (
'data' in payload &&

src/use-swr.ts Outdated Show resolved Hide resolved
src/use-swr.ts Outdated Show resolved Hide resolved
src/use-swr.ts Outdated Show resolved Hide resolved
@shuding shuding merged commit 37e90b1 into vercel:master Mar 28, 2021
@shuding
Copy link
Member

shuding commented Mar 28, 2021

Thank you so much for working on this! This is a pretty big change and lucky nothing breaks and the code is cleaner.

@shuding
Copy link
Member

shuding commented Mar 28, 2021

Published as swr@1.0.0-beta.1 🎉

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