-
-
Notifications
You must be signed in to change notification settings - Fork 424
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
fix(dark-mode): avoid localStorage overwrite by system dark mode status #636
base: master
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: bd1cd0c The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
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.
LGTM
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 this helps so much!
Problem Description
While using
useDarkMode
, I noticed that after refreshing the page, the value saved inlocalStorage
does not take effect (observing thatlocalStorage
is always overwritten by the current system's dark mode status), even though I set theinitializeWithValue
option to true (the documentation states, "If true (default), the hook will initialize by readinglocalStorage
").Cause of the Issue
The value saved in
localStorage
does not take effect on the next page load because the value retrieved byconst isDarkOS = useMediaQuery()
immediately overwrites the result read fromuseLocalStorage()
when theuseDarkMode
hook initializes (sinceuseIsomorphicLayoutEffect
executes immediately). This behavior is unexpected, causing the functionality of saving the dark mode state tolocalStorage
to fail.Fix
Add
const allowDarkOSChange = useIsMounted()
. Before the page is mounted, prevent applying the dark mode result fromisDarkOS
, rather than immediately executing and overwriting the value fromlocalStorage
.Test cases have been added to verify the existence of this issue, and the problem was confirmed. The code modification has passed the tests successfully.
fixes #512