Skip to content

feature: provide config.isPaused indicating idle state and stop revalidation #845

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

Merged
merged 3 commits into from
Dec 31, 2020

Conversation

huozhi
Copy link
Member

@huozhi huozhi commented Dec 31, 2020

Description

give user ability to decide whether they want revalidation in some edge cases:

  • page transition on slow connection that revalidations can cause more errors
  • page is in background sometimes they don't want any revalidation

Changes

  • add config.isPaused()
  • stop doing revalidation on config.isPaused() is true

related discussion 841

@codesandbox-ci
Copy link

codesandbox-ci bot commented Dec 31, 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 5af2834:

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

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.

This is a good start! We can have built-in beforeunload event handling and other features based on this. 2 questions:
Why using a function (isIdle: () => boolean) instead of a value (isIdle: boolean)? Value should be more consistent with our existing APIs. Another concern is that I think pause might be a better name, idle makes me feel that it means SWR is currently not active (not fetching anything).

@huozhi
Copy link
Member Author

huozhi commented Dec 31, 2020

@shuding Thanks for the nice feedback! I was thinking the paused state might be related to sth (like DOM state) out of react, i.e. swr can be paused when document.visible is false like isOnline. So I feel we can rename to isPaused but keep it as a functional property?

will update PR by comments 🙏

@shuding
Copy link
Member

shuding commented Dec 31, 2020

But the behavior of this option is to pause requests right (everything will be skipped)?

@huozhi
Copy link
Member Author

huozhi commented Dec 31, 2020

yup, I've changed to config.isPuased() to let user add some condition like

isPuased() { return document.visibility === 'hidden' }

and add one more case in revalidation catch branch, which is ignoring the errors when promises are been rejected under paused state

@huozhi huozhi requested a review from shuding December 31, 2020 18:56
@shuding
Copy link
Member

shuding commented Dec 31, 2020

Thanks, this is a good fix!

@shuding shuding merged commit 00c5847 into vercel:master Dec 31, 2020
@huozhi huozhi deleted the is-idle branch January 1, 2021 02:25
@huozhi huozhi changed the title feature: provide config.isIdle indicating idle state and stop revalidation feature: provide config.isPaused indicating idle state and stop revalidation Jan 8, 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.

2 participants