Uh oh!
There was an error while loading. Please reload this page.
Author the nine catalog space-x-* flex nodes as gap, and ratchet the tenth out - #5826
Merged
Merged
Conversation
…tchet the tenth out Nine `flex` nodes across five catalog files spelled horizontal spacing as `space-x-N` — the one token #4891's sweep excluded by design, which is why that card's headline figure re-measured to the digit. Triage ruled the nine drift, not intent, with the ruling made checkable: a node that wants "gutters between siblings but no gutter above a wrapped row" is asking for `space-x`, and converting that node would be a regression. All nine were checked, per node, on both divergence axes; none is the intent case. Neither the node as shipped nor as converted renders any `flex-wrap` token, so each is a `nowrap` container — single-line at every viewport width by CSS definition, which subsumes any finite sample of widths — and each child list is a static pair with no conditional sibling, so `space-x`'s `:not([hidden]) ~ :not([hidden])` selector spans the same one gutter `gap` does. The check is a test, not a PR sentence. The measurement also turned up what nothing before it had noticed: none of the nine declared `gap`, so `flex.tsx`'s `schema.gap ?? 2` was rendering its ladder on all nine, additively under the hand-written margin — `tailwind-merge` does not collapse `gap-*` against `space-x-*`, they are different CSS properties. The shipped gutter on a `space-x-2` node was gap 0.375rem PLUS margin 0.5rem. Converting to `gap: N` drops the accidental half and tops the ladder out at exactly the value the author wrote. `ownPropTokens()` gains the `space-x-*` arm so a tenth cannot land. It takes `direction`, defaulted per renderer: on a column-major node `space-x` indents the children rather than spacing them and `gap` cannot express it, so flagging it would demand a conversion that changes what renders. Part of #5690 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EuPCi56cnGyykygi3z9w4m
os-zhuang
marked this pull request as ready for review
August 23, 2026 13:52
Uh oh!
There was an error while loading. Please reload this page.
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 freeto 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.
Fixes#5690
Nine
flexnodes across five catalog files spelled horizontal spacing asspace-x-N— the one token the #4891 sweep excluded by design, which is exactly why that card's headline figure re-measured to the digit. Triage ruled them drift, not intent, and ruled the ruling checkable. Both arms of the card are implemented here: the nine are converted, andownPropTokens()gains the arm that keeps the tenth out.The part that was not mechanical
space-x-Nandgap: Nare not two spellings of one thing.space-x-Ncompiles to a margin on& > :not([hidden]) ~ :not([hidden]);gapis the flexbox gap property. They diverge on two axes — a wrapped row gets nospace-xgutter above it, and the selector skips a hidden sibling in a waygapdoes not. So no byte-equivalence claim is made for these nine, and none could be.Every one of the nine was checked on both axes, per node. Zero are the intent case; nine converted, none left behind. The check is
examples/schema-catalog/test/layout-props-conversion.test.tsx, last describe — not a sentence in this PR.How the wrap question is decided, and why not by sampling pixel layout: a flex container wraps if and only if its computed
flex-wrapiswraporwrap-reverse(CSS Flexible Box §5.1 — anowrapcontainer is single-line; its children shrink or overflow, they never move to a second row). That is a property of the container, not of the viewport.flex.tsxemitsflex-wraponly forwrap: true, and Tailwind's.flexsets none of its own, so a rendered class list with no flex-wrap token leaves the CSS initial value standing. Reading that token off the real renderer, on both the shipped and the converted side of each node, answers the question at every width at once — which strictly subsumes any finite sample of widths, and is the only honest instrument available in this suite anyway: happy-dom has no layout engine, sooffsetTopis0for everything and "did this row wrap" cannot be observed by measuring boxes.Per node, all nine: no
wrap: true, noflex-wraptoken at any breakpoint on the node or any ancestor, exactly two children, and nohidden/visibleexpression on either child. They are an auth checkbox and its label (three sites), an activity avatar and its name block (five sites), and a cancel/save button pair (one site).What the measurement turned up
Nothing before this had noticed that the nine were rendering both spacings, additively. None of them declared
gap, soflex.tsx'sschema.gap ?? 2was emittinggap-1.5 sm:gap-2on all nine, andtailwind-mergedoes not collapse that againstspace-x-N— they are different CSS properties. Measured through the real renderer:So the shipped gutter on a
space-x-2node was gap0.375remplus margin0.5rem, not0.5rem. The conversion togap: Ndrops the accidental half and tops the ladder out at exactly the value the author wrote (gap: 2→gap-1.5 sm:gap-2,0.5rematsmand above;gap: 4→gap-2 sm:gap-3 md:gap-4,1rematmdand above). That is a real rendering change at every width, which is why these areladdercases and notidenticalones. The card's premise — "space-x-4is one dead value at every width" — was half the picture; the true pre-state was a live ladder plus a dead value.The ratchet arm
ownPropTokens()takesdirectionnow, defaulted per renderer (flex.tsx→row,stack.tsx→col). On a column-major nodespace-xindents the children rather than spacing them, andgapcannot express that, so flagging it would demand a conversion that changes what renders. The arm is gated on the type's gap ladder for the same reason thegap-Narm is: a step the renderer does not map emits no gap class, so extracting it would delete the spacing rather than move it.The docblock line that listed
space-x-2as a token "describing this node AS A FLEX ITEM rather than as a container" is corrected —flex-shrink-0is item-level,space-x-Nstyles the node's own children and is container-level. Nine nodes sat behind that sentence for two sweeps.space-y-*on astackre-measured 0 nodes on this branch. The sweep was not widened looking for it.One pin moved, and why it is a re-measurement rather than a silencer
sees enough layout nodes WITH a classNamehad a floor of180. The nine spelledspace-x-Nand nothing else, so extracting it emptied theirclassNametoo, and the census goes187 → 178— the same way #4891 took it252 → 187when 65 nodes had nothing left to say. The floor is re-stated at175, with the arithmetic in the comment, and it is deliberately tighter than the 7 of headroom it replaces. Leg C below is the measurement that this pin was moved by the ruled sweep and not chosen: it is the only thing that fails when the pre-change test file is run against the converted catalog.Verification
All on the final commit
67f81918f, tree clean, run from the repo root (neverpnpm --filterfor vitest).pnpm exec vitest run examples/schema-catalog/ --maxWorkers=2Test Files 9 passed (9)/Tests 1611 passed (1611)pnpm --workspace-concurrency=2 --filter '@object-ui/example-schema-catalog^...' buildBUILD_EXIT=0(run before any type judgement)pnpm --filter @object-ui/example-schema-catalog type-checktsc --noEmit && tsc -p tsconfig.test.json,TYPECHECK_EXIT=0node scripts/check-control-bytes.mjs✅ check-control-bytes: OK (scanned 4843 tracked text file(s); skipped 85 binary).node scripts/check-changeset-presence.mjs✅ No source of a released package changed in this range, so no changeset is owed.pnpm --filter @object-ui/example-schema-catalog regenerate:checkexamples/schema-catalog/src/index.ts is up to date (423 entries).eslint --no-inline-configon the changed filesDeclared narrowing — lint. Repo-wide
pnpm lintis CI's run; the local one is targeted, and here is the measurement that the narrowing excludes nothing. (1) The population comes from eslint's own config, not from a guess: the five changed.jsonfiles are reported by eslint itself asFile ignored because no matching configuration was supplied, because the config's only block isfiles: ['**/*.{ts,tsx}']. (2) The count comes from--format json: 6 entries returned, 5 of them ignored, so 1 file was actually linted —layout-props-conversion.test.tsx, 0 errors, 0 warnings. (3) Config invariance for untouched files:eslint.config.jsextendstseslint.configs.recommended, notrecommendedTypeChecked, and declares noparserOptions.project/projectService— type-aware linting is off, so no rule reads across files and this diff cannot move the verdict on any file it does not contain.Reverse verification, on a real commit
Each leg is a mutation of the committed tree, confirmed on disk before the run (substitution counts and token censuses, never an editor's exit code), with a restore trap;
git diff HEAD --statis empty after each.flex/stack/container的节点,仍在 className 里手写自己已声明的 props #4891 ratchet names all nine by path (auth/login-simple :: flex :: space-x-2 (in "space-x-2"), …), the three newladdercases cannot find their converted node, and the site test reportsclassName survived. This is the "a tenth cannot land" proof against real nodes.space-xarm fromownPropTokens(), converted tree. RED, exactly 1 failure:the ratchet arm refuses a tenth space-x node. Nothing else moves — and that is the point. Once the catalog holds zerospace-xnodes, the catalog-wide offenders test cannot discriminate the arm at all; only the unit test can, which is why it exists.sees enough layout nodes WITH a className for the ratchet to mean something. The pin move was forced by the sweep, not chosen. (28 tests then, 35 now.)isSpacingTokenback togap|p. RED, exactly 3 failures: the three newladdercases. The nine pre-finding: 9 catalogflexnodes still hand-write spacing asspace-x-*, the one spelling #4891's sweep deliberately left behind #5690 cases stay green, which is the other half of the claim — widening the spacing group to includespace-[xy]is load-bearing for the new cases and inert for the old ones.Legs that do NOT discriminate, stated rather than omitted. Reverting the JSON and the test file together is just
HEAD~1: all green, no signal. And the wrap check itself —not one of the nine could wrap— stayed green in legs A, B and D. It is an observation pinned as an assertion; its discriminating power is against a future edit that gives one of these nodeswrap: true, aflex-wrapclass, or a conditional child, not against this diff. No leg here can turn it red, and claiming otherwise would be the easiest false sentence in this PR.Changeset
None owed and none added.
check-changeset-presence.mjsreports0 of them under the src/ of a package the release covers—@object-ui/example-schema-catalogisprivate: true, so changesets does not version it, and the other changed file is a test.Generated by Claude Code