DEV Community

Cover image for A documented prop that silently stacked every component at the origin
Asuran
Asuran

Posted on

A documented prop that silently stacked every component at the origin

Summer Bug Smash: Clear the Lineup 🐛🛹

This is a submission for DEV's Summer Bug Smash: Clear the Lineup powered by Sentry.

Project Overview

tscircuit lets you describe a printed circuit board in React. You write <resistor name="R1" footprint="0805" /> and the renderer produces real manufacturing output: component placement, copper, silkscreen, a netlist, Gerbers. tscircuit/core is the renderer at the centre of that, so a placement bug there ships into every board anyone exports.

I found this one by building a board, not by reading the source. That distinction matters. I come back to it at the end.

Bug Fix or Performance Improvement

I was laying out an LED matrix and reached for pcbLayout={{ matchAdapt: true }}, a documented prop. Every component landed on top of every other component at the group origin.

Not an error. Not a warning. Four components, one coordinate, a board that looks plausible until you notice all the footprints are in the same place.

The cause is four lines of dispatch in Group.ts:

if (pcbLayoutMode === "grid") {
  this._doInitialPcbLayoutGrid()
} else if (pcbLayoutMode === "pack") {
  this._doInitialPcbLayoutPack()
} else if (pcbLayoutMode === "flex") {
  this._doInitialPcbLayoutFlex()
}
// "match-adapt" falls off the end
Enter fullscreen mode Exit fullscreen mode

Group._getPcbLayoutMode() happily returns "match-adapt". The dispatch handles grid, pack and flex. So the mode is valid on the way in, no branch claims it, no layout runs, so every component keeps the default position it was born with. Schematic match-adapt works fine, which is what makes it convincing: the same prop name does something real on the other view.

That is the shape of bug I have learned to trust. Not a crash you can grep for in a log, but a documented input that quietly produces geometry a fab house would happily build wrong.

Code

PR: tscircuit/core#3137, which fixes issue #3136. +76 lines, no deletions.

Six lines of source:

} else if (pcbLayoutMode === "match-adapt") {
  // There is no dedicated PCB match-adapt layout. Without this branch the
  // mode falls through and every component stays on the group origin,
  // producing overlapping footprints. Fall back to packing so the board is
  // still laid out. Schematic match-adapt is handled separately.
  this._doInitialPcbLayoutPack()
}
Enter fullscreen mode Exit fullscreen mode

Plus a regression test and a PCB snapshot.

My Improvements

The test asserts the bug, not the feature. It would be easy to write a test that renders the board and checks it looks right. That passes for the wrong reasons. Instead it asserts the two things that were actually false before:

// Components must be laid out, not collapsed onto a single origin point.
const distinctCenters = new Set(
  pcbComponents.map((c) => `${c.center.x.toFixed(2)},${c.center.y.toFixed(2)}`),
)
expect(distinctCenters.size).toBe(pcbComponents.length)

// Packing keeps footprints apart, so there should be no overlap DRC errors.
const overlapErrors = (circuit.getCircuitJson() as any[]).filter(
  (el) =>
    el.type === "pcb_courtyard_overlap_error" ||
    el.type === "pcb_footprint_overlap_error",
)
expect(overlapErrors).toHaveLength(0)
Enter fullscreen mode Exit fullscreen mode

Four distinct centres and zero overlap errors. Both fail on the old code, both for the real reason. The PCB snapshot comes along so a reviewer can see the board rather than trust the assertions.

I said what the fix is not. Match-adapt on the PCB side does not exist in this repo. My branch routes it to the packing layout, which lays the board out correctly. I put that in the PR body as a limitation rather than presenting it as a match-adapt implementation. A maintainer might want the real algorithm instead. Either way the current behavior, silent overlap on a documented prop, is not defensible. The fallback is at least honest about being a fallback.

Green before submit, on their gates not mine. CI runs format-check, a smoke test and ten test shards. All green on the PR.

What building a board taught me that reading code did not

This is the part I would actually tell someone.

I had spent a stretch scanning this codebase for bugs by reading it. I filed several. Two maintainers pushed back on the batch. The substance of it was: these are nitpicks in the wrong places, they are not user issues, go build a board.

That was fair and it was useful. So I built boards. An LED matrix, a Bluetooth speaker front end. Within one afternoon of actually using the library the way a user does, I hit a documented prop that silently destroys the layout, worth more than everything the reading pass produced. Reading finds code that looks wrong. Using finds behavior that is wrong.

The bug was in a file I had already read. I did not see it, because a dispatch handling three of four cases reads like a dispatch. It only becomes a bug when you type the fourth case into a real board and watch the components pile up.


Written with AI assistance (Claude, Anthropic). The board that surfaced the bug, the diagnosis, the fix and the tests are mine, verified before submitting: the repo's own test suite plus format checks green locally and in CI across ten shards, the new assertions confirmed failing against unpatched code, plus the issue checked live for an existing fix PR before opening mine.

Top comments (0)