-
Notifications
You must be signed in to change notification settings - Fork 1.3k
fix: revalidateOnFocus not working on '< iOS 14' #889
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
Conversation
….addEventListener
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 b74ad97:
|
src/use-swr.ts
Outdated
) | ||
const hasWindow = typeof window !== 'undefined' | ||
const hasDocument = typeof document !== 'undefined' | ||
if (hasWindow) { |
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.
I think the 2 conditions doesn't have to be nested?
if (hasDocument) {
// focus revalidators
}
if (hasWindow) {
// other revalidators
}
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.
Does RN environment also have document
object(like the web)?
I think that if RN doesn't have document
object we should use window.addEventListener
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.
nope. it has window object but not document.
I double check again locally with a RN app. it shows that window doesn't have addEventListener
. and we need to check addEventListener. looks like your first commit also works well and it's shorter
I'll approve it on my side if can be reverted to the previous commit. sorry for my mistake and thanks for the question.
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.
Aha. Is this logic ONLY for web, not RN?
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.
yes, swr supports browser runtime by default. since react is been ported to different rumtime, people are trying adopt swr along with react apps. I was wondering if it's possible to make swr more flexible to be adopted among runtimes. seems not easy.
This reverts commit 5fd759c.
After merging this fix, it's now crashing when debugging react native and giving:
|
I found that revalidateOnFocus is not working on iOS 13.
To fix,
window.addEventListener('visibilityevent', callback)
should be replaced bydocument.addEventListener('visibilityevent', callback)
Ref: https://developer.mozilla.org/en-US/docs/Web/API/Document/visibilitychange_event
