happykit / flags

⛳️ Feature Flags for Next.js
https://happykit.dev
MIT License
1.02k stars 11 forks source link

Update to utilize the PageVisibility API instead of `focus` event #3

Closed stramel closed 3 years ago

stramel commented 3 years ago

Hello 👋, I was looking into @happykit/flags implementation for the revalidateOnFocus I noticed that it utilizes window.addEventListener('focus', ...) for handling the focus. I found a few issues with this implementation:

Thank you for taking the time to consider my suggestion!

vercel[bot] commented 3 years ago

@stramel is attempting to deploy a commit to the HappyKit Team on Vercel.

A member of the Team first needs to authorize it.

dferber90 commented 3 years ago

Wow, that's pretty neat! I wasn't aware of this API.

I looked into swr and it seems like they're doing it that way as well.

I'm currently working on the next iteration of @happykit/flags which will be out in a few days, and which is a complete rewrite. Your suggestion will make it into the rewrite. I don't want to release an update to the current version to avoid having to make people upgrade twice within a few days. So unfortunately I can't merge this PR but the approach will make it in!

In case you're curious, you can check out the rewrite under the next branch. There's an example folder there which is a Next.js application that uses the unpublished new client from the package folder.

Thanks again, seeing your contribution made me really happy :)

dferber90 commented 3 years ago

Your suggestion has made it into next now: https://github.com/happykit/flags/commit/30c9038cf9afa1b48b17cd422cca94ea92158167

Thanks again!