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.

Sign up for GitHub

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

Jump to bottom

Use SSR rendered as initial html for runtime hydration test #4444

Merged
merged 11 commits into from
Jun 19, 2021
Merged

Use SSR rendered as initial html for runtime hydration test #4444

merged 11 commits into from
Jun 19, 2021

Conversation

tanhauhau
Copy link
Member

@tanhauhau tanhauhau commented Feb 22, 2020 •
edited
Loading

When running runtime (hydration) test, we are expecting the component to create the DOM while failing to claim elements from HTML.

This may fail to surface bugs such as hydrated DOM different from client rendered DOM.

Fixes:

  • claiming HtmlTag element
  • hydrating extra attributes from SSR

Not fixed for now:

  • claiming HtmlTag in head

@tanhauhau tanhauhau mentioned this pull request Feb 22, 2020
Closed
@tanhauhau tanhauhau mentioned this pull request Mar 11, 2020
Closed
@tanhauhau
Copy link
Member Author

@Conduitry I'm thinking about this PR, should we test for both:

  • claiming from empty HTML, repair/reconstruct when needed
  • claiming from SSR rendered HTML, making sure no breaking during hydration

@Conduitry
Copy link
Member

Yeah I think that makes sense to run the hydration tests starting with both the SSR output and from an empty document.

@tanhauhau
Copy link
Member Author

wow.. finally circling back to this PR! okay. let me tidy up the PR for that.

@tanhauhau tanhauhau force-pushed the tanhauhau/better-runtime-hydration-test branch from 83fb842 to b629ae4 Compare April 20, 2020 13:46
@tanhauhau
Copy link
Member Author

@Conduitry updated the PR!

@benmccann benmccann mentioned this pull request Jun 5, 2020
Closed
@benmccann benmccann mentioned this pull request May 3, 2021
Closed
@tanhauhau tanhauhau force-pushed the tanhauhau/better-runtime-hydration-test branch from b629ae4 to 3707b9a Compare June 15, 2021 03:02
benmccann
benmccann reviewed Jun 17, 2021
View reviewed changes
Copy link
Member

@benmccann benmccann left a comment

Choose a reason for hiding this comment

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

This is great. Thank you so much! 😄

If I put the change from #6204 back in then I get more test failures than I can count, so this makes me much more optimistic about our ability to make hydration improvements

test/runtime/index.ts Outdated Show resolved Hide resolved
test/runtime/samples/attribute-boolean-indeterminate/_config.js Outdated Show resolved Hide resolved
@tanhauhau tanhauhau force-pushed the tanhauhau/better-runtime-hydration-test branch from efac66e to 841137a Compare June 18, 2021 16:53
@benmccann benmccann merged commit 3f990a9 into sveltejs:master Jun 19, 2021
@benmccann benmccann mentioned this pull request Jun 28, 2021
Closed
@Conduitry Conduitry mentioned this pull request Oct 6, 2021
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@benmccann benmccann benmccann left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@tanhauhau @Conduitry @benmccann