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

perf: avoid unnecessary re-renders with the suspense mode #979

Merged
merged 2 commits into from
Feb 22, 2021

Conversation

koba04
Copy link
Collaborator

@koba04 koba04 commented Feb 15, 2021

Fixes #454
Currently, SWR doesn't apply the performance optimization mentioned in https://swr.vercel.app/advanced/performance with the suspense option, and it leads to unnecessary re-renders.

I've applied the same optimization using stateDependencies for the suspense mode.
Is there any reason not to apply the optimization for the suspense mode?

This is a fork of reproduction in #454, which is based on this PR.

@codesandbox-ci
Copy link

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 0771899:

Sandbox Source
SWR-Basic Configuration
SWR-States Configuration
SWR-Infinite Configuration
great-lumiere-441zk Issue #454

@shuding
Copy link
Member

shuding commented Feb 21, 2021

Sorry for the delay and thank you for this PR!

Is there any reason not to apply the optimization for the suspense mode?

Mainly because that Suspense is still an experimental feature, and we are not implementing it 100% correctly (mostly with Concurrent mode). Also according to facebook/react#17526 (comment), I feel that it's better to not optimize it too early, and get everyone relying on it too much.

@koba04
Copy link
Collaborator Author

koba04 commented Feb 22, 2021

@shuding Thank you! I understand your concern.
I think rendering 6 times with a normal case (const { data } = useSWR(url, fetcher, { suspense: true })) is a bit surprising and might be a problem in complex applications. IMHO, this optimization doesn't rely on Suspense, which is just avoiding unnecessary rendering, so it would be nice if the suspense option has this optimization even this is an experimental option.

@shuding
Copy link
Member

shuding commented Feb 22, 2021

Yeah I think I agree with applying those perf optimizations for now since they are not specifically for Suspense. Also the code is much cleaner 👍

@shuding shuding merged commit 1a3f3a9 into vercel:master Feb 22, 2021
@koba04 koba04 deleted the unnecessary-rerender-with-suspense branch February 23, 2021 15:34
@koba04
Copy link
Collaborator Author

koba04 commented Feb 23, 2021

Thank you!

This was referenced Mar 12, 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.

Re-rendering too often with suspense mode
2 participants