From 1818c79b28882e0b794966cc1dae32d763663eec Mon Sep 17 00:00:00 2001 From: Jiho Lee Date: Sun, 26 Jul 2026 02:26:52 +0900 Subject: [PATCH] fix: make getString render ids stable per circuit (#2848) --- .../PrimitiveComponent/PrimitiveComponent.ts | 46 ++++++++++++-- .../render-id-is-circuit-stable.test.tsx | 63 +++++++++++++++++++ 2 files changed, 103 insertions(+), 6 deletions(-) create mode 100644 tests/components/base-components/render-id-is-circuit-stable.test.tsx diff --git a/lib/components/base-components/PrimitiveComponent/PrimitiveComponent.ts b/lib/components/base-components/PrimitiveComponent/PrimitiveComponent.ts index ec708b281..3065b7874 100644 --- a/lib/components/base-components/PrimitiveComponent/PrimitiveComponent.ts +++ b/lib/components/base-components/PrimitiveComponent/PrimitiveComponent.ts @@ -1384,22 +1384,56 @@ export abstract class PrimitiveComponent< getString(): string { const { lowercaseComponentName: cname, _parsedProps: props, parent } = this + const id = this._getCircuitRelativeRenderId() if (props?.pinNumber !== undefined && parent?.props?.name && props?.name) { - return `<${cname}#${this._renderId}(pin:${props.pinNumber} .${parent?.props.name}>.${props.name}) />` + return `<${cname}#${id}(pin:${props.pinNumber} .${parent?.props.name}>.${props.name}) />` } if (parent?.props?.name && props?.name) { - return `<${cname}#${this._renderId}(.${parent?.props.name}>.${props?.name}) />` + return `<${cname}#${id}(.${parent?.props.name}>.${props?.name}) />` } if (props?.from && props?.to) { - return `<${cname}#${this._renderId}(from:${props.from} to:${props?.to}) />` + return `<${cname}#${id}(from:${props.from} to:${props?.to}) />` } if (props?.name) { - return `<${cname}#${this._renderId} name=".${props?.name}" />` + return `<${cname}#${id} name=".${props?.name}" />` } if (props?.portHints) { - return `<${cname}#${this._renderId}(${props.portHints.map((ph: string) => `.${ph}`).join(", ")}) />` + return `<${cname}#${id}(${props.portHints.map((ph: string) => `.${ph}`).join(", ")}) />` } - return `<${cname}#${this._renderId} />` + return `<${cname}#${id} />` + } + + /** + * A render id that is stable for a given circuit. + * + * `_renderId` comes from a module-level counter that is never reset, so it + * keeps climbing across every circuit built in the same process. That leaks + * into user-facing warnings and errors via `getString()`, making the same + * board produce different messages on each render. This offsets it against + * the lowest id seen in the circuit, so the number is relative to the + * circuit rather than to the process. + */ + _getCircuitRelativeRenderId(): string { + const root = this.root as unknown as { + _renderIdOrigin?: number + children?: any[] + } | null + if (!root) return this._renderId + + if (root._renderIdOrigin === undefined) { + let lowest = Number.POSITIVE_INFINITY + const visit = (node: any) => { + const value = Number(node?._renderId) + if (Number.isFinite(value)) lowest = Math.min(lowest, value) + for (const child of node?.children ?? []) visit(child) + } + for (const child of root.children ?? []) visit(child) + root._renderIdOrigin = Number.isFinite(lowest) ? lowest : 0 + } + + const relative = Number(this._renderId) - root._renderIdOrigin + if (!Number.isFinite(relative) || relative < 0) return this._renderId + return `${relative}` } getDisplayName(): string { diff --git a/tests/components/base-components/render-id-is-circuit-stable.test.tsx b/tests/components/base-components/render-id-is-circuit-stable.test.tsx new file mode 100644 index 000000000..41a8ad7e9 --- /dev/null +++ b/tests/components/base-components/render-id-is-circuit-stable.test.tsx @@ -0,0 +1,63 @@ +import { expect, test } from "bun:test" +import { getTestFixture } from "tests/fixtures/get-test-fixture" + +const buildAndGetWarning = async () => { + const { circuit } = getTestFixture() + + circuit.add( + + + + + , + ) + + await circuit.renderUntilSettled() + + const warning = circuit + .getCircuitJson() + .find((e: any) => e.type === "source_unnamed_trace_warning") as any + + return warning?.message as string | undefined +} + +test("warning messages are identical for identical circuits", async () => { + // `getString()` embeds `_renderId`, which comes from a module-level counter + // that is never reset. Rendering the same board three times in one process + // used to yield "trace#14", "trace#36", "trace#58" — so any consumer that + // snapshots or diffs warnings sees spurious changes. + const messages = [ + await buildAndGetWarning(), + await buildAndGetWarning(), + await buildAndGetWarning(), + ] + + for (const message of messages) { + expect(message).toBeDefined() + expect(message).toContain("is missing a name") + } + + expect(new Set(messages).size).toBe(1) +}) + +test("render ids stay distinct within a circuit", async () => { + const { circuit } = getTestFixture() + + circuit.add( + + + + , + ) + + await circuit.renderUntilSettled() + + const strings = (circuit.selectAll("resistor") as any[]).map((r) => + r.getString(), + ) + + // Making the id circuit-relative must not collapse distinct components onto + // the same identifier. + expect(strings.length).toBe(2) + expect(new Set(strings).size).toBe(2) +})