From b72d8056329d2aef5ee2ee9e25654a3bd7741352 Mon Sep 17 00:00:00 2001 From: Mobeen Abdullah Date: Sun, 13 Sep 2026 19:09:15 +0300 Subject: [PATCH 1/4] fix(blocks-engine): a reference follows the node the page renders A saved forest holding a visible target and a condition-gated namesake with the same current id counted both as renderers. With the visible one restored and the gated one kept, the id looked contested, so an unrelated reference fell to its holder and kept an id nothing renders once the gated node is pruned. Outcomes are now noted in two maps split by hiddenSubtreeNodes, the renderer's own pruning rule with inherited gating. settledTarget decides what a reference follows: visible renderers whenever any carries the id, gated ones only where none does, so a link to a target gated today is not split from it when the gate opens. A gated node still has its own id restored. --- .../src/composition-planners.test.ts | 41 ++++++++++ .../src/tree.reid-with-map.test.ts | 56 ++++++++++++++ packages/blocks-engine/src/tree.ts | 76 +++++++++++++------ 3 files changed, 151 insertions(+), 22 deletions(-) diff --git a/packages/blocks-engine/src/composition-planners.test.ts b/packages/blocks-engine/src/composition-planners.test.ts index 1e73e7c491..19c25132c4 100644 --- a/packages/blocks-engine/src/composition-planners.test.ts +++ b/packages/blocks-engine/src/composition-planners.test.ts @@ -4825,6 +4825,47 @@ describe("a rename record names the nodes it renamed", () => { ).toBe("pricing"); }); + it("points an unrelated link at the visible renamed node past a gated namesake", () => { + // The namesake is gated, so the page renders only the renamed node. The + // link belongs to a component that renamed nothing; it has to follow the + // node that actually renders the id rather than keep one nothing renders. + const doc = insertedPage(); + const renamed = marked([...doc.nodes], "renamed"); + const mid = marked([...doc.nodes], "mid"); + const withGatedNamesake = applyOps(doc, [ + { + kind: "insert", + node: node("namesake", { + cssId: renamed.cssId, + props: { mark: "namesake" }, + visibility: { + conditions: [[{ field: "tier", op: "eq", value: "pro" }]], + }, + }), + at: { parentId: mid.id, slot: "children", index: 0 }, + }, + { + kind: "insert", + node: node("stranger", { + origin: { from: "component", id: "def-1" }, + attributes: { "aria-describedby": renamed.cssId ?? "" }, + props: { mark: "stranger" }, + } as Partial), + at: { parentId: mid.id, slot: "children", index: 0 }, + }, + ]).document; + + expect(stored(withGatedNamesake, "wrap", "renamed").cssId).toBe("pricing"); + expect(stored(withGatedNamesake, "wrap", "namesake").cssId).toBe( + renamed.cssId + ); + expect( + stored(withGatedNamesake, "wrap", "stranger").attributes?.[ + "aria-describedby" + ] + ).toBe("pricing"); + }); + it("keeps a governed link pointing at a target that keeps its id", () => { // The listed node stays on the page but outside the selection, so the // record is live and the link's own record would put the source name back. diff --git a/packages/blocks-engine/src/tree.reid-with-map.test.ts b/packages/blocks-engine/src/tree.reid-with-map.test.ts index 3b77fb2dfb..f11093bdb6 100644 --- a/packages/blocks-engine/src/tree.reid-with-map.test.ts +++ b/packages/blocks-engine/src/tree.reid-with-map.test.ts @@ -987,6 +987,62 @@ describe('the "restoreEach" DOM id policy', () => { expect([...domIds.entries()]).toEqual([["hero-7f3", "hero"]]); }); + /** A condition gate the renderer prunes for a viewer who does not match. */ + const gate = { + conditions: [[{ field: "tier", op: "eq", value: "pro" }]], + } as BlockNode["visibility"]; + + it("follows the visible target past a gated namesake", () => { + // The gated namesake renders nothing, so the reference resolves to the + // visible node on the page. Counting both as targets made the id look + // contested and left the reference on an id nothing renders. + const { nodes } = reidForestWithMap( + [ + node("visible", { cssId: "shared-1" }), + node("hidden", { cssId: "shared-1", visibility: gate }), + node("holder", { attributes: { "aria-describedby": "shared-1" } }), + ], + { restoreEach: answers({ visible: { "shared-1": "shared" } }) } + ); + + expect(nodes[0].cssId).toBe("shared"); + expect(nodes[1].cssId).toBe("shared-1"); + expect(nodes[2].attributes?.["aria-describedby"]).toBe("shared"); + }); + + it("follows a gated target when nothing visible carries the id", () => { + // Excluding gated nodes outright would hand this reference to its holder, + // which answers nothing — storing a link to an id its target no longer + // carries once the gate opens. + const { nodes } = reidForestWithMap( + [ + node("hidden", { cssId: "shared-1", visibility: gate }), + node("holder", { attributes: { "aria-describedby": "shared-1" } }), + ], + { restoreEach: answers({ hidden: { "shared-1": "shared" } }) } + ); + + expect(nodes[0].cssId).toBe("shared"); + expect(nodes[1].attributes?.["aria-describedby"]).toBe("shared"); + }); + + it("treats a node inside a gated subtree as gated", () => { + // Gating is inherited: the renderer prunes the whole subtree, so a + // namesake under a gated parent renders nothing either. + const { nodes } = reidForestWithMap( + [ + node("visible", { cssId: "shared-1" }), + node("wrapper", { visibility: gate }, [ + node("nested", { cssId: "shared-1" }), + ]), + node("holder", { attributes: { "aria-describedby": "shared-1" } }), + ], + { restoreEach: answers({ visible: { "shared-1": "shared" } }) } + ); + + expect(nodes[0].cssId).toBe("shared"); + expect(nodes[2].attributes?.["aria-describedby"]).toBe("shared"); + }); }); describe("only the id a node RENDERS may be reminted", () => { diff --git a/packages/blocks-engine/src/tree.ts b/packages/blocks-engine/src/tree.ts index b84fab3763..7203dbe2c7 100644 --- a/packages/blocks-engine/src/tree.ts +++ b/packages/blocks-engine/src/tree.ts @@ -1506,8 +1506,12 @@ type RestoreEach = (node: BlockNode, value: string) => string | undefined; /** What the per-node copier carries from node to node. */ interface EachCopy { readonly nodeIds: Map; - /** Each rendered id → every id the nodes rendering it ended up with. */ + /** Each id a VISIBLE node renders → every id those nodes ended up with. */ readonly outcomes: Map>; + /** The same, for nodes inside a subtree the renderer prunes. */ + readonly gatedOutcomes: Map>; + /** Every node the renderer would prune, by identity. */ + readonly hidden: ReadonlySet; /** Each copy's id → the original it was made from, for the reference pass. */ readonly holders: Map; readonly restoreEach: RestoreEach; @@ -1522,14 +1526,21 @@ interface EachCopy { * settle takes the settled id, so it follows its target; any other reference is * decided by the node holding it. * - * No hidden-subtree set, and no shared memo of replacements. Putting an id back - * asks nothing about the page, so gating is irrelevant; and two nodes spelling - * one id may now answer differently, which a memo keyed by the id alone would - * collapse into one answer for both. - * - * `domIds` reports a rendered id only where every node rendering it moved to - * the same id — the one reading of "what this copy moved" that stays true when - * renderers disagree. + * Gating decides only which nodes a reference follows, never which ids move. + * Putting an id back asks nothing about the page, so a gated node still has its + * own id restored. But a reference resolves to the element the page RENDERS, + * and a gated namesake renders nothing — so the nodes the renderer keeps settle + * a reference first, and gated ones only when no visible node carries the id. + * Treating both alike made a visible target and its gated namesake look + * contested, and left an unrelated link on an id nothing renders. + * + * No shared memo of replacements: two nodes spelling one id may answer + * differently, which a memo keyed by the id alone would collapse into one + * answer for both. + * + * `domIds` reports a rendered id only where the nodes that settle it all moved + * to the same id — the one reading of "what this copy moved" that stays true + * when renderers disagree. */ function reidForestRestoringEach( nodes: BlockNode[], @@ -1538,6 +1549,10 @@ function reidForestRestoringEach( const each: EachCopy = { nodeIds: new Map(), outcomes: new Map(), + gatedOutcomes: new Map(), + // The renderer's own pruning rule, inherited gating included, rather than + // a second reading of which nodes are visible. + hidden: hiddenSubtreeNodes(nodes), holders: new Map(), restoreEach, }; @@ -1553,7 +1568,7 @@ function reidForestRestoringEach( return { nodes: linked, nodeIds: each.nodeIds, - domIds: settledMoves(each.outcomes), + domIds: settledMoves(each), }; } @@ -1566,7 +1581,11 @@ function reidOneRestoringEach(node: BlockNode, each: EachCopy): BlockNode { const rendered = renderedDomId(node); if (rendered !== undefined) { const answer = each.restoreEach(node, rendered); - noteOutcome(each.outcomes, rendered, answer ?? rendered); + noteOutcome( + each.hidden.has(node) ? each.gatedOutcomes : each.outcomes, + rendered, + answer ?? rendered + ); if (answer !== undefined) { moveOwnIds( copy, @@ -1598,13 +1617,29 @@ function soleOutcome( return only; } -/** Each rendered id that moved, to the one id all of its renderers became. */ -function settledMoves( - outcomes: ReadonlyMap> -): Map { +/** + * The one id a reference to `value` follows, or nothing when the copy does not + * settle it. + * + * The nodes the page renders decide whenever any of them carries the id; gated + * nodes decide only where none does, so a link to a target that is gated today + * still follows it and is not split from it when the gate opens. + */ +function settledTarget(each: EachCopy, value: string): string | undefined { + const visible = each.outcomes.get(value); + if (visible !== undefined) return soleOutcome(visible); + return soleOutcome(each.gatedOutcomes.get(value)); +} + +/** Each rendered id that moved, to the one id the nodes settling it became. */ +function settledMoves(each: EachCopy): Map { const moved = new Map(); - for (const [value, became] of outcomes) { - const only = soleOutcome(became); + const values = new Set([ + ...each.outcomes.keys(), + ...each.gatedOutcomes.keys(), + ]); + for (const value of values) { + const only = settledTarget(each, value); if (only !== undefined && only !== value) moved.set(value, only); } return moved; @@ -1629,11 +1664,8 @@ function referenceAnswers( return { size: 1, get(value: string): string | undefined { - const became = each.outcomes.get(value); - if (became !== undefined && became.size === 1) { - const settled = soleOutcome(became); - return settled === value ? undefined : settled; - } + const settled = settledTarget(each, value); + if (settled !== undefined) return settled === value ? undefined : settled; return holder === undefined ? undefined : each.restoreEach(holder, value); }, }; From a6a1ad5f3b250ec4c0e765d96282b0154e31305e Mon Sep 17 00:00:00 2001 From: Mobeen Abdullah Date: Sun, 13 Sep 2026 19:11:31 +0300 Subject: [PATCH 2/4] chore(blocks-engine): record that a link follows the element the page shows The changeset for the reference-target fix, which changes what a saved pattern stores when a visible element and a gated copy share one id. --- ...link-follows-the-element-the-page-shows.md | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 .changeset/a-link-follows-the-element-the-page-shows.md diff --git a/.changeset/a-link-follows-the-element-the-page-shows.md b/.changeset/a-link-follows-the-element-the-page-shows.md new file mode 100644 index 0000000000..939fbc7145 --- /dev/null +++ b/.changeset/a-link-follows-the-element-the-page-shows.md @@ -0,0 +1,37 @@ +--- +"@nextlyhq/adapter-drizzle": patch +"@nextlyhq/adapter-mysql": patch +"@nextlyhq/adapter-postgres": patch +"@nextlyhq/adapter-sqlite": patch +"@nextlyhq/admin": patch +"@nextlyhq/admin-css": patch +"@nextlyhq/blocks-engine": patch +"@nextlyhq/blocks-react": patch +"@nextlyhq/builder": patch +"create-nextly-app": patch +"@nextlyhq/eslint-config": patch +"@nextlyhq/eslint-plugin": patch +"@nextlyhq/module-specifiers": patch +"nextly": patch +"@nextlyhq/plugin-form-builder": patch +"@nextlyhq/plugin-mcp": patch +"@nextlyhq/plugin-page-builder": patch +"@nextlyhq/plugin-sdk": patch +"@nextlyhq/plugin-seo": patch +"@nextlyhq/prettier-config": patch +"@nextlyhq/storage-s3": patch +"@nextlyhq/storage-uploadthing": patch +"@nextlyhq/storage-vercel-blob": patch +"@nextlyhq/telemetry": patch +"@nextlyhq/tsconfig": patch +"@nextlyhq/ui": patch +--- + +A link saved in a pattern now points at the element the page actually shows. +When a selection held a visible renamed element and a hidden copy with the +same id (hidden by a visibility condition), a link from an unrelated block could +keep the old id, which nothing renders once the hidden copy is left off the +page. The visible element now decides where such a link points. A hidden +element decides only when nothing visible carries that id, so a link and its +target stay together when the condition later shows it. Hidden elements still +get their own ids put back. From ce2c7387608f8758bd3a1baed9492975cae1c3b0 Mon Sep 17 00:00:00 2001 From: Mobeen Abdullah Date: Sun, 13 Sep 2026 20:13:00 +0300 Subject: [PATCH 3/4] fix(blocks-engine): a reference follows what renders whenever it does Letting visible renderers settle every reference handed a reference inside a gated subtree to an unrelated visible namesake. With the gated target restored beside it, the pattern's own link kept the old id and pointed at the namesake once the gate opened. A reference renders only when every gate above it is open, and then so does every node behind those same gates and everything ungated. Those nodes now settle it: where they agree the reference takes their id, where they disagree its holder decides, and where none carries the id every carrier decides. hiddenSubtreeNodes is derived from a new reading of the gates over each node, so which nodes are pruned and which gates prune them come from one walk. --- .../src/composition-planners.test.ts | 38 ++++ .../src/tree.reid-with-map.test.ts | 74 ++++++++ packages/blocks-engine/src/tree.ts | 169 +++++++++++++----- 3 files changed, 233 insertions(+), 48 deletions(-) diff --git a/packages/blocks-engine/src/composition-planners.test.ts b/packages/blocks-engine/src/composition-planners.test.ts index 19c25132c4..39dc950f29 100644 --- a/packages/blocks-engine/src/composition-planners.test.ts +++ b/packages/blocks-engine/src/composition-planners.test.ts @@ -4866,6 +4866,44 @@ describe("a rename record names the nodes it renamed", () => { ).toBe("pricing"); }); + it("restores a pattern's link inside a gated container beside a visible namesake", () => { + // The container holding the renamed node and its link is gated, and an + // unrelated visible node carries the minted id. Whenever the link renders, + // its own target renders with it, so the visible namesake must not decide. + const doc = insertedPage(); + const renamed = marked([...doc.nodes], "renamed"); + const wrap = marked([...doc.nodes], "wrap"); + const mid = marked([...doc.nodes], "mid"); + const gatedWithNamesake = applyOps(doc, [ + { + kind: "update", + id: mid.id, + patch: { + visibility: { + conditions: [[{ field: "tier", op: "eq", value: "pro" }]], + }, + }, + }, + { + kind: "insert", + node: node("unrelated", { + origin: { from: "component", id: "def-1" }, + cssId: renamed.cssId, + props: { mark: "unrelated" }, + } as Partial), + at: { parentId: wrap.id, slot: "children", index: 1 }, + }, + ]).document; + + expect(stored(gatedWithNamesake, "wrap", "renamed").cssId).toBe("pricing"); + expect(stored(gatedWithNamesake, "wrap", "unrelated").cssId).toBe( + renamed.cssId + ); + expect( + stored(gatedWithNamesake, "wrap", "link").attributes?.["aria-describedby"] + ).toBe("pricing"); + }); + it("keeps a governed link pointing at a target that keeps its id", () => { // The listed node stays on the page but outside the selection, so the // record is live and the link's own record would put the source name back. diff --git a/packages/blocks-engine/src/tree.reid-with-map.test.ts b/packages/blocks-engine/src/tree.reid-with-map.test.ts index f11093bdb6..4527511af3 100644 --- a/packages/blocks-engine/src/tree.reid-with-map.test.ts +++ b/packages/blocks-engine/src/tree.reid-with-map.test.ts @@ -1043,6 +1043,80 @@ describe('the "restoreEach" DOM id policy', () => { expect(nodes[0].cssId).toBe("shared"); expect(nodes[2].attributes?.["aria-describedby"]).toBe("shared"); }); + it("lets a link inside a gated subtree follow the target it renders with", () => { + // The link renders only when its gate opens, and then its gated target + // renders too, beside an unrelated visible namesake. The two disagree, so + // there is no single target and the link's own record decides — it must not + // be handed to the visible namesake just because that one always renders. + const { nodes } = reidForestWithMap( + [ + node("unrelated", { cssId: "shared-1" }), + node("wrapper", { visibility: gate }, [ + node("target", { cssId: "shared-1" }), + node("link", { attributes: { "aria-describedby": "shared-1" } }), + ]), + ], + { + restoreEach: answers({ + target: { "shared-1": "shared" }, + link: { "shared-1": "shared" }, + }), + } + ); + const inside = nodes[1].slots?.children ?? []; + + expect(nodes[0].cssId).toBe("shared-1"); + expect(inside[0]?.cssId).toBe("shared"); + expect(inside[1]?.attributes?.["aria-describedby"]).toBe("shared"); + }); + + it("keeps a gated link on the one node that renders with it", () => { + // Nothing visible carries the id; inside the gate the only node rendering + // it keeps its id. Following the link's own record instead would restore + // the link and leave its target behind. + const { nodes } = reidForestWithMap( + [ + node("wrapper", { visibility: gate }, [ + node("namesake", { cssId: "shared-1" }), + node("link", { attributes: { "aria-describedby": "shared-1" } }), + ]), + ], + { restoreEach: answers({ link: { "shared-1": "shared" } }) } + ); + const inside = nodes[0].slots?.children ?? []; + + expect(inside[0]?.cssId).toBe("shared-1"); + expect(inside[1]?.attributes?.["aria-describedby"]).toBe("shared-1"); + }); + + it("counts every gate above a link, not only the nearest", () => { + // The target sits under the outer gate; the link under the outer gate and + // an inner one. Whenever the link renders both gates are open, so the target + // renders with it — and disagrees with the visible namesake, leaving the + // link's own record to decide. + const { nodes } = reidForestWithMap( + [ + node("unrelated", { cssId: "shared-1" }), + node("outer", { visibility: gate }, [ + node("target", { cssId: "shared-1" }), + node("inner", { visibility: gate }, [ + node("link", { attributes: { "aria-describedby": "shared-1" } }), + ]), + ]), + ], + { + restoreEach: answers({ + target: { "shared-1": "shared" }, + link: { "shared-1": "shared" }, + }), + } + ); + const outer = nodes[1].slots?.children ?? []; + const inner = outer[1]?.slots?.children ?? []; + + expect(outer[0]?.cssId).toBe("shared"); + expect(inner[0]?.attributes?.["aria-describedby"]).toBe("shared"); + }); }); describe("only the id a node RENDERS may be reminted", () => { diff --git a/packages/blocks-engine/src/tree.ts b/packages/blocks-engine/src/tree.ts index 7203dbe2c7..17f7a3e66c 100644 --- a/packages/blocks-engine/src/tree.ts +++ b/packages/blocks-engine/src/tree.ts @@ -1506,12 +1506,13 @@ type RestoreEach = (node: BlockNode, value: string) => string | undefined; /** What the per-node copier carries from node to node. */ interface EachCopy { readonly nodeIds: Map; - /** Each id a VISIBLE node renders → every id those nodes ended up with. */ - readonly outcomes: Map>; - /** The same, for nodes inside a subtree the renderer prunes. */ - readonly gatedOutcomes: Map>; - /** Every node the renderer would prune, by identity. */ - readonly hidden: ReadonlySet; + /** + * Each rendered id → the nearest gate over the nodes rendering it + * (`undefined` for none) → every id those nodes ended up with. + */ + readonly outcomes: Map>>; + /** The condition gates over every node of the forest. */ + readonly gates: ConditionGates; /** Each copy's id → the original it was made from, for the reference pass. */ readonly holders: Map; readonly restoreEach: RestoreEach; @@ -1528,11 +1529,13 @@ interface EachCopy { * * Gating decides only which nodes a reference follows, never which ids move. * Putting an id back asks nothing about the page, so a gated node still has its - * own id restored. But a reference resolves to the element the page RENDERS, - * and a gated namesake renders nothing — so the nodes the renderer keeps settle - * a reference first, and gated ones only when no visible node carries the id. - * Treating both alike made a visible target and its gated namesake look - * contested, and left an unrelated link on an id nothing renders. + * own id restored. But a reference resolves to the element that renders WITH + * it: a reference renders only when every gate above it is open, and then so + * does every node behind those same gates, as well as everything ungated. Those + * nodes settle it. A gated namesake the reference does not share a gate with + * renders nothing when the reference does, so it has no say — and a reference + * inside a gated subtree is not handed to a visible namesake just because that + * one always renders. * * No shared memo of replacements: two nodes spelling one id may answer * differently, which a memo keyed by the id alone would collapse into one @@ -1549,10 +1552,9 @@ function reidForestRestoringEach( const each: EachCopy = { nodeIds: new Map(), outcomes: new Map(), - gatedOutcomes: new Map(), - // The renderer's own pruning rule, inherited gating included, rather than - // a second reading of which nodes are visible. - hidden: hiddenSubtreeNodes(nodes), + // The reading the renderer's pruning rule is derived from, inherited gating + // included, rather than a second reading of which nodes are visible. + gates: conditionGates(nodes), holders: new Map(), restoreEach, }; @@ -1581,11 +1583,7 @@ function reidOneRestoringEach(node: BlockNode, each: EachCopy): BlockNode { const rendered = renderedDomId(node); if (rendered !== undefined) { const answer = each.restoreEach(node, rendered); - noteOutcome( - each.hidden.has(node) ? each.gatedOutcomes : each.outcomes, - rendered, - answer ?? rendered - ); + noteOutcome(each, node, rendered, answer ?? rendered); if (answer !== undefined) { moveOwnIds( copy, @@ -1599,12 +1597,19 @@ function reidOneRestoringEach(node: BlockNode, each: EachCopy): BlockNode { } function noteOutcome( - outcomes: Map>, + each: EachCopy, + node: BlockNode, value: string, became: string ): void { - const found = outcomes.get(value); - if (found === undefined) outcomes.set(value, new Set([became])); + let byGate = each.outcomes.get(value); + if (byGate === undefined) { + byGate = new Map(); + each.outcomes.set(value, byGate); + } + const gate = each.gates.nearest.get(node); + const found = byGate.get(gate); + if (found === undefined) byGate.set(gate, new Set([became])); else found.add(became); } @@ -1618,28 +1623,47 @@ function soleOutcome( } /** - * The one id a reference to `value` follows, or nothing when the copy does not - * settle it. - * - * The nodes the page renders decide whenever any of them carries the id; gated - * nodes decide only where none does, so a link to a target that is gated today - * still follows it and is not split from it when the gate opens. + * The one id a reference to `value`, held by `holder`, follows — or nothing + * when the copy does not settle it. + * + * The nodes that render whenever the holder does decide: everything ungated, + * and everything behind a gate that is also over the holder. Where they agree + * on one id, the reference takes it; where they disagree, nothing is settled. + * Where none of them carries the id at all, every node carrying it decides, so + * a link whose only target is gated today still follows it and is not split + * from it when that gate opens. */ -function settledTarget(each: EachCopy, value: string): string | undefined { - const visible = each.outcomes.get(value); - if (visible !== undefined) return soleOutcome(visible); - return soleOutcome(each.gatedOutcomes.get(value)); +function settledTarget( + each: EachCopy, + value: string, + holder: BlockNode | undefined +): string | undefined { + const byGate = each.outcomes.get(value); + if (byGate === undefined) return undefined; + const alongside = outcomesBehind(byGate, gatesOver(each.gates, holder)); + return soleOutcome(alongside ?? outcomesBehind(byGate, byGate.keys())); } -/** Each rendered id that moved, to the one id the nodes settling it became. */ +/** Every id the renderers behind these gates became, or nothing if none did. */ +function outcomesBehind( + byGate: ReadonlyMap>, + gates: Iterable +): Set | undefined { + let found: Set | undefined; + for (const gate of gates) { + const became = byGate.get(gate); + if (became === undefined) continue; + found ??= new Set(); + for (const id of became) found.add(id); + } + return found; +} + +/** Each rendered id that moved, to the one id the page's own renderers became. */ function settledMoves(each: EachCopy): Map { const moved = new Map(); - const values = new Set([ - ...each.outcomes.keys(), - ...each.gatedOutcomes.keys(), - ]); - for (const value of values) { - const only = settledTarget(each, value); + for (const value of each.outcomes.keys()) { + const only = settledTarget(each, value, undefined); if (only !== undefined && only !== value) moved.set(value, only); } return moved; @@ -1664,7 +1688,7 @@ function referenceAnswers( return { size: 1, get(value: string): string | undefined { - const settled = settledTarget(each, value); + const settled = settledTarget(each, value, holder); if (settled !== undefined) return settled === value ? undefined : settled; return holder === undefined ? undefined : each.restoreEach(holder, value); }, @@ -1789,15 +1813,64 @@ export function remapIdReferences( export function hiddenSubtreeNodes( nodes: readonly BlockNode[] ): ReadonlySet { - const hidden = new Set(); - const seen = new Map(); + return new Set(conditionGates(nodes).nearest.keys()); +} + +/** + * Which condition gates cover each node, by identity. + * + * `nearest` maps a node to the innermost condition-gated node on its path — + * itself when it is gated — and holds no entry for a node no gate covers. + * `enclosing` maps each gated node to the next gate out. Following one into the + * other lists every gate that has to open before a node renders. + */ +interface ConditionGates { + readonly nearest: ReadonlyMap; + readonly enclosing: ReadonlyMap; +} + +/** + * The gates over every node of a forest. + * + * The richer reading {@link hiddenSubtreeNodes} is derived from, so which nodes + * are pruned and which gates prune them come from one walk and cannot disagree. + * + * A node inherits from its parent's CURRENT visit, the way the pruning rule + * always has: the walk reaches a node placed in two slots twice, and the gate + * over it is the one on the path it was reached by. It is covered if any visit + * found a gate. + */ +function conditionGates(nodes: readonly BlockNode[]): ConditionGates { + const nearest = new Map(); + const enclosing = new Map(); + const current = new Map(); walkNodes([...nodes], (node, parent) => { - const inherited = parent !== undefined && seen.get(parent) === true; - const gated = inherited || isConditionGated(node); - seen.set(node, gated); - if (gated) hidden.add(node); + const inherited = parent === undefined ? undefined : current.get(parent); + const gate = isConditionGated(node) ? node : inherited; + if (gate === node) enclosing.set(node, inherited); + current.set(node, gate); + if (gate !== undefined) nearest.set(node, gate); }); - return hidden; + return { nearest, enclosing }; +} + +/** + * Every gate that has to open for a node to render, innermost first, with + * `undefined` standing for the page itself — what renders with no gate at all. + * + * Bounded by what it has already listed, so a malformed chain cannot loop. + */ +function gatesOver( + gates: ConditionGates, + node: BlockNode | undefined +): Set { + const chain = new Set([undefined]); + let gate = node === undefined ? undefined : gates.nearest.get(node); + while (gate !== undefined && !chain.has(gate)) { + chain.add(gate); + gate = gates.enclosing.get(gate); + } + return chain; } /** From 2c090ffc80b84353fab1af108bf61d2eb6de63c5 Mon Sep 17 00:00:00 2001 From: Mobeen Abdullah Date: Sun, 13 Sep 2026 21:33:19 +0300 Subject: [PATCH 4/4] fix(blocks-engine): a reference takes its own record's answer first Settling a reference by the element it points at meant deciding which elements render together, and each review round found another case: nested gates, one node placed under two gates, sibling gates with equal conditions. A reference's holder knows where it pointed, so its answer now decides; only a reference with no answer follows its target, visible renderers first. The gate-chain reading is removed. A node placed both open and gated counts as open when an unrelated reference follows it, from one walk that also still gives hiddenSubtreeNodes its any-placement-pruned answer. --- .../src/composition-planners.test.ts | 15 +- .../blocks-engine/src/composition-planners.ts | 8 +- .../src/tree.reid-with-map.test.ts | 55 +++-- packages/blocks-engine/src/tree.ts | 220 ++++++++---------- 4 files changed, 144 insertions(+), 154 deletions(-) diff --git a/packages/blocks-engine/src/composition-planners.test.ts b/packages/blocks-engine/src/composition-planners.test.ts index 39dc950f29..f865eda253 100644 --- a/packages/blocks-engine/src/composition-planners.test.ts +++ b/packages/blocks-engine/src/composition-planners.test.ts @@ -4868,8 +4868,8 @@ describe("a rename record names the nodes it renamed", () => { it("restores a pattern's link inside a gated container beside a visible namesake", () => { // The container holding the renamed node and its link is gated, and an - // unrelated visible node carries the minted id. Whenever the link renders, - // its own target renders with it, so the visible namesake must not decide. + // unrelated visible node carries the minted id. The link's own record knows + // where it pointed, so the visible namesake must not decide. const doc = insertedPage(); const renamed = marked([...doc.nodes], "renamed"); const wrap = marked([...doc.nodes], "wrap"); @@ -4904,11 +4904,12 @@ describe("a rename record names the nodes it renamed", () => { ).toBe("pricing"); }); - it("keeps a governed link pointing at a target that keeps its id", () => { + it("restores a governed link from its own record past a namesake that keeps its id", () => { // The listed node stays on the page but outside the selection, so the - // record is live and the link's own record would put the source name back. - // The link's target in the saved forest is an unlisted namesake, which - // keeps the minted id — so the link follows it and keeps the id too. + // record is live and says what the link pointed at. The saved forest holds + // an unlisted namesake that keeps the minted id; the link's own record + // decides, so it goes back to the source name rather than following the + // namesake — as a link saved without its target already does. const doc = insertedPage(); const renamed = marked([...doc.nodes], "renamed"); const mid = marked([...doc.nodes], "mid"); @@ -4939,7 +4940,7 @@ describe("a rename record names the nodes it renamed", () => { expect(marked([...saved.nodes], "namesake").cssId).toBe(renamed.cssId); expect( marked([...saved.nodes], "link").attributes?.["aria-describedby"] - ).toBe(renamed.cssId); + ).toBe("pricing"); }); it("restores nothing when the recorded node id occurs twice", () => { diff --git a/packages/blocks-engine/src/composition-planners.ts b/packages/blocks-engine/src/composition-planners.ts index f376137e19..1679f9b464 100644 --- a/packages/blocks-engine/src/composition-planners.ts +++ b/packages/blocks-engine/src/composition-planners.ts @@ -3826,8 +3826,9 @@ function liveEntries( * namesake can carry one id, and a reference the run's record governs can sit * beside an unrelated author's reference to the same id — in each, the right * outcome differs by node. The copier decides which question a node is asked: - * what the id it renders was, or, for an id it references that the copy does not - * settle by its target, what that reference was. + * what the id it renders was, or what an id it references was. A reference takes + * its node's answer where there is one, and follows its target only where there + * is none. * * INVERTED from the record, which reads source → copy because that is the * direction an insert renames in. @@ -3904,7 +3905,8 @@ function restoredFor( * the scope lists the nodes it renamed and this is not one of them. * * Only for the node carrying the id itself. A reference is answered by the - * scope alone; whether it follows its target instead is the copier's question. + * scope alone, and that answer decides it; the copier falls back to the target + * only where the scope has none. */ function renderedAnswer( scope: RenameScope | undefined, diff --git a/packages/blocks-engine/src/tree.reid-with-map.test.ts b/packages/blocks-engine/src/tree.reid-with-map.test.ts index 4527511af3..cb44664d7e 100644 --- a/packages/blocks-engine/src/tree.reid-with-map.test.ts +++ b/packages/blocks-engine/src/tree.reid-with-map.test.ts @@ -880,9 +880,9 @@ describe('the "restoreEach" DOM id policy', () => { expect((nodes[1].props as { href: string }).href).toBe("#hero"); }); - it("keeps a reference whose one target keeps its id", () => { - // The holder alone would put the source name back, which would store a - // link naming an id its target no longer carries. + it("restores a reference from its holder's answer even when its target keeps its id", () => { + // The holder knows where its reference pointed, so its answer decides; the + // target is followed only for a reference the holder has no answer for. const { nodes } = reidForestWithMap( [ node("target", { cssId: "hero-7f3" }), @@ -892,7 +892,27 @@ describe('the "restoreEach" DOM id policy', () => { ); expect(nodes[0].cssId).toBe("hero-7f3"); - expect(nodes[1].attributes?.["aria-describedby"]).toBe("hero-7f3"); + expect(nodes[1].attributes?.["aria-describedby"]).toBe("hero"); + }); + + it("follows a target placed both open and gated as an open one", () => { + // One node object sits in an open slot and inside a gated container. It + // renders wherever its open placement does, so an unrelated reference + // follows it past a gated namesake — reading it as gated made the two look + // like competing gated targets and left the reference behind. + const sharedTarget = node("shared", { cssId: "shared-1" }); + const { nodes } = reidForestWithMap( + [ + sharedTarget, + node("wrapper", { visibility: gate }, [sharedTarget]), + node("namesake", { cssId: "shared-1", visibility: gate }), + node("holder", { attributes: { "aria-describedby": "shared-1" } }), + ], + { restoreEach: answers({ shared: { "shared-1": "shared" } }) } + ); + + expect(nodes[0].cssId).toBe("shared"); + expect(nodes[3].attributes?.["aria-describedby"]).toBe("shared"); }); it("lets each holder decide a reference nothing in the forest renders", () => { @@ -1043,11 +1063,10 @@ describe('the "restoreEach" DOM id policy', () => { expect(nodes[0].cssId).toBe("shared"); expect(nodes[2].attributes?.["aria-describedby"]).toBe("shared"); }); - it("lets a link inside a gated subtree follow the target it renders with", () => { - // The link renders only when its gate opens, and then its gated target - // renders too, beside an unrelated visible namesake. The two disagree, so - // there is no single target and the link's own record decides — it must not - // be handed to the visible namesake just because that one always renders. + it("restores a link inside a gated subtree from its own answer past a visible namesake", () => { + // The link's holder has an answer, so it decides: the link must not be + // handed to an unrelated visible namesake just because that one always + // renders. const { nodes } = reidForestWithMap( [ node("unrelated", { cssId: "shared-1" }), @@ -1070,10 +1089,10 @@ describe('the "restoreEach" DOM id policy', () => { expect(inside[1]?.attributes?.["aria-describedby"]).toBe("shared"); }); - it("keeps a gated link on the one node that renders with it", () => { - // Nothing visible carries the id; inside the gate the only node rendering - // it keeps its id. Following the link's own record instead would restore - // the link and leave its target behind. + it("restores a gated link from its own answer beside a namesake that keeps its id", () => { + // The only node rendering the id inside the gate keeps it, and nothing + // visible carries it. The link's holder knows where it pointed, so its + // answer decides rather than whichever node happens to share the id. const { nodes } = reidForestWithMap( [ node("wrapper", { visibility: gate }, [ @@ -1086,14 +1105,12 @@ describe('the "restoreEach" DOM id policy', () => { const inside = nodes[0].slots?.children ?? []; expect(inside[0]?.cssId).toBe("shared-1"); - expect(inside[1]?.attributes?.["aria-describedby"]).toBe("shared-1"); + expect(inside[1]?.attributes?.["aria-describedby"]).toBe("shared"); }); - it("counts every gate above a link, not only the nearest", () => { - // The target sits under the outer gate; the link under the outer gate and - // an inner one. Whenever the link renders both gates are open, so the target - // renders with it — and disagrees with the visible namesake, leaving the - // link's own record to decide. + it("restores a link under nested gates from its own answer past a visible namesake", () => { + // However many gates sit above the link, its holder's answer decides; a + // visible namesake that always renders does not. const { nodes } = reidForestWithMap( [ node("unrelated", { cssId: "shared-1" }), diff --git a/packages/blocks-engine/src/tree.ts b/packages/blocks-engine/src/tree.ts index 17f7a3e66c..94221b1459 100644 --- a/packages/blocks-engine/src/tree.ts +++ b/packages/blocks-engine/src/tree.ts @@ -1110,12 +1110,11 @@ export type DomIdPolicy = * asks with the ORIGINAL node: * * - for the id a node RENDERS, and moves that id to the answer; - * - for an id a node REFERENCES, only when the copy does not settle it. A - * reference follows its target: where the copied forest renders the id - * and every node rendering it ends up with one id, the reference takes - * that id, so a link and its target are never stored apart. Where - * nothing renders it, or its renderers end up with different ids, the - * node holding the reference decides. + * - for an id a node REFERENCES, and a reference takes that answer when + * there is one. The node holding it knows where it pointed. Only a + * reference its holder has no answer for follows its target: where the + * copied forest renders the id and the nodes settling it end up with one + * id, the reference takes that id; otherwise it keeps what it carries. * * Gating is not consulted, for the reason `restore` gives: putting an id * back asks nothing about the page. @@ -1506,13 +1505,12 @@ type RestoreEach = (node: BlockNode, value: string) => string | undefined; /** What the per-node copier carries from node to node. */ interface EachCopy { readonly nodeIds: Map; - /** - * Each rendered id → the nearest gate over the nodes rendering it - * (`undefined` for none) → every id those nodes ended up with. - */ - readonly outcomes: Map>>; - /** The condition gates over every node of the forest. */ - readonly gates: ConditionGates; + /** Each id a VISIBLE node renders → every id those nodes ended up with. */ + readonly outcomes: Map>; + /** The same, for nodes inside a subtree the renderer prunes. */ + readonly gatedOutcomes: Map>; + /** Every node the renderer would prune, by identity. */ + readonly hidden: ReadonlySet; /** Each copy's id → the original it was made from, for the reference pass. */ readonly holders: Map; readonly restoreEach: RestoreEach; @@ -1523,19 +1521,24 @@ interface EachCopy { * * Two passes, as every policy takes. The first moves the id each node renders * to that node's own answer and notes, per id, every id its renderers ended up - * with. The second rewrites references: a reference to an id those notes - * settle takes the settled id, so it follows its target; any other reference is - * decided by the node holding it. + * with. The second rewrites references: a reference its holder has an answer + * for takes that answer, and only one it has none for follows the id those notes + * settle. + * + * The holder first, because it knows where its reference pointed. Settling a + * reference by its target instead means deciding which elements render + * together, and that question has no end: gated namesakes, nested gates, a node + * placed under two gates, sibling gates whose conditions agree, and conditions + * equal in meaning but written differently, which nothing can decide from the + * document. Following the target is kept for the reference nobody recorded. * * Gating decides only which nodes a reference follows, never which ids move. * Putting an id back asks nothing about the page, so a gated node still has its - * own id restored. But a reference resolves to the element that renders WITH - * it: a reference renders only when every gate above it is open, and then so - * does every node behind those same gates, as well as everything ungated. Those - * nodes settle it. A gated namesake the reference does not share a gate with - * renders nothing when the reference does, so it has no say — and a reference - * inside a gated subtree is not handed to a visible namesake just because that - * one always renders. + * own id restored. But a reference resolves to the element the page RENDERS, + * and a gated namesake renders nothing — so the nodes the renderer keeps settle + * a reference first, and gated ones only when no visible node carries the id. + * Treating both alike made a visible target and its gated namesake look + * contested, and left an unrelated link on an id nothing renders. * * No shared memo of replacements: two nodes spelling one id may answer * differently, which a memo keyed by the id alone would collapse into one @@ -1552,9 +1555,12 @@ function reidForestRestoringEach( const each: EachCopy = { nodeIds: new Map(), outcomes: new Map(), - // The reading the renderer's pruning rule is derived from, inherited gating - // included, rather than a second reading of which nodes are visible. - gates: conditionGates(nodes), + gatedOutcomes: new Map(), + // Gated where no placement of the node is open. The pruning rule's own walk, + // read the other way: `hiddenSubtreeNodes` asks whether ANY placement is + // pruned, and a node an unrelated reference follows renders wherever any + // placement is not. + hidden: prunedInEveryPlacement(nodes), holders: new Map(), restoreEach, }; @@ -1583,7 +1589,11 @@ function reidOneRestoringEach(node: BlockNode, each: EachCopy): BlockNode { const rendered = renderedDomId(node); if (rendered !== undefined) { const answer = each.restoreEach(node, rendered); - noteOutcome(each, node, rendered, answer ?? rendered); + noteOutcome( + each.hidden.has(node) ? each.gatedOutcomes : each.outcomes, + rendered, + answer ?? rendered + ); if (answer !== undefined) { moveOwnIds( copy, @@ -1597,19 +1607,12 @@ function reidOneRestoringEach(node: BlockNode, each: EachCopy): BlockNode { } function noteOutcome( - each: EachCopy, - node: BlockNode, + outcomes: Map>, value: string, became: string ): void { - let byGate = each.outcomes.get(value); - if (byGate === undefined) { - byGate = new Map(); - each.outcomes.set(value, byGate); - } - const gate = each.gates.nearest.get(node); - const found = byGate.get(gate); - if (found === undefined) byGate.set(gate, new Set([became])); + const found = outcomes.get(value); + if (found === undefined) outcomes.set(value, new Set([became])); else found.add(became); } @@ -1623,47 +1626,28 @@ function soleOutcome( } /** - * The one id a reference to `value`, held by `holder`, follows — or nothing - * when the copy does not settle it. - * - * The nodes that render whenever the holder does decide: everything ungated, - * and everything behind a gate that is also over the holder. Where they agree - * on one id, the reference takes it; where they disagree, nothing is settled. - * Where none of them carries the id at all, every node carrying it decides, so - * a link whose only target is gated today still follows it and is not split - * from it when that gate opens. + * The one id a reference to `value` follows, or nothing when the copy does not + * settle it. + * + * The nodes the page renders decide whenever any of them carries the id; gated + * nodes decide only where none does, so a link to a target that is gated today + * still follows it and is not split from it when the gate opens. */ -function settledTarget( - each: EachCopy, - value: string, - holder: BlockNode | undefined -): string | undefined { - const byGate = each.outcomes.get(value); - if (byGate === undefined) return undefined; - const alongside = outcomesBehind(byGate, gatesOver(each.gates, holder)); - return soleOutcome(alongside ?? outcomesBehind(byGate, byGate.keys())); -} - -/** Every id the renderers behind these gates became, or nothing if none did. */ -function outcomesBehind( - byGate: ReadonlyMap>, - gates: Iterable -): Set | undefined { - let found: Set | undefined; - for (const gate of gates) { - const became = byGate.get(gate); - if (became === undefined) continue; - found ??= new Set(); - for (const id of became) found.add(id); - } - return found; +function settledTarget(each: EachCopy, value: string): string | undefined { + const visible = each.outcomes.get(value); + if (visible !== undefined) return soleOutcome(visible); + return soleOutcome(each.gatedOutcomes.get(value)); } -/** Each rendered id that moved, to the one id the page's own renderers became. */ +/** Each rendered id that moved, to the one id the nodes settling it became. */ function settledMoves(each: EachCopy): Map { const moved = new Map(); - for (const value of each.outcomes.keys()) { - const only = settledTarget(each, value, undefined); + const values = new Set([ + ...each.outcomes.keys(), + ...each.gatedOutcomes.keys(), + ]); + for (const value of values) { + const only = settledTarget(each, value); if (only !== undefined && only !== value) moved.set(value, only); } return moved; @@ -1672,9 +1656,9 @@ function settledMoves(each: EachCopy): Map { /** * What each id one node references should now be. * - * Settled by the copy where it can be: an id the forest renders, whose - * renderers all ended up with one id, is that id — kept or moved — whatever the - * holder would have said. Otherwise the holder decides. + * The holder's own answer where it has one. Otherwise settled by the copy where + * it can be: an id the forest renders, whose settling renderers all ended up + * with one id, is that id — kept or moved. Otherwise the reference is kept. * * A lookup rather than a map, because the answer depends on the holder and * building a whole map per node would put every id the records name into the @@ -1688,9 +1672,11 @@ function referenceAnswers( return { size: 1, get(value: string): string | undefined { - const settled = settledTarget(each, value, holder); - if (settled !== undefined) return settled === value ? undefined : settled; - return holder === undefined ? undefined : each.restoreEach(holder, value); + const own = + holder === undefined ? undefined : each.restoreEach(holder, value); + if (own !== undefined) return own; + const settled = settledTarget(each, value); + return settled === value ? undefined : settled; }, }; } @@ -1813,64 +1799,48 @@ export function remapIdReferences( export function hiddenSubtreeNodes( nodes: readonly BlockNode[] ): ReadonlySet { - return new Set(conditionGates(nodes).nearest.keys()); + return placementGating(nodes).gated; } /** - * Which condition gates cover each node, by identity. + * Every node no placement of which is open, by identity. * - * `nearest` maps a node to the innermost condition-gated node on its path — - * itself when it is gated — and holds no entry for a node no gate covers. - * `enclosing` maps each gated node to the next gate out. Following one into the - * other lists every gate that has to open before a node renders. + * The walk reaches a node object placed in two slots once per placement. A + * placement under a gate is pruned and one under none renders, so such a node + * renders wherever any placement is open — the question an unrelated reference + * following its target needs answered, where {@link hiddenSubtreeNodes} answers + * whether any placement is pruned. */ -interface ConditionGates { - readonly nearest: ReadonlyMap; - readonly enclosing: ReadonlyMap; +function prunedInEveryPlacement( + nodes: readonly BlockNode[] +): ReadonlySet { + const { gated, open } = placementGating(nodes); + return new Set([...gated].filter(node => !open.has(node))); } /** - * The gates over every node of a forest. - * - * The richer reading {@link hiddenSubtreeNodes} is derived from, so which nodes - * are pruned and which gates prune them come from one walk and cannot disagree. + * Which nodes are reached under a gate on some placement, and which are reached + * under none on some placement. * - * A node inherits from its parent's CURRENT visit, the way the pruning rule - * always has: the walk reaches a node placed in two slots twice, and the gate - * over it is the one on the path it was reached by. It is covered if any visit - * found a gate. + * One walk for both readings, so the pruning rule and its converse cannot come + * to disagree about inheritance. A placement inherits from its parent's CURRENT + * visit, because the gate over a placement is the one on the path it was + * reached by. */ -function conditionGates(nodes: readonly BlockNode[]): ConditionGates { - const nearest = new Map(); - const enclosing = new Map(); - const current = new Map(); +function placementGating(nodes: readonly BlockNode[]): { + gated: Set; + open: Set; +} { + const gated = new Set(); + const open = new Set(); + const current = new Map(); walkNodes([...nodes], (node, parent) => { - const inherited = parent === undefined ? undefined : current.get(parent); - const gate = isConditionGated(node) ? node : inherited; - if (gate === node) enclosing.set(node, inherited); - current.set(node, gate); - if (gate !== undefined) nearest.set(node, gate); + const inherited = parent !== undefined && current.get(parent) === true; + const isGated = inherited || isConditionGated(node); + current.set(node, isGated); + (isGated ? gated : open).add(node); }); - return { nearest, enclosing }; -} - -/** - * Every gate that has to open for a node to render, innermost first, with - * `undefined` standing for the page itself — what renders with no gate at all. - * - * Bounded by what it has already listed, so a malformed chain cannot loop. - */ -function gatesOver( - gates: ConditionGates, - node: BlockNode | undefined -): Set { - const chain = new Set([undefined]); - let gate = node === undefined ? undefined : gates.nearest.get(node); - while (gate !== undefined && !chain.has(gate)) { - chain.add(gate); - gate = gates.enclosing.get(gate); - } - return chain; + return { gated, open }; } /**