-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Optimize isValidating status #967
Optimize isValidating status #967
Conversation
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 269459b:
|
Thanks! I don't like the |
I don't like the false -> true -> false either and hope this PR will be merged. @shuding did you have a chance to think about it? Thank you! |
This would be very appreciated. It's odd to have the initial loading state set to |
Hi @chybisov @LRNZ09 @Kexin-Li! I think we can have this merged and release a beta version, once the above suggestion is merged 🙏 Thanks for your effort @Kexin-Li @huozhi! And I need to test it a bit in a large application before releasing a stable version, because this change is quite critical. But once landed, this is gonna help the performance a lot! Also, we need to update the docs when we release it: https://swr.vercel.app/advanced/performance |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Released as |
This PR is set the initial
isValidating
to be true if there is a request will be sent on mount.Currently, if we will fetching on mount, the
isValidating
will change likefalse -> true -> false
. If we usingisValidating
to indicate a loading status, which is a bit weird.I think it could be better to set the initial
isValidating
totrue
if we will start fetching. So it istrue -> false
.If there is anything wrong, please correct me. 🙈 Thanks!