-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
feat: add hydrate method, make hydration treeshakeable #10497
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
Conversation
🦋 Changeset detectedLatest commit: c2dfc0c The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice! this will be great!!
Co-authored-by: Ben McCann <[email protected]>
…into treeshakeable-hydration
/** @type {Node} */ | ||
let target_node = anchor_node; | ||
if (current_hydration_fragment !== null) { | ||
if (hydrating) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we need this (and similar) guards? surely we're already only calling functions like hydrate_block_anchor
when we're hydrating. rollup is smart enough to deal with it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Depends on the method, in this case there's no if block at the call site because it would result in more if block code overall.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is what I was talking about last week in our call. I wasn't seeing the tree shaking working as expected by Rollup. However, if you put into this boolean guard then it works great – something I didn't do.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mostly looks good but i have some questions, will finish reviewing soon — making sure we don't merge in the meantime
Introduces a new
hydrate
method which does hydration. Refactors code so that hydration-related code is treeshaken out when not using that method.closes #9533
part of #9827
Before submitting the PR, please make sure you do the following
feat:
,fix:
,chore:
, ordocs:
.Tests and linting
pnpm test
and lint the project withpnpm lint