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

Executing useBreakpoints isormophically no longer triggers a Hydration mismatch error or rendering bugs. #10886

Merged
merged 1 commit into from
Oct 5, 2023

Conversation

jesstelford
Copy link
Contributor

@jesstelford jesstelford commented Oct 5, 2023

To reproduce:

  1. Visit https://polaris.shopify.com/examples/card-with-rounded-corners which should show rounded corners from 768px viewport width and above.
  2. Set your viewport width to 768px or more (ie; so it matches md breakpoint or more)
  3. Refresh the page
  4. Notice there are no rounded corners
  5. Resize your viewport to be 767px or less.
  6. Resize your viewport to be 768px or more.
  7. Notice the rounded corners now appear.

Before
no-rounded-corners

This PR
no-rounded-corners-fixed


More details

This is an issue anywhere useBreakpoints() is used with SSR.

For example, with <Banner>;

  1. Style props are being set based on a variable smUp: https://github.com/Shopify/polaris/blob/next/polaris-react/src/components/Banner/Banner.tsx#L202-L205
  2. smUp is defined above as const {smUp} = useBreakpoints();
  3. In SSR, useBreakpionts initially returns a call to getMatches(), forwarding any "default" parameters (none are passed in by <Banner>)
  4. In SSR, getMatches() returns false for all breakpoints unless a default is set
  5. Back in useBreakpoints(), an effect is used to setup media query match event listeners. These trigger each time the matchy-ness of a media query changes, but not in between.
    • For example, between sm (>490px) to md (<768px), the matchy-ness of the media queries are not changing, so none of the event listeners will be called. But once the window hits 768px, md will now match, and so the event listener is triggered which updates state and re-renders the component.
    • Note also that when doing SSR, the initial values of the getMatches() calls will differ client-side than server-side, resulting in a "Hydration error", but critically will not re-render with the client-side values.
  6. So the end result; smUp === false in SSR, then is immediately smUp === true in CSR causing a client side "Hydration error", but continues to render with the SSR values until the window is resized past a breakpoint triggering a state update to reflect the new breakpoint.

@jesstelford jesstelford requested review from a team as code owners October 5, 2023 01:29
Copy link
Contributor

@gwyneplaine gwyneplaine left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

xsOnly: true,
xsUp: true,
renders++;
breakpoints = useBreakpoints({
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The useEffect inside of useBreakpoints will cause this component to render twice during mount, so we assert the initial and subsequent values are what we expect.

xsDown: true,
xsOnly: true,
xsUp: true,
});
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moving the assertions after the mount call (instead of inside it) gives useBreakpoint's useEffect a chance to run, so we're asserting against the final values, not the SSR defaults.

@jesstelford jesstelford merged commit d2518f2 into main Oct 5, 2023
17 checks passed
@jesstelford jesstelford deleted the docs-ssr-responsive-render branch October 5, 2023 04:32
This was referenced Oct 5, 2023
aaronccasanova added a commit that referenced this pull request Oct 6, 2023
…a Hydration mismatch error or rendering bugs." (#10916)

Reverts #10886

Moving this change to the `next` branch cc @jesstelford
aaronccasanova added a commit that referenced this pull request Oct 6, 2023
#10918)

…riggers a Hydration mismatch error or rendering bugs." (#10916)"

This reverts #10916 to reapply
#10886

cc @jesstelford
mrcthms pushed a commit that referenced this pull request Oct 12, 2023
…a Hydration mismatch error or rendering bugs." (#10916)

Reverts #10886

Moving this change to the `next` branch cc @jesstelford
mrcthms pushed a commit that referenced this pull request Oct 12, 2023
#10918)

…riggers a Hydration mismatch error or rendering bugs." (#10916)"

This reverts #10916 to reapply
#10886

cc @jesstelford
mrcthms pushed a commit that referenced this pull request Oct 12, 2023
…a Hydration mismatch error or rendering bugs." (#10916)

Reverts #10886

Moving this change to the `next` branch cc @jesstelford
mrcthms pushed a commit that referenced this pull request Oct 12, 2023
#10918)

…riggers a Hydration mismatch error or rendering bugs." (#10916)"

This reverts #10916 to reapply
#10886

cc @jesstelford
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