From d2b11c8adfcb4f64955e3f6e19bece13efeea9c7 Mon Sep 17 00:00:00 2001 From: Adnaan Date: Thu, 29 Jan 2026 20:52:32 +0100 Subject: [PATCH 1/5] =?UTF-8?q?fix:=20handle=20nested=20range=E2=86=92non-?= =?UTF-8?q?range=20transitions=20correctly?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous implementation only checked for range structures directly on a node using isRangeNode(). This missed cases where the range is nested inside another structure, such as: {{if .ShowList}} {{else}}

List is hidden

{{end}} When ShowList changes from true to false: - Position 0 changes from {"0": {range...}, "s": [""]} - To {"s": ["

List is hidden

"]} The nested range at position 0.0 was being preserved during merge, causing orphaned data in the tree state. Fix: - Add hasRangeAnywhere() to recursively check for nested ranges - Add shouldFullReplace() to detect all structure transitions - Update deepMergeTreeNodes to use shouldFullReplace instead of just isRangeNode Tests: - Add 3 new test cases for nested range→non-range transitions - Test deeply nested ranges (3 levels deep) - Test transition back from else to nested range Co-Authored-By: Claude Opus 4.5 --- state/tree-renderer.ts | 90 ++++++++++++++++++++-- tests/tree-renderer.test.ts | 145 ++++++++++++++++++++++++++++++++++++ 2 files changed, 228 insertions(+), 7 deletions(-) diff --git a/state/tree-renderer.ts b/state/tree-renderer.ts index 9ccf706..3d82c56 100644 --- a/state/tree-renderer.ts +++ b/state/tree-renderer.ts @@ -40,6 +40,79 @@ function isRangeNode(node: any): boolean { ); } +/** + * Checks if a node or any of its nested children contains a range structure. + * + * This is needed for detecting structure transitions like: + * - {{if .ShowList}}{{else}}

Hidden

{{end}} + * + * When ShowList changes from true to false, the outer node doesn't have `d` directly, + * but its child at position "0" does. We need to detect this nested range to know + * that a full replacement is required instead of a merge. + * + * @param node - The tree node to check + * @returns true if this node or any nested child has a range structure + */ +function hasRangeAnywhere(node: any): boolean { + if (node == null || typeof node !== "object" || Array.isArray(node)) { + return false; + } + + // Check if this node itself is a range + if (isRangeNode(node)) { + return true; + } + + // Check numeric keys (dynamic positions) for nested ranges + for (const key of Object.keys(node)) { + if (/^\d+$/.test(key)) { + const child = node[key]; + if (child != null && typeof child === "object" && !Array.isArray(child)) { + if (hasRangeAnywhere(child)) { + return true; + } + } + } + } + + return false; +} + +/** + * Determines if a structure transition requires full replacement instead of merge. + * + * This handles cases where: + * 1. Old structure has a range (directly or nested) but new structure doesn't + * 2. New structure has statics (indicating a complete structure definition) + * 3. Old structure has dynamics that new structure doesn't have + * + * @param existing - The existing tree node + * @param update - The update tree node + * @returns true if the update should fully replace existing instead of merging + */ +function shouldFullReplace(existing: any, update: any): boolean { + // If update doesn't have statics, it's a partial update, not a replacement + if (!update.s || !Array.isArray(update.s)) { + return false; + } + + // Check for range→non-range transition (including nested ranges) + if (hasRangeAnywhere(existing) && !hasRangeAnywhere(update)) { + return true; + } + + // Check if existing has dynamics that update doesn't have + for (const key of Object.keys(existing)) { + if (/^\d+$/.test(key) && !(key in update)) { + // Existing has a dynamic position that update doesn't + // If update has statics, this is a structure change + return true; + } + } + + return false; +} + /** * Handles tree state management and HTML reconstruction logic for LiveTemplate. */ @@ -132,14 +205,17 @@ export class TreeRenderer { return update; } - // Detect range→non-range transition: when existing has a range structure - // but update does NOT, we must do a full replacement instead of merge. - // Otherwise, the old range items would be preserved and rendered with - // the new (else clause) statics, causing wrong content. - // See isRangeNode() for definition of "range" vs "non-range" structures. - if (isRangeNode(existing) && !isRangeNode(update)) { + // Detect structure transitions that require full replacement instead of merge. + // This handles: + // 1. Direct range→non-range: existing has d/s arrays, update doesn't + // 2. Nested range→non-range: existing has range in a child, update doesn't + // 3. Structure changes: existing has dynamics that update doesn't have + // + // Without this check, old range data would be preserved and mixed with + // new statics, causing wrong content or memory leaks. + if (shouldFullReplace(existing, update)) { this.logger.debug( - `[deepMerge] Range→non-range transition at path ${currentPath}, replacing instead of merging` + `[deepMerge] Structure transition at path ${currentPath}, replacing instead of merging` ); return update; } diff --git a/tests/tree-renderer.test.ts b/tests/tree-renderer.test.ts index 9fc8235..8df62fd 100644 --- a/tests/tree-renderer.test.ts +++ b/tests/tree-renderer.test.ts @@ -184,4 +184,149 @@ describe("TreeRenderer", () => { expect(elseResult.html).toContain("

No items available

"); }); }); + + describe("render - NESTED range to non-range transition", () => { + /** + * This tests the case where a range is nested inside another structure: + * + * Template: + * {{if .ShowList}} + * + * {{else}} + *

List is hidden

+ * {{end}} + * + * When ShowList goes from true to false: + * - Position 0 changes from {0: {range...}, s: [""]} + * - To {s: ["

List is hidden

"]} + * + * The nested range at position 0.0 must be removed, not preserved. + */ + it("should replace nested range structure with else clause", () => { + // Initial state: ShowList=true with items + // Structure: position 0 contains a UL wrapper with nested range at 0.0 + const initialUpdate = { + s: ["
", "
"], + 0: { + // The if-branch: UL wrapper containing the range + 0: { + // The range (nested at position 0.0) + d: [ + { 0: "item-1", 1: "Apple", _k: "item-1" }, + { 0: "item-2", 1: "Banana", _k: "item-2" }, + ], + s: ['
  • ', "
    • "], + }, + s: [""], // UL wrapper statics + }, + }; + const initialResult = renderer.applyUpdate(initialUpdate); + + expect(initialResult.html).toContain(""); + + // Update: ShowList=false, show else clause + // The nested range should be completely replaced + const elseUpdate = { + 0: { + // The else-branch: just a paragraph + s: ["

    List is hidden

    "], + }, + }; + const elseResult = renderer.applyUpdate(elseUpdate); + + // Verify the tree state doesn't contain orphaned range data + const treeState = renderer.getTreeState(); + expect(treeState[0]).not.toHaveProperty("d"); + expect(treeState[0][0]).toBeUndefined(); // Nested range should be gone + + // Should NOT contain old items + expect(elseResult.html).not.toContain("Apple"); + expect(elseResult.html).not.toContain("Banana"); + expect(elseResult.html).not.toContain("