[PR #680] [MERGED] fix(solid): handle ScrollBox parent mismatch in getParentNode #708

New issue

Closed

opened 2026-03-02 23:47:46 +03:00 by kerem · 0 comments

kerem commented

2026-03-02 23:47:46 +03:00

Owner

📋 Pull Request Information

Original PR: https://github.com/anomalyco/opentui/pull/680
Author: @IlyaGulya
Created: 2/13/2026
Status: ✅ Merged
Merged: 2/15/2026
Merged by: @kommander

Base: main ← Head: fix/scrollbox-for-cleanup

📝 Commits (1)

9716630 fix(solid): handle ScrollBox parent mismatch in getParentNode to fix child removal

📊 Changes

2 files changed (+631 additions, -0 deletions)

View changed files

📝 packages/solid/src/reconciler.ts (+9 -0)
➕ packages/solid/tests/scrollbox-cleanchildren.test.tsx (+622 -0)

📄 Description

Summary

ScrollBox delegates add/remove to its internal content wrapper (scrollbox → wrapper → viewport → content), so children report content as their parent. The reconciler passes scrollbox itself — causing the identity check in cleanChildren (getParentNode(el) === parent) to always fail, leaving stale nodes behind.
Fix: make _getParentNode in reconciler.ts detect when the child's parent is the content of a ScrollBoxRenderable and return the ScrollBox instead. This keeps universal.js untouched.

Root cause

ScrollBox.add(obj) delegates to this.content.add(obj), which sets obj.parent = content
The reconciler's parent argument is scrollbox (the JSX parent)
cleanChildren() checked getParentNode(el) === parent → content !== scrollbox → false
removeNode() was skipped → stale children remained in the tree

Only triggered with multiple dynamic siblings (two+ <For>, <For> + <Show>, etc.) because that's when cleanChildren uses the marker-based path where the guard exists.

AI Disclosure

This PR was created with assistance of AI agent (Claude Code + Claude Opus 4.6)

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/anomalyco/opentui/pull/680 **Author:** [@IlyaGulya](https://github.com/IlyaGulya) **Created:** 2/13/2026 **Status:** ✅ Merged **Merged:** 2/15/2026 **Merged by:** [@kommander](https://github.com/kommander) **Base:** `main` ← **Head:** `fix/scrollbox-for-cleanup` --- ### 📝 Commits (1) - [`9716630`](https://github.com/anomalyco/opentui/commit/9716630629dd4408d5f80917a0d8ea78c574bbeb) fix(solid): handle ScrollBox parent mismatch in getParentNode to fix child removal ### 📊 Changes **2 files changed** (+631 additions, -0 deletions) <details> <summary>View changed files</summary> 📝 `packages/solid/src/reconciler.ts` (+9 -0) ➕ `packages/solid/tests/scrollbox-cleanchildren.test.tsx` (+622 -0) </details> ### 📄 Description ## Summary - ScrollBox delegates `add`/`remove` to its internal `content` wrapper (`scrollbox → wrapper → viewport → content`), so children report `content` as their parent. The reconciler passes `scrollbox` itself — causing the identity check in `cleanChildren` (`getParentNode(el) === parent`) to always fail, leaving stale nodes behind. - Fix: make `_getParentNode` in `reconciler.ts` detect when the child's parent is the `content` of a `ScrollBoxRenderable` and return the ScrollBox instead. This keeps `universal.js` untouched. ## Root cause 1. `ScrollBox.add(obj)` delegates to `this.content.add(obj)`, which sets `obj.parent = content` 2. The reconciler's `parent` argument is `scrollbox` (the JSX parent) 3. `cleanChildren()` checked `getParentNode(el) === parent` → `content !== scrollbox` → `false` 4. `removeNode()` was skipped → stale children remained in the tree Only triggered with multiple dynamic siblings (two+ `<For>`, `<For>` + `<Show>`, etc.) because that's when `cleanChildren` uses the marker-based path where the guard exists. ## AI Disclosure This PR was created with assistance of AI agent (Claude Code + Claude Opus 4.6) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>