rendering: free slot placeholders and cached internal views

Solid layout placeholders inherited a no-op destroyRecursively(), so
removed or unmounted placeholders never freed their Yoga nodes. Destroy
them on that path, free placeholders a host rejects, and make the slot
forget destroyed placeholders so a reinserted slot never reuses a freed
node.

LineNumberRenderable now frees its gutter in clearTarget() and destroy(),
which previously threw. DiffRenderable frees views it cached while
detached and the default syntax style it creates.

packages/core/src/renderables/Diff.test.ts

@@ -2470,7 +2470,26 @@ test("DiffRenderable - gutter remains in correct position after updates", async
   }
 })
 
-test("DiffRenderable - properly cleans up listeners on destroy", async () => {
+test("DiffRenderable - releases default syntax styles after replacing them with a borrowed style", () => {
+  const diff = new DiffRenderable(currentRenderer, { diff: simpleDiff, view: "split" })
+  currentRenderer.root.add(diff)
+  const defaults = [diff["leftCodeRenderable"]!.syntaxStyle, diff["rightCodeRenderable"]!.syntaxStyle]
+  const borrowed = SyntaxStyle.create()
+
+  try {
+    diff.syntaxStyle = borrowed
+    diff.destroyRecursively()
+
+    for (const style of defaults) expect(() => style.getStyleCount()).toThrow("destroyed")
+    expect(borrowed.getStyleCount()).toBe(0)
+  } finally {
+    diff.destroyRecursively()
+    for (const style of defaults) style.destroy()
+    borrowed.destroy()
+  }
+})
+
+test.each(["destroy", "destroyRecursively"] as const)("DiffRenderable - %s frees cached views", async (cleanup) => {
   const syntaxStyle = SyntaxStyle.fromStyles({
     default: { fg: RGBA.fromValues(1, 1, 1, 1) },
   })
@@ -2503,19 +2522,24 @@ test("DiffRenderable - properly cleans up listeners on destroy", async () => {
   expect(leftCountBeforeDestroy).toBeGreaterThan(0)
   expect(rightCountBeforeDestroy).toBeGreaterThan(0)
 
-  // Destroy the diff
-  diffRenderable.destroyRecursively()
+  const sides = diffRenderable.getChildren()
+  diffRenderable.diff = "--- a/test.js\n+++ b/test.js\n@@ -a,b +c,d @@\n invalid"
+  await renderOnce()
+  const errorNodes = diffRenderable.getChildren()
+  expect(errorNodes).toHaveLength(2)
+  diffRenderable.diff = simpleDiff
+  diffRenderable.view = "unified"
+  await renderOnce()
 
-  // The LineNumberRenderables should have been destroyed
-  // Check that they're either null or destroyed
-  const leftSide = (diffRenderable as any).leftSide
-  const rightSide = (diffRenderable as any).rightSide
-
-  if (leftSide) {
-    expect(leftSide.isDestroyed).toBe(true)
-  }
-  if (rightSide) {
-    expect(rightSide.isDestroyed).toBe(true)
+  try {
+    diffRenderable[cleanup]()
+    for (const node of [...sides, ...errorNodes, leftCodeRenderable, rightCodeRenderable]) {
+      expect(node.getLayoutNode().isFreed()).toBe(true)
+      expect(node.listenerCount("line-info-change")).toBe(0)
+    }
+  } finally {
+    for (const node of [...sides, ...errorNodes]) node.destroyRecursively()
+    syntaxStyle.destroy()
   }
 })
 

packages/core/src/renderables/Diff.ts

@@ -66,6 +66,7 @@ export class DiffRenderable extends Renderable {
   private _fg?: RGBA
   private _filetype?: string
   private _syntaxStyle?: SyntaxStyle
+  private _defaultSyntaxStyle?: SyntaxStyle
   private _wrapMode?: "word" | "char" | "none"
   private _conceal: boolean
   private _selectionBg?: RGBA
@@ -299,12 +300,27 @@ export class DiffRenderable extends Renderable {
     this._lineInfoChangeHandler = null
   }
 
-  public override destroyRecursively(): void {
+  protected override destroySelf(): void {
     this.detachLineInfoListeners()
     this.pendingRebuild = false
     this.leftSideAdded = false
     this.rightSideAdded = false
-    super.destroyRecursively()
+    for (const node of [this.leftSide, this.rightSide, this.errorTextRenderable, this.errorCodeRenderable]) {
+      node?.destroyRecursively()
+    }
+    this.leftSide = null
+    this.rightSide = null
+    this.leftCodeRenderable = null
+    this.rightCodeRenderable = null
+    this.errorTextRenderable = null
+    this.errorCodeRenderable = null
+    this._defaultSyntaxStyle?.destroy()
+    this._defaultSyntaxStyle = undefined
+    super.destroySelf()
+  }
+
+  private getCodeSyntaxStyle(): SyntaxStyle {
+    return this._syntaxStyle ?? (this._defaultSyntaxStyle ??= SyntaxStyle.create())
   }
 
   private buildErrorView(): void {
@@ -342,7 +358,7 @@ export class DiffRenderable extends Renderable {
         id: this.id ? `${this.id}-error-code` : undefined,
         content: this._diff,
         filetype: "diff",
-        syntaxStyle: this._syntaxStyle ?? SyntaxStyle.create(),
+        syntaxStyle: this.getCodeSyntaxStyle(),
         wrapMode: this._wrapMode,
         conceal: this._conceal,
         width: "100%",
@@ -379,7 +395,7 @@ export class DiffRenderable extends Renderable {
         filetype: this._filetype,
         wrapMode,
         conceal: this._conceal,
-        syntaxStyle: this._syntaxStyle ?? SyntaxStyle.create(),
+        syntaxStyle: this.getCodeSyntaxStyle(),
         width: "100%",
         height: "100%",
         ...(this._fg !== undefined && { fg: this._fg }),

packages/core/src/renderables/LineNumberRenderable.ts

@@ -375,7 +375,6 @@ export class LineNumberRenderable extends Renderable {
   private _lineNumberOffset: number
   private _hideLineNumbers: Set<number>
   private _lineNumbers: Map<number, number>
-  private _isDestroying: boolean = false
   private handleLineInfoChange = (): void => {
     // When line info changes in the target, remeasure the gutter
     this.gutter?.remeasure()
@@ -444,17 +443,7 @@ export class LineNumberRenderable extends Renderable {
   private setTarget(target: Renderable & LineInfoProvider): void {
     if (this.target === target) return
 
-    if (this.target) {
-      // Remove event listener from old target
-      this.target.off("line-info-change", this.handleLineInfoChange)
-      super.remove(this.target)
-    }
-
-    if (this.gutter) {
-      super.remove(this.gutter)
-      this.gutter = null
-    }
-
+    this.clearTarget()
     this.target = target
 
     // Listen for line info changes from target
@@ -497,11 +486,6 @@ export class LineNumberRenderable extends Renderable {
 
   // Override remove to prevent removing gutter/target directly
   public override remove(child: BaseRenderable): void {
-    if (this._isDestroying) {
-      super.remove(child)
-      return
-    }
-
     if (this.gutter && child === this.gutter) {
       throw new Error("LineNumberRenderable: Cannot remove gutter directly.")
     }
@@ -511,18 +495,16 @@ export class LineNumberRenderable extends Renderable {
     super.remove(child)
   }
 
-  // Override destroyRecursively to properly clean up internal components
   public override destroyRecursively(): void {
-    this._isDestroying = true
-
-    if (this.target) {
-      this.target.off("line-info-change", this.handleLineInfoChange)
-    }
-
+    const target = this.target
+    this.clearTarget()
+    target?.destroyRecursively()
     super.destroyRecursively()
+  }
 
-    this.gutter = null
-    this.target = null
+  public override destroy(): void {
+    this.clearTarget()
+    super.destroy()
   }
 
   public clearTarget(): void {
@@ -533,6 +515,7 @@ export class LineNumberRenderable extends Renderable {
     }
     if (this.gutter) {
       super.remove(this.gutter)
+      this.gutter.destroy()
       this.gutter = null
     }
   }

packages/core/src/renderables/__tests__/LineNumberRenderable.test.ts

@@ -110,6 +110,27 @@ afterEach(() => {
 })
 
 describe("LineNumberRenderable", () => {
+  test.each(["clearTarget", "destroy"] as const)("%s frees the gutter and preserves the target", async (cleanup) => {
+    const { renderer } = await createTestRenderer({ width: 20, height: 5 })
+    const target = new MockTextBuffer(renderer, { text: "content" })
+    const numbers = new LineNumberRenderable(renderer, { target })
+    renderer.root.add(numbers)
+    const gutter = numbers.getChildren()[0]!
+
+    try {
+      numbers[cleanup]()
+
+      expect(numbers.getChildren()).toHaveLength(0)
+      expect(target.parent).toBeNull()
+      expect(target.isDestroyed).toBe(false)
+      expect(target.listenerCount("line-info-change")).toBe(0)
+      expect(gutter.getLayoutNode().isFreed()).toBe(true)
+    } finally {
+      gutter.destroy()
+      target.destroy()
+    }
+  })
+
   test("renders line numbers correctly", async () => {
     const { renderer, renderOnce, captureCharFrame } = await createTestRenderer({
       width: 20,

packages/solid/src/elements/slot.ts

@@ -83,8 +83,8 @@ export class TextSlotRenderable extends TextNodeRenderable {
     const slotParent = this.slotParent
     this.slotParent = undefined
 
-    slotParent?.destroy()
-    super.destroy()
+    this.parent?.remove(this)
+    slotParent?.didDestroySlotChild(this)
   }
 }
 
@@ -145,6 +145,11 @@ export class LayoutSlotRenderable extends SlotBaseRenderable {
     this.freeYogaNode()
   }
 
+  // Parents and the reconciler release children through destroyRecursively().
+  public override destroyRecursively(): void {
+    this.destroy()
+  }
+
   public override destroy(): void {
     if (this.destroyed) {
       return
@@ -154,8 +159,9 @@ export class LayoutSlotRenderable extends SlotBaseRenderable {
     const slotParent = this.slotParent
     this.slotParent = undefined
 
+    this.parent?.remove(this)
     this.freeYogaNode()
-    slotParent?.destroy()
+    slotParent?.didDestroySlotChild(this)
   }
 }
 
@@ -372,11 +378,22 @@ export class SlotRenderable extends SlotBaseRenderable {
     }
   }
 
+  didDestroySlotChild(child: LayoutSlotRenderable | TextSlotRenderable): void {
+    const nodesByParent: Map<BaseRenderable, BaseRenderable> =
+      child instanceof LayoutSlotRenderable ? this.layoutNodesByParent : this.textNodesByParent
+    for (const [parent, node] of nodesByParent) {
+      if (node !== child) continue
+      nodesByParent.delete(parent)
+      if (this.parent === parent) this.parent = this.getAttachedSlotParent()
+    }
+  }
+
   public override destroy(): void {
     if (this.destroyed) {
       return
     }
     this.destroyed = true
+    this.parent = null
 
     const layoutNodes = new Set(this.layoutNodesByParent.values())
     this.layoutNodesByParent.clear()

packages/solid/src/reconciler.ts

@@ -68,13 +68,14 @@ function _insertNode(parent: DomNode, node: DomNode, anchor?: DomNode): void {
     node instanceof TextNode,
   )
 
-  if (node instanceof SlotRenderable) {
-    node.parent = parent
-    node = node.getSlotChild(parent)
+  const slot = node instanceof SlotRenderable ? node : undefined
+  if (slot) {
+    slot.parent = parent
+    node = slot.getSlotChild(parent)
   }
 
   if (anchor && anchor instanceof SlotRenderable) {
-    anchor = anchor.getSlotChild(parent)
+    anchor = anchor.getSlotChildForRemoval(parent)
   }
 
   if (isTextNodeRenderable(node)) {
@@ -95,19 +96,19 @@ function _insertNode(parent: DomNode, node: DomNode, anchor?: DomNode): void {
     throw new Error("Tried to mount a non base renderable")
   }
 
-  if (!anchor) {
-    parent.add(node)
-    return
-  }
-
-  const children = getNodeChildren(parent)
-
-  const anchorIndex = children.indexOf(anchor)
-  if (anchorIndex === -1) {
-    log("[INSERT]", "Could not find anchor", logId(parent), logId(anchor), "[children]", ...children.map((c) => c.id))
+  let anchorIndex: number | undefined
+  if (anchor) {
+    const children = getNodeChildren(parent)
+    anchorIndex = children.indexOf(anchor)
+    if (anchorIndex === -1) {
+      log("[INSERT]", "Could not find anchor", logId(parent), logId(anchor), "[children]", ...children.map((c) => c.id))
+    }
   }
 
   parent.add(node, anchorIndex)
+
+  // Hosts such as LineNumberRenderable ignore children they do not render.
+  if (slot && !node.parent) node.destroy()
 }
 
 function _removeNode(parent: DomNode, node: DomNode): void {

packages/solid/tests/control-flow.test.tsx

@@ -1,5 +1,5 @@
 import { describe, expect, it, beforeEach, afterEach, test } from "bun:test"
-import { testRender } from "../index.js"
+import { LayoutSlotRenderable, testRender } from "../index.js"
 import { createSignal, createEffect, createMemo, For, Show, Switch, Match, Index, ErrorBoundary } from "solid-js"
 
 let testSetup: Awaited<ReturnType<typeof testRender>>
@@ -215,7 +215,7 @@ describe("SolidJS Renderer - Control Flow Components", () => {
       expect(frame).not.toContain("Count is high")
     })
 
-    it("should handle <Show> without fallback", async () => {
+    it.each(["replacement", "renderer destruction"])("should handle <Show> without fallback (%s)", async (cleanup) => {
       const [visible, setVisible] = createSignal(true)
 
       testSetup = await testRender(
@@ -247,6 +247,25 @@ describe("SolidJS Renderer - Control Flow Components", () => {
       frame = testSetup.captureCharFrame()
       expect(frame).not.toContain("Visible content")
       expect(frame).toContain("Always visible")
+
+      const placeholder = children.find((child) => child instanceof LayoutSlotRenderable)!
+      try {
+        expect(placeholder).toBeInstanceOf(LayoutSlotRenderable)
+        expect(placeholder.getLayoutNode().isFreed()).toBe(false)
+
+        if (cleanup === "replacement") {
+          setVisible(true)
+        } else {
+          testSetup.renderer.destroy()
+        }
+        await new Promise<void>((resolve) => process.nextTick(resolve))
+
+        expect(placeholder.parent).toBeNull()
+        expect(placeholder.getLayoutNode().isFreed()).toBe(true)
+      } finally {
+        testSetup.renderer.destroy()
+        placeholder?.destroy()
+      }
     })
 
     it("should conditionally render content with <Show> in the correct order", async () => {

packages/solid/tests/dynamic-portal.test.tsx

@@ -1,5 +1,5 @@
 import { describe, expect, it, beforeEach, afterEach } from "bun:test"
-import { testRender, Dynamic, Portal } from "../index.js"
+import { testRender, Dynamic, LayoutSlotRenderable, Portal } from "../index.js"
 import { createSignal, Show } from "solid-js"
 import { createSpy } from "@opentui/core/testing"
 import type { BoxRenderable } from "@opentui/core"
@@ -193,6 +193,40 @@ describe("SolidJS Renderer - Dynamic and Portal Components", () => {
       expect(frame).not.toContain("Portal content")
     })
 
+    it("should give a reinserted portal marker a live placeholder", async () => {
+      const [showPortal, setShowPortal] = createSignal(true)
+      const nextTick = () => new Promise<void>((resolve) => process.nextTick(resolve))
+      let host!: BoxRenderable
+
+      testSetup = await testRender(
+        () => {
+          const portal = (
+            <Portal>
+              <text>Portal content</text>
+            </Portal>
+          )
+          return (
+            <box ref={host}>
+              {showPortal() ? portal : null}
+              <text>After portal</text>
+            </box>
+          )
+        },
+        { width: 20, height: 5 },
+      )
+
+      for (let i = 0; i < 3; i++) {
+        setShowPortal(false)
+        await nextTick()
+        setShowPortal(true)
+        await nextTick()
+
+        const placeholder = host.getChildren()[0]
+        expect(placeholder).toBeInstanceOf(LayoutSlotRenderable)
+        expect(placeholder!.getLayoutNode().isFreed()).toBe(false)
+      }
+    })
+
     it("should handle multiple portals", async () => {
       testSetup = await testRender(
         () => (

packages/solid/tests/line-number.test.tsx

@@ -1,6 +1,6 @@
-import { describe, test, expect, beforeEach, afterEach } from "bun:test"
+import { describe, test, expect, beforeEach, afterEach, spyOn } from "bun:test"
 import { testRender } from "../index.js"
-import { SyntaxStyle } from "@opentui/core"
+import { SyntaxStyle, Yoga } from "@opentui/core"
 import { MockTreeSitterClient } from "@opentui/core/testing"
 import { createSignal, Show } from "solid-js"
 
@@ -74,6 +74,31 @@ console.log(test())`
     expect(frame).toContain(" 4 ") // Line number 4
   })
 
+  test("frees ignored conditional placeholders", async () => {
+    const createNode = spyOn(Yoga.default.Node, "create")
+    try {
+      testSetup = await testRender(() => (
+        <line_number>
+          <Show when={false}>
+            <text>inactive</text>
+          </Show>
+          <text>target</text>
+        </line_number>
+      ))
+      await testSetup.renderOnce()
+      expect(testSetup.captureCharFrame()).toContain("target")
+      expect(createNode).toHaveBeenCalled()
+
+      testSetup.renderer.destroy()
+      await new Promise<void>((resolve) => process.nextTick(resolve))
+      for (const { value } of createNode.mock.results) expect(value.isFreed()).toBe(true)
+    } finally {
+      testSetup?.renderer.destroy()
+      for (const { value } of createNode.mock.results) value.free()
+      createNode.mockRestore()
+    }
+  })
+
   test("handles conditional removal of line number element", async () => {
     const syntaxStyle = SyntaxStyle.fromStyles({
       keyword: { fg: "#C792EA" },

packages/solid/tests/slot-reconciler-move.test.ts

@@ -1,8 +1,9 @@
 import { describe, expect, it } from "bun:test"
-import { BoxRenderable, Yoga } from "@opentui/core"
+import { BoxRenderable, LineNumberRenderable, TextRenderable, Yoga } from "@opentui/core"
 import { createTestRenderer } from "@opentui/core/testing"
 import { batch, createRoot, createSignal } from "solid-js"
-import { createSlotNode, insert } from "../index.js"
+import { createSlotNode, insert, insertNode } from "../index.js"
+import { LayoutSlotRenderable } from "../src/elements/slot.js"
 
 type MoveOrder = "remove-then-insert" | "insert-then-remove"
 
@@ -86,6 +87,38 @@ async function runMoveScenario(order: MoveOrder) {
 }
 
 describe("slot placeholder moves", () => {
+  it.each([false, true])("keeps rejected slots reusable (existing host: %p)", async (attached) => {
+    const setup = await createTestRenderer({ width: 20, height: 5 })
+    const rejecting = new LineNumberRenderable(setup.renderer, {})
+    setup.renderer.root.add(rejecting)
+    const slot = createSlotNode()
+
+    try {
+      if (attached) insertNode(setup.renderer.root, slot)
+      const rejected = slot.getSlotChild(rejecting)
+      if (!(rejected instanceof LayoutSlotRenderable)) throw new Error("Expected layout placeholder")
+      const yoga = rejected.getLayoutNode()
+      insertNode(rejecting, slot)
+
+      expect(yoga.isFreed()).toBe(true)
+      expect(slot.getSlotChildForRemoval(rejecting)).toBeUndefined()
+
+      insertNode(rejecting, new TextRenderable(setup.renderer, { content: "target" }), slot)
+      expect(slot.getSlotChildForRemoval(rejecting)).toBeUndefined()
+
+      insertNode(setup.renderer.root, slot)
+      const accepted = slot.getSlotChild(setup.renderer.root)
+      if (!(accepted instanceof LayoutSlotRenderable)) throw new Error("Expected layout placeholder")
+      expect(accepted.parent).toBe(setup.renderer.root)
+      expect(accepted.getLayoutNode().isFreed()).toBe(false)
+      setup.renderer.destroy()
+      expect(accepted.getLayoutNode().isFreed()).toBe(true)
+    } finally {
+      setup.renderer.destroy()
+      slot.destroy()
+    }
+  })
+
   it("recreates incompatible layout placeholders for remove-then-insert moves", async () => {
     const { controls, movedChild, originalChild, parentA, parentB, setup, slot } =
       await runMoveScenario("remove-then-insert")
@@ -122,7 +155,7 @@ describe("slot placeholder moves", () => {
     }
   })
 
-  it("promotes slot.parent back to another attached host when the newest placeholder is removed", async () => {
+  it.each(["remove", "destroy subtree"])("preserves the other attached slot host on %s", async (cleanup) => {
     const setup = await createTestRenderer({ width: 40, height: 10 })
     const parentA = new BoxRenderable(setup.renderer, {
       id: "slot-parent-host-a",
@@ -144,21 +177,33 @@ describe("slot placeholder moves", () => {
       slot.parent = parentA
       const childA = slot.getSlotChild(parentA)
       parentA.add(childA)
+      const yogaA = parentA.getLayoutNode().getChild(0)!
 
       slot.parent = parentB
       const childB = slot.getSlotChild(parentB)
       parentB.add(childB)
+      const yogaB = parentB.getLayoutNode().getChild(0)!
 
       expect(slot.parent).toBe(parentB)
 
-      parentB.remove(childB)
-      slot.didRemoveSlotChild(parentB, childB)
+      if (cleanup === "remove") {
+        parentB.remove(childB)
+        slot.didRemoveSlotChild(parentB, childB)
+      } else {
+        parentB.destroyRecursively()
+      }
 
       expect(slot.parent).toBe(parentA)
       expect(parentA.getChildren()[0]).toBe(childA)
       expect(parentB.getChildren()).toHaveLength(0)
+      expect(yogaA.isFreed()).toBe(false)
+      expect(yogaB.isFreed()).toBe(true)
+
+      setup.renderer.destroy()
+      expect(yogaA.isFreed()).toBe(true)
     } finally {
       setup.renderer.destroy()
+      slot.destroy()
     }
   })
 })