-
-
Notifications
You must be signed in to change notification settings - Fork 33.7k
fix(ssr): ignore empty text vnode when hydrating (fix #11109) #11111
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
base: dev
Are you sure you want to change the base?
Conversation
for GitHub: this closes #11109 |
const child = children[i] | ||
// ignore empty text vnode | ||
if (isUndef(child.tag) && isFalse(child.isComment) && child.text === '') { | ||
continue |
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 results in child.elm === undefined and causes a crash in next patch.
Reproduction: deqwin/vue@fix_ssr_hydration...contribu:fix_ssr_hydration_crash
I think it is better to treat empty string nodes like #5117
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.
Thanks for your suggestions.
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.
The fact is not that the vnode tree has an extra empty text node, but the DOM tree is missing a necessary empty text node.
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.
@posva I have modified the code to deal with this problem and also updated the test.
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.
Thanks!
I am unsure if this causes reflow during hydration and if the existing hydration code allows it.
I think it shouldn't cause reflow if the existing hydration code disallows reflow.
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.
Yeah, it‘s worth further studying. However, it is necessary to keep the vnode structure and DOM structure consistent at all times. So I think it's ok to do this step here.
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.
Another solution is rendering empty text nodes as comment nodes in SSR like async placeholder or v-if="false".
I am unsure which solution is better.
Co-Authored-By: Eduardo San Martin Morote <posva@users.noreply.github.com>
…t patch when hydrating
Hi All, is there any update on this PR being merged? This is causing us issues in our of SSR apps |
Hey there! 👋🏻 Small bump to see if there is any way to see this being merged and released for Vue 3? |
This is only for Vue 2. Check in the core repo if the issue exists |
What kind of change does this PR introduce? (check at least one)
Does this PR introduce a breaking change? (check one)
If yes, please describe the impact and migration path for existing applications:
The PR fulfills these requirements:
dev
branch for v2.x (or to a previous version branch), not themaster
branchfix #xxx[,#xxx]
, where "xxx" is the issue number)If adding a new feature, the PR's description includes:
Other information:
Closes #11109