fix: make getString render ids stable per circuit (#2848)#2849
Open
DPS0340 wants to merge 1 commit into
Open
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #2848.
Rendering the same board three times in one process produced three different warnings:
getString()embeds_renderId, which is a module-level counter that never resets — so the number reflects how many components the process has constructed, not anything about the circuit.Why the fix is in the display path, not the counter
_renderIdhas to stay process-unique:cssSelectPrimitiveComponentAdaptercompares it for identity (a._renderId === b._renderId). Resetting the counter per circuit would break that. SogetString()now offsets against the lowest id in the circuit, computed once and memoised on the root:It falls back to the raw id whenever the offset can't be computed (no root yet, non-numeric id, negative result), so no call site can end up worse off than today.
Verification
The test bites. Reverting
PrimitiveComponent.ts:A second test guards the obvious way to "fix" this wrongly: ids must stay distinct within a circuit. Two resistors on one board still produce two different strings, so collapsing everything to a constant can't pass.
I also checked that circuits of different shapes still get different numbers (a board with an extra capacitor reports
trace#26vstrace#14) — the id remains meaningful, it just stops depending on process history.Suite:
1261 pass / 0 fail,tsc --noEmit0 errors,biome formatclean. No snapshots changed.How this was found
Diffing two renders of an identical board element by element. Of 105 elements,
source_unnamed_trace_warningwas the only one that differed — which is what pointed atgetString()rather than at any single component.Worth noting the blast radius:
getString()also feeds the "routed outside the board boundaries", "chipRef without connections", "manual placement and prop coordinates", "schSheetName does not match", "is ambiguous: references multiple non-overlapping pads", "Missing ports" and "Failed to route net islands" messages. All of them inherit the same instability today.