fix(Teleport): hydrate disabled Teleport with undefined target #11235
Conversation
…eleport should use the node of nextSibling
|
Hello, thanks for the PR. Is there any chance it will get merged? |
Size ReportBundles
Usages
|
bd3ac0e to
49581b7
Compare
|
Sorry for the bump, but is there any chance it will get merged? Thanks a lot for your work @linzhe141 🙏 |
Maybe the current modification is not correct, so it may not be merged. |
@vue/compiler-core
@vue/compiler-dom
@vue/compiler-sfc
@vue/compiler-ssr
@vue/reactivity
@vue/runtime-core
@vue/runtime-dom
@vue/server-renderer
@vue/shared
vue
@vue/compat
commit: |
|
Any update on this one? There's still dependencies relying on this changes 🙏 |
WalkthroughHydration logic for Teleport was extended to explicitly handle disabled teleports both when a target is resolved and when it is undefined. Two SSR tests were added verifying hydration of a disabled Teleport with an undefined target; no public API changes were made. Changes
Sequence Diagram(s)sequenceDiagram
participant SSR as Server
participant Container as DOM Container
participant App as Vue App
participant Teleport as Teleport Hydrator
Note over SSR,Container: server-rendered HTML contains teleport markers
SSR->>Container: renderToString -> HTML with <!--teleport start-->/<--teleport end-->
App->>Container: mount()
App->>Teleport: hydrateTeleport(vnode)
Teleport->>Teleport: compute disabled state
alt target resolved
Teleport->>Teleport: if disabled -> hydrateDisabledTeleport(startNode, nextSibling)
else no target
Teleport->>Teleport: if disabled and children array -> hydrateDisabledTeleport(curNode, curNode.nextSibling)
end
Teleport->>Container: hydrate children in-place and set anchors/targets
Teleport->>App: hydration complete
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes(No out-of-scope functional changes detected related to the linked issue objectives.) Suggested labels
Poem
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
/ecosystem-ci run |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/runtime-core/__tests__/hydration.spec.ts (2)
2360-2382: Stabilize assertion by removing indentation-sensitive whitespace.The equality currently depends on template indentation. Inline the template text and assert without leading/trailing spaces to avoid brittle failures across compiler whitespace tweaks.
- template: ` - <Teleport :to="undefined" :disabled="true"> - <div v-if="isOpen"> - Menu is open... - </div> - </Teleport>`, + template: `<Teleport :to="undefined" :disabled="true"><div v-if="isOpen">Menu is open...</div></Teleport>`, @@ - expect(container.innerHTML).toBe( - `<!--teleport start--><div> Menu is open... </div><!--teleport end-->`, - ) + expect(container.innerHTML).toBe( + `<!--teleport start--><div>Menu is open...</div><!--teleport end-->`, + ) + expect(`Hydration node mismatch`).not.toHaveBeenWarned() + expect(`Hydration children mismatch`).not.toHaveBeenWarned()
2360-2360: Align test name with suite style.Match nearby Teleport tests’ naming for consistency.
- test('with disabled teleport + undefined target', async () => { + test('Teleport (disabled + undefined target)', async () => {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/runtime-core/__tests__/hydration.spec.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/runtime-core/__tests__/hydration.spec.ts (2)
packages/runtime-core/src/apiCreateApp.ts (1)
App(33-119)packages/runtime-dom/src/index.ts (1)
createSSRApp(148-165)
🪛 ast-grep (0.38.6)
packages/runtime-core/__tests__/hydration.spec.ts
[warning] 2373-2373: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: container.innerHTML = await renderToString(h(App))
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
[warning] 2373-2373: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: container.innerHTML = await renderToString(h(App))
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test / e2e-test
🔇 Additional comments (1)
packages/runtime-core/__tests__/hydration.spec.ts (1)
2373-2376: innerHTML usage in test is fine.Assigning SSR output to innerHTML is standard in this suite and safe here (trusted source). No change needed despite static-analysis noise.
|
📝 Ran ecosystem CI: Open
|
close #11230
Summary by CodeRabbit
New Features
Tests