Skip to content

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

Open
wants to merge 3 commits into
base: dev
Choose a base branch
from

Conversation

deqwin
Copy link

@deqwin deqwin commented Feb 15, 2020

What kind of change does this PR introduce? (check at least one)

  • Bugfix
  • Feature
  • Code style update
  • Refactor
  • Build-related changes
  • Other, please describe:

Does this PR introduce a breaking change? (check one)

  • Yes
  • No

If yes, please describe the impact and migration path for existing applications:

The PR fulfills these requirements:

If adding a new feature, the PR's description includes:

  • A convincing reason for adding this feature (to avoid wasting your time, it's best to open a suggestion issue first and wait for approval before working on it)

Other information:
Closes #11109

@deqwin deqwin requested review from yyx990803 and posva February 15, 2020 02:24
@deqwin deqwin changed the title ignore empty text vnode when hydrating (fix #11109) fix(ssr): ignore empty text vnode when hydrating (fix #11109) Feb 15, 2020
@posva
Copy link
Member

posva commented Feb 15, 2020

for GitHub: this closes #11109

const child = children[i]
// ignore empty text vnode
if (isUndef(child.tag) && isFalse(child.isComment) && child.text === '') {
continue
Copy link

@contribu contribu Feb 15, 2020

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

Copy link
Author

@deqwin deqwin Feb 16, 2020

Choose a reason for hiding this comment

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

Thanks for your suggestions.

Copy link
Author

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.

Copy link
Author

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.

Copy link

@contribu contribu Feb 16, 2020

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.

Copy link
Author

@deqwin deqwin Feb 16, 2020

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.

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.

deqwin and others added 2 commits February 15, 2020 19:38
Co-Authored-By: Eduardo San Martin Morote <posva@users.noreply.github.com>
@OllysCoding
Copy link

Hi All, is there any update on this PR being merged? This is causing us issues in our of SSR apps

@RomainLanz
Copy link

RomainLanz commented Dec 21, 2024

Hey there! 👋🏻

Small bump to see if there is any way to see this being merged and released for Vue 3?

@posva
Copy link
Member

posva commented Dec 21, 2024

This is only for Vue 2. Check in the core repo if the issue exists

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.

Hydration fails when write like "<div><span>123</span>{{value}}</div>" in the template and the data "value" is a empty string
5 participants