fix(ssr): ensure duplicate component VNodes render after hydration#14636
fix(ssr): ensure duplicate component VNodes render after hydration#14636cernymatej wants to merge 1 commit intovuejs:mainfrom
Conversation
📝 WalkthroughWalkthroughFixed an SSR hydration regression where duplicate component VNodes rendered after hydration would not display all instances. Modified Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/runtime-core/__tests__/hydration.spec.ts (1)
2142-2144: Prefer DOM-structure assertions over HTML substring matching.
toContainoninnerHTMLworks, but asserting element count/text directly is less brittle across formatting/serialization differences.♻️ Optional assertion hardening
- expect(container.innerHTML).toContain( - '<p>Click this <a href="#">link</a> and that <a href="#">link</a>.</p>', - ) + const p = container.querySelector('p') + expect(p).not.toBeNull() + expect(p!.querySelectorAll('a[href="#"]')).toHaveLength(2) + expect(p!.textContent).toBe('Click this link and that link.')🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/runtime-core/__tests__/hydration.spec.ts` around lines 2142 - 2144, Replace the brittle innerHTML substring assertion with DOM-structure checks: use container.querySelector('p') to get the paragraph and assert its textContent (or innerText) equals "Click this link and that link.", and use container.querySelectorAll('p a') to assert there are 2 anchor elements and each anchor's textContent equals "link"; update the failing expectation that currently references container.innerHTML to these element-based assertions (look for the test using the container variable in hydration.spec.ts).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/runtime-core/__tests__/hydration.spec.ts`:
- Around line 2142-2144: Replace the brittle innerHTML substring assertion with
DOM-structure checks: use container.querySelector('p') to get the paragraph and
assert its textContent (or innerText) equals "Click this link and that link.",
and use container.querySelectorAll('p a') to assert there are 2 anchor elements
and each anchor's textContent equals "link"; update the failing expectation that
currently references container.innerHTML to these element-based assertions (look
for the test using the container variable in hydration.spec.ts).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f202c71f-c0b7-404c-b8f0-4dce509ea1ed
📒 Files selected for processing (3)
packages/runtime-core/__tests__/hydration.spec.tspackages/runtime-core/__tests__/vnode.spec.tspackages/runtime-core/src/vnode.ts
|
see #14635 (comment) |
fix #14635
This should fix a problem where the renderer incorrectly enters the hydration path for a cloned component VNode during client-side rendering.
core/packages/runtime-core/src/renderer.ts
Lines 1341 to 1344 in 81615d3
I'm not familiar with the Vue codebase that much, so please let me know if there is something I might have overlooked. It's a little intimidating for me to touch the renderer 🙈😅
Summary by CodeRabbit
Release Notes
Bug Fixes
Tests