LearnNewsExamplesServices
Frontmatter
id8868
titleFix: syncVnodeTree should nullify vnode for removed children
stateClosed
labels
bugaicore
assigneestobiu
createdAtJan 23, 2026, 8:58 PM
updatedAtJan 23, 2026, 11:50 PM
githubUrlhttps://github.com/neomjs/neo/issues/8868
authortobiu
commentsCount2
parentIssuenull
subIssues[]
subIssuesCompleted0
subIssuesTotal0
blockedBy[]
blocking[]
closedAtJan 23, 2026, 11:50 PM

Fix: syncVnodeTree should nullify vnode for removed children

Closed v11.24.0 bugaicore
tobiu
tobiu commented on Jan 23, 2026, 8:58 PM

Description: The syncVnodeTree method in src/mixin/VdomLifecycle.mjs updates component state based on the new VNode structure returned by the VDom Worker. Currently, it iterates only over the new VNode's children. If a child component was removed (e.g. removeDom: true), it is skipped by syncVnodeTree, leaving the child with a stale _vnode reference and mounted: true.

Goal: Ensure component._vnode is null and mounted is false when a component is removed from the DOM. This aligns the App Worker state with the actual DOM state.

Proposed Change:

  1. Modify syncVnodeTree to iterate Logical Children (ComponentManager.find({parentId: me.id})).
  2. If a logical child is not found in the new VNode (and is not floating), explicitly set _vnode = null and mounted = false.

Verification: This change will likely break test/playwright/unit/vdom/VdomLifecycle.spec.mjs, specifically the test "vnode should PERSIST when component is hidden". This test should be updated to reflect the new, correct behavior (vnode should be null).

Related Files:

  • src/mixin/VdomLifecycle.mjs
tobiu added the bug label on Jan 23, 2026, 8:58 PM
tobiu added the ai label on Jan 23, 2026, 8:58 PM
tobiu added the core label on Jan 23, 2026, 8:58 PM
tobiu
tobiu Jan 23, 2026, 11:04 PM

Input from Gemini 3 Pro:

✦ I attempted to implement the fix for this issue but encountered regressions that caused VDOM update cycles to time out in tests.

Learnings & Analysis:

  1. Logical Child Iteration: Switching from getChildren(vnode) to ComponentManager.find({parentId}) is correct for identifying all managed children, including those that might have been removed.
  2. Placeholder Complexity: My attempt to handle "placeholders" (nodes with componentId) inside syncVnodeTree was flawed. The logic to distinguish between a full VNode update and a placeholder reference is subtle. createVdomReference generates an object with componentId, but addVnodeComponentReferences transforms the tree. The interaction between these methods and syncVnodeTree needs careful tracing.
  3. Update Cycle Deadlocks: The changes caused promiseUpdate() to hang (timeout) in multiple tests. This strongly suggests that by modifying how vnodeInitialized or mounted was set (or skipped), I inadvertently broke the resolution chain for VDomUpdate, leaving updates in an "in-flight" state.
  4. The "Silent" Update: The _vnode update in syncVnodeTree is critical. If we skip it (as I tried for placeholders), we might miss necessary state synchronization that the framework expects, leading to the deadlocks.

Recommendation for Next Attempt:

  • Focus strictly on the missing children case first.
  • Debug syncVnodeTree to log exactly what the VDom Worker sends back for a "placeholder" vs. a "full" node.
  • Ensure that any logic modifying mounted or _vnode does not interfere with VDomUpdate.executeCallbacks.
tobiu referenced in commit 286640b - "fix: syncVnodeTree should nullify vnode for removed children (#8868)" on Jan 23, 2026, 11:49 PM
tobiu
tobiu Jan 23, 2026, 11:50 PM

Input from Gemini 3 Pro:

✦ I have implemented the fix for this issue.

Changes:

  1. Modified src/mixin/VdomLifecycle.mjs to use a Hybrid Two-Pass Strategy:

    • Pass 1 (Update): Iterates over visibleChildren (found in the new VNode structure). This matches the original behavior and preserves the full VNode structure for placeholders, preventing regressions in TreeBuilder/Helper logic (which relies on symmetry).
    • Pass 2 (Unmount): Iterates over ALL logical children (ComponentManager.find). If a child is NOT visible AND not found as a placeholder (checked via VNodeUtil.find), it is correctly unmounted (_vnode = null, mounted = false).
  2. Added a regression test test/playwright/unit/vdom/HiddenChildren.spec.mjs that reproduces the issue by hiding a child (removeDom: true) inside a container and verifying it unmounts correctly.

Verification:

  • HiddenChildren.spec.mjs passes (Child is unmounted).
  • RealWorldUpdates.spec.mjs passes (No extra deltas/regressions).
  • Full npm run test-unit passes (250 tests).

The fix ensures correct state synchronization without disrupting the delicate balance of VDOM merging and placeholder handling.

tobiu assigned to @tobiu on Jan 23, 2026, 11:50 PM
tobiu closed this issue on Jan 23, 2026, 11:50 PM
tobiu cross-referenced by #8872 on Jan 24, 2026, 12:21 AM
tobiu cross-referenced by #8874 on Jan 24, 2026, 12:30 AM