Skip to content
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(useMediaQuery): On misconfiguration, cause hydration error instea… #1042

Merged
merged 1 commit into from
Dec 7, 2022

Conversation

ArttuOll
Copy link
Contributor

@ArttuOll ArttuOll commented Dec 7, 2022

What is the current behavior, and the steps to reproduce the issue?

When the hook is misconfigured, that is, when initializeWithValue is true during SSR, the hooks crashes instead of producing a hydration error, which is not in line with the behavior of the rest of the isomorphic hooks.

What is the expected behavior?

Hydration error instead of a crash.

How does this PR fix the problem?

If the rendering environment is not a browser, set initializeWithValue to false.

This solution makes me wonder: what is the purpose of the initializeWithValue option anymore? If it is set to false during SSR anyway, why have it at all? Is there a usecase for having the hook return undefined on first render when doing client-side rendering?

Checklist

  • Have you read contribution guideline?
  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Is there an existing issue for this PR?
  • Have the files been linted and formatted?
  • Have the docs been updated to match the changes in the PR?
  • Have the tests been updated to match the changes in the PR?
  • Have you run the tests locally to confirm they pass?

…d of SSR crash

In #1000 it was noted, that the current implementation does not follow the convention of causing a
hydration error on misconfiguration. This is now fixed.

re #1000
@ArttuOll ArttuOll requested a review from xobotyi December 7, 2022 18:19
@codecov
Copy link

codecov bot commented Dec 7, 2022

Codecov Report

Merging #1042 (2bee68e) into master (087b2b1) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #1042   +/-   ##
=======================================
  Coverage   99.61%   99.62%           
=======================================
  Files          61       61           
  Lines        1049     1054    +5     
  Branches      166      169    +3     
=======================================
+ Hits         1045     1050    +5     
  Misses          2        2           
  Partials        2        2           
Impacted Files Coverage Δ
src/useMediaQuery/useMediaQuery.ts 100.00% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@ArttuOll ArttuOll merged commit 46e5bcc into master Dec 7, 2022
@ArttuOll ArttuOll deleted the pr/fix-1000-2 branch December 7, 2022 18:55
github-actions bot pushed a commit that referenced this pull request Dec 7, 2022
## [20.0.1](v20.0.0...v20.0.1) (2022-12-07)

### Bug Fixes

* **useMediaQuery:** On misconfiguration, cause hydration error instead of SSR crash ([#1042](#1042)) ([46e5bcc](46e5bcc)), closes [#1000](#1000) [#1000](#1000)
@xobotyi
Copy link
Contributor

xobotyi commented Dec 7, 2022

🎉 This PR is included in version 20.0.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

2 participants