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/prerender hydration #895

Merged
merged 4 commits into from
Jan 19, 2022
Merged

Fix/prerender hydration #895

merged 4 commits into from
Jan 19, 2022

Conversation

rschristian
Copy link
Member

Hydration vs rendering is determined by whether script[type=isodata] can be found, however, #585 limited this to only existing when returning prerender data.

I attempted to pick through the slack as noted here , though I'm still not sure I haven't missed something that would indicate this is intentional, so let me know. This just seemed like the simplest solution to bring this up with.

If we don't want preact-iso to return isodata we'll probably need some other marker to decide whether to hydrate or not?

@changeset-bot
Copy link

changeset-bot bot commented Nov 15, 2021

🦋 Changeset detected

Latest commit: ac05e3a

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
preact-iso Patch
wmr Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@rschristian rschristian marked this pull request as draft November 15, 2021 15:32
@github-actions
Copy link
Contributor

github-actions bot commented Nov 15, 2021

Size Change: 0 B

Total Size: 6.31 kB

ℹ️ View Unchanged
Filename Size
packages/preact-iso/.dist/index.********.js 1.72 kB
packages/preact-iso/.dist/prerender.********.js 349 B
packages/preact-iso/hydrate.js 290 B
packages/preact-iso/index.js 195 B
packages/preact-iso/lazy.js 594 B
packages/preact-iso/prerender.js 629 B
packages/preact-iso/router.js 2.54 kB

compressed-size-action

@rschristian rschristian marked this pull request as ready for review November 15, 2021 15:46
@rschristian rschristian marked this pull request as draft November 15, 2021 15:51
@rschristian rschristian marked this pull request as ready for review November 15, 2021 16:05
@rschristian
Copy link
Member Author

There's a couple of flakey tests that seem to be taking turns on failing, reran the CI a few times now with no luck.

Copy link
Member

@developit developit left a comment

Choose a reason for hiding this comment

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

Good catch, we never actually finished making this change.

The idea was that isodata should be specific to preact-iso (though there's not currently an API exposed for generating it). WMR should also inject data into a script if given result.data, but it needs to use a different name.

My preference:

  • <script type="wmrdata"></script> gets appended to HTML by WMR if result.data is provided.
  • preact-iso's prerender() appends <script type="isodata"></script> to the HTML it generates.
  • preact-iso's hydrate() uses script[type="isodata"] to trigger hydration.

@rschristian
Copy link
Member Author

That makes sense. Should be corrected now, assuming I'm not missing something.

CI gods finally graced me with a pass too.

@developit developit merged commit add1bef into main Jan 19, 2022
@developit developit deleted the fix/prerender-hydration branch January 19, 2022 22:45
@github-actions github-actions bot mentioned this pull request Jan 19, 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.

None yet

2 participants