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 SSR issue with AutoId. #445

Merged
merged 1 commit into from
Mar 22, 2021
Merged

Fix SSR issue with AutoId. #445

merged 1 commit into from
Mar 22, 2021

Conversation

aappoalander
Copy link
Contributor

Description

This PR fixes AutoId issues with Server Side Rendering. Current implementation returns no children wrapped in AutoId when rendering in server for dehydrate/rehydrate. Desired behaviour would be to render the DOM in server, as closely to what is rendered in client, as possible. In this case, ids should be empty when rendered on server and patched in place with hooks on client. Current behaviour does not break anything, but may cause additional renders on client.

Motivation and Context

To enable efficient rendering and utilisation of SSR, rendered DOM's should match between server and client, with the exception of portals and and content that has to be positioned based on rendered content.

How Has This Been Tested?

Tested with Styleguidist, Create React App TypeScript project and SSR test project on Mac OS and Chrome. Also DOM contents were manually inspected after the change.

Release notes

  • Fix AutoId SSR issue and return children always, also on server but with empty string as id.

@aappoalander aappoalander added the bug Something isn't working label Mar 19, 2021
@aappoalander aappoalander self-assigned this Mar 19, 2021
Copy link
Contributor

@ketsappi ketsappi left a comment

Choose a reason for hiding this comment

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

Looking good!
Tested with SSR test bench; first without the fix to see the server not returning the content as expected and after this fix when it was returning the content as wanted.

@aappoalander aappoalander merged commit 713906e into develop Mar 22, 2021
@aappoalander aappoalander deleted the fix/auto-id-ssr branch April 12, 2021 13:26
@aappoalander aappoalander mentioned this pull request Jun 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants