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

fix: revalidate with initialData when changing the key #961

Merged
merged 3 commits into from
Mar 9, 2021

Conversation

koba04
Copy link
Collaborator

@koba04 koba04 commented Feb 3, 2021

This fixes #947.
SWR seems not to revalidate when having initialData.

Another strange thing: when state contains empty string refetching happens. But if you change state to any string it returns initial data.

This PR doesn't fix the above that is also mentioned on #947.
I'll investigate it as another issue.

@codesandbox-ci
Copy link

codesandbox-ci bot commented Feb 3, 2021

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 655fec4:

Sandbox Source
SWR-Basic Configuration
SWR-States Configuration
SWR-Infinite Configuration
crimson-sunset-pdgv2 Issue #947

@koba04
Copy link
Collaborator Author

koba04 commented Feb 3, 2021

The test has been passed in this PR, but I think this shouldn't be passed.
What should SWR return as data with initialData when being revalidating?
I expect that it returns undefined, but SWR in this PR seems to return initialData even it's not an initial mount, so I have to fix the behavior.

  it('should revalidate even if initialData is provided', async () => {
    const fetcher = async (key) => {
      sleep(100);
      return key
    }

    function Page() {
      const [key, setKey] = useState("initial-data-returns-empty-string")
      const { data } = useSWR(key, fetcher, {
        initialData: 'Initial'
      })
      return (
        <div onClick={() => setKey('initial-data-returns-empty-string-update')}>
          hello, {data}
        </div>
      )
    }

    const { container } = render(<Page />)

    // render with the initial data
    await screen.findByText('hello, Initial')

    await act(() => sleep(1))
    fireEvent.focus(window)

    await screen.findByText('hello, initial-data-returns-empty-string')

    // change the key
    await act(() => sleep(1))
    fireEvent.click(container.firstElementChild)

    await act(() => sleep(10))
    // THIS SHOULD BE FAILED, BUT IT'S PASSED
    expect(container.firstChild.textContent).toMatchInlineSnapshot(`"hello, Initial"`)

    // render with data the fetcher returns
    await screen.findByText('hello, initial-data-returns-empty-string-update')
  })

You can also see the behavior on https://codesandbox.io/s/swr-basic-forked-qjnnp?file=/src/App.js.

@koba04 koba04 force-pushed the fix-update-with-initial-data branch from b174b9a to 1fd4569 Compare February 4, 2021 14:11

// a request is still in flight
await act(() => sleep(10))
expect(container.firstChild.textContent).toMatchInlineSnapshot(`"loading"`)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm not sure what the expected value is.
But I expected SWR returns undefined as the return value of data when revalidating. But SWR returns the initialData Is it an expected behavior?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah initialData works like a fallback value currently. I think we need to better name for it, maybe just fallback. And for the real initialData, we need a completely new API, which works like a prefill. (also cc @huozhi)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That seems to be great 👍 fallback would be more intuitive than initialData for the behavior.

@koba04 koba04 force-pushed the fix-update-with-initial-data branch from a8e1b01 to 655fec4 Compare March 9, 2021 15:23
Copy link
Member

@shuding shuding left a comment

Choose a reason for hiding this comment

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

The fix looks good, thanks!

@shuding shuding merged commit 3f7e579 into vercel:master Mar 9, 2021
@koba04
Copy link
Collaborator Author

koba04 commented Mar 10, 2021

Thank you!

This was referenced Mar 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SWR uses old key while using ssr
2 participants