fix(runtime-core): prevent crash when disabled Teleport with component child is inside Suspense#14737
Closed
ZackaryShen wants to merge 2 commits intovuejs:mainfrom
Closed
Conversation
added 2 commits
April 19, 2026 16:10
Fix issue vuejs#14724 where :deep at the start of :is/:where selector was not being properly resolved to [data-v-xxx]. Before: :is(:deep(.foo)) -> :is(:deep(.foo)) After: :is(:deep(.foo)) -> :is([data-v-xxx] .foo) The fix checks if __deep was set during recursion and injects the scoped attribute at the start of the inner selector.
…t child is inside Suspense Fix issue vuejs#14701 where moving a disabled Teleport with a component child inside Suspense causes 'TypeError: Cannot read properties of null (reading subTree)'. Root cause: When a disabled Teleport is inside a Suspense boundary, the component children haven't been mounted yet (deferred mount via queuePendingMount). The move() function was unconditionally accessing vnode.component.subTree without checking if component exists. Fix: Add null check for vnode.component before trying to move its subTree. If component hasn't been mounted yet, skip the move.
📝 WalkthroughWalkthroughFixes regression in issue Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/compiler-sfc/src/style/pluginScoped.ts (1)
245-268:⚠️ Potential issue | 🟠 MajorFix cross-container node references and trailing selector handling in
:is/:where+:deep()processing.Lines 253 and 262 call
selector.insertBefore()(the outer selector container) using(node as selectorParser.Pseudo).nodes[0](a child of the inner selector container) as the anchor. According to postcss-selector-parser,insertBeforerequires the anchor to be a direct child of the calling container. Additionally, the node selection logic (lines 128–135) causes this entire block to be skipped for cases like:is(:deep(.foo)) .barsincenodebecomes.bar, not:is. To fix this, the:deep()handling for:is/:whereshould either:
- Detect
:deep()inside:is/:wherebefore the node selection loop completes, or- Insert using the inner selector's container (
(node as selectorParser.Pseudo).nodes[0].parent) instead of the outer selectorAlso consider whether
(rule as any).__deeppersisting across multiple selectors in a comma-separated list can cause unexpected state carryover.Verify with:
compileScoped(`:is(:deep(.foo)) .bar { color: red; }`) compileScoped(`:is(:deep(.foo)), :is(.bar) { color: red; }`)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/compiler-sfc/src/style/pluginScoped.ts` around lines 245 - 268, The insertion into the selector is using selector.insertBefore with an anchor that is a child of the inner :is/:where pseudo, which violates postcss-selector-parser’s container/anchor rules and also misses cases where node becomes the outer sibling (e.g. :is(:deep(.foo)) .bar); to fix, detect :deep() inside :is/:where earlier (inside the rewriteSelector traversal of the pseudo) or change the insert target to the inner selector container (use (node as selectorParser.Pseudo).nodes[0].parent as the container when calling insertBefore instead of selector), and ensure (rule as any).__deep is cleared/isolated per selector (reset before moving to the next comma-separated selector) to avoid state carryover; update the logic around selector.insertBefore, the pseudo handling block that uses (node as selectorParser.Pseudo).nodes[0], and the __deep lifecycle to implement these changes and verify with compileScoped tests provided.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/runtime-core/__tests__/components/Suspense.spec.ts`:
- Around line 2575-2619: The test currently uses real DOM nodes
(document.createElement/appendChild/removeChild) but the renderer imported via
render (from `@vue/runtime-test`) expects test nodes from nodeOps; replace any
document.createElement('div') usages for target and root with
nodeOps.createElement('div'), stop using document.body.appendChild/removeChild,
and update the assertion to use serializeInner(root) (or serializeInner(target)
as appropriate) instead of checking root.innerHTML; keep the rest of the logic
(Comp, Child, Parent, toggle, resolve, Suspense, Teleport) unchanged so the test
uses nodeOps.createElement and serializeInner from `@vue/runtime-test` utilities.
---
Outside diff comments:
In `@packages/compiler-sfc/src/style/pluginScoped.ts`:
- Around line 245-268: The insertion into the selector is using
selector.insertBefore with an anchor that is a child of the inner :is/:where
pseudo, which violates postcss-selector-parser’s container/anchor rules and also
misses cases where node becomes the outer sibling (e.g. :is(:deep(.foo)) .bar);
to fix, detect :deep() inside :is/:where earlier (inside the rewriteSelector
traversal of the pseudo) or change the insert target to the inner selector
container (use (node as selectorParser.Pseudo).nodes[0].parent as the container
when calling insertBefore instead of selector), and ensure (rule as any).__deep
is cleared/isolated per selector (reset before moving to the next
comma-separated selector) to avoid state carryover; update the logic around
selector.insertBefore, the pseudo handling block that uses (node as
selectorParser.Pseudo).nodes[0], and the __deep lifecycle to implement these
changes and verify with compileScoped tests provided.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2a4b1332-1cfc-4eed-aa66-d9096900772f
📒 Files selected for processing (4)
packages/compiler-sfc/__tests__/compileStyle.spec.tspackages/compiler-sfc/src/style/pluginScoped.tspackages/runtime-core/__tests__/components/Suspense.spec.tspackages/runtime-core/src/renderer.ts
Comment on lines
+2575
to
+2619
| const target = document.createElement('div') | ||
| document.body.appendChild(target) | ||
|
|
||
| const Comp = { | ||
| render() { | ||
| return [ | ||
| h('div', 'content'), | ||
| h(Teleport, { to: target, disabled: true }, h(Child, { msg: 'hello' })), | ||
| ] | ||
| }, | ||
| } | ||
|
|
||
| let resolve!: () => void | ||
| const Child = { | ||
| props: ['msg'], | ||
| async setup(props: any) { | ||
| await new Promise(r => (resolve = r)) | ||
| return () => h('span', props.msg) | ||
| }, | ||
| } | ||
|
|
||
| const toggle = ref(true) | ||
| const Parent = { | ||
| render() { | ||
| return toggle.value | ||
| ? h(Suspense, null, { | ||
| default: () => h(Comp), | ||
| fallback: h('div', 'fallback'), | ||
| }) | ||
| : null | ||
| }, | ||
| } | ||
|
|
||
| const root = document.createElement('div') | ||
| render(h(Parent), root) | ||
|
|
||
| // Trigger move by toggling | ||
| toggle.value = false | ||
| await nextTick() | ||
|
|
||
| // Should not throw "Cannot read properties of null (reading 'subTree')" | ||
| expect(root.innerHTML).toBe('') | ||
|
|
||
| // Clean up | ||
| document.body.removeChild(target) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Inspect `@vue/runtime-test` nodeOps to confirm it expects runtime-test nodes, not native DOM nodes.
fd -i 'runtime-test' -t f | sed -n '1,80p'
rg -n -C4 'createRenderer|nodeOps|insert\s*\(|children\.(push|splice)|parentNode\s*=' --iglob '*.ts' --iglob '*runtime-test*'Repository: vuejs/core
Length of output: 50369
Use nodeOps test nodes with @vue/runtime-test render.
This test imports render from @vue/runtime-test, which uses a custom node system backed by nodeOps. Passing real DOM nodes created via document.createElement() will cause failures—the renderer expects test nodes with children and parentNode properties. Match surrounding Suspense/Teleport tests by using nodeOps.createElement() and serializeInner() instead.
Proposed fix
- const target = document.createElement('div')
- document.body.appendChild(target)
+ const target = nodeOps.createElement('div')
@@
- const root = document.createElement('div')
+ const root = nodeOps.createElement('div')
render(h(Parent), root)
@@
- expect(root.innerHTML).toBe('')
-
- // Clean up
- document.body.removeChild(target)
+ expect(serializeInner(root)).toBe('')🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/runtime-core/__tests__/components/Suspense.spec.ts` around lines
2575 - 2619, The test currently uses real DOM nodes
(document.createElement/appendChild/removeChild) but the renderer imported via
render (from `@vue/runtime-test`) expects test nodes from nodeOps; replace any
document.createElement('div') usages for target and root with
nodeOps.createElement('div'), stop using document.body.appendChild/removeChild,
and update the assertion to use serializeInner(root) (or serializeInner(target)
as appropriate) instead of checking root.innerHTML; keep the rest of the logic
(Comp, Child, Parent, toggle, resolve, Suspense, Teleport) unchanged so the test
uses nodeOps.createElement and serializeInner from `@vue/runtime-test` utilities.
Member
|
duplicate of #14702 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix Issue #14701
Problem
When a disabled Teleport containing a component child is placed inside a Suspense boundary, moving the Teleport causes a crash:
Root Cause
When Suspense resolves, it calls
move()to reposition content. The renderer was unconditionally accessingvnode.component.subTree, but for disabled Teleports, component children haven't been mounted yet.Fix
Added null check for
vnode.componentbefore accessing.subTree.Testing
Added regression test.
Closes #14701
Summary by CodeRabbit
Bug Fixes
Style
Tests