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

Avoid synchronous loading on client if options.ssr is false #346

Merged
merged 2 commits into from
Jan 9, 2020

Conversation

jdb8
Copy link
Contributor

@jdb8 jdb8 commented May 20, 2019

Summary

Fixes #345. See description there for more details.

tl;dr: in cases where ssr: false is set, this PR fixes the way we render the component on the client to match what the server is doing, avoiding problematic SSR hydration mismatches (https://reactjs.org/docs/react-dom.html#hydrate).

Test plan

Updated the SSR example to add a new letter, G, which is rendered on the page with both ssr: true and ssr: false. Without the change applied, the DOM looks like this (incorrect):

image

with the change applied, the DOM now correctly looks like this:

image

As mentioned in #345, I can't see any existing integration tests for SSR: if I missed some or if you think it's worth adding some, I'm happy to help set that up. Thanks in advance for reviewing!

@gregberge
Copy link
Owner

I think it makes sense, but we should add a test on it. There are actually some tests, could you just add one to cover this case?

@gregberge
Copy link
Owner

Hello @jdb8, any test?

@jdb8
Copy link
Contributor Author

jdb8 commented Dec 2, 2019

@gregberge sorry, this got lost somehow.

There are actually some tests, could you just add one to cover this case?

Would you mind pointing me to a good place to add this ssr integration test? The issue I had was that this is a change that you'd ideally want to validate across the server/component boundary, to verify that SSR hydration works fully. Apart from the examples folder (which I added to), I couldn't see anywhere suitable for such an end-to-end test without setting up a new suite.

@gregberge gregberge merged commit 338bf55 into gregberge:master Jan 9, 2020
@gregberge
Copy link
Owner

Thanks!

fivethreeo pushed a commit to fivethreeo/loadable-components that referenced this pull request Nov 16, 2022
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.

Hydration can be incorrect if same component rendered on page in both ssr and non-ssr mode
2 participants