fix(components): flex declares the containment it renders - #6762
Merged
Conversation
`flex` has always rendered `schema.children`, but its registration omitted `isContainer` while `grid`, `card`, `container` and `stack` all declare it. The render path never reads the flag, so nothing was broken at runtime; its consumers are elsewhere, and the gap made them contradict the renderer. Measured through the mechanism rather than the property: the manifest built the way the app builds it, with a `flex` node carrying children put through `validateTree`, returned ["not-a-container"] while grid/card/container under the identical probe returned [] — the control that makes the reading real. Pinned over the family, not over `flex` alone: the defect's shape was "three declare it and one does not", so a pin covering only the missing one would let the next registration rot the same way. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CRJge11jso9TpXRWFt1Z49
Contributor
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
os-sales
marked this pull request as ready for review
August 29, 2026 08:01
This was referenced Aug 29, 2026
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
2 participants
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 #6740
flexhas always renderedschema.children, but its registration omittedisContainerwhilegrid,card,containerandstack— same directory, sameuinamespace — all declare it. The render path never reads the flag, so nothing was broken at runtime; the flag's consumers are elsewhere, and the gap made them contradict the renderer.Reproduced first, with the card's own control
Base
b76ca6764, the card's re-check verbatim:That zero is a reading only because the positive control returns non-zero in the same corpus and under the same command shape.
The defect measured through the mechanism, inside this repo
@object-ui/sdui-parseris already a dependency of@object-ui/components, so the downstream path is constructible here — no NOT MEASURED fallback was needed. The manifest was built exactly the way the app builds it (getKnownTypes()+getMeta()fed tomanifestFromConfigs, mirroringrenderers/layout/page.tsx:462) and a node carrying children put throughvalidateTree.On base
b76ca6764:flexwith children["not-a-container"]gridwith children[]cardwith children[]containerwith children[]With this change,
flexreturns[]and the three controls are unchanged. This is the same warning objectstack measured downstream: its three shippedexamples/app-showcasehtml pages drew 232 diagnostics, and every one of the 32 warnings wasnot-a-containeronflex, withflextheir only source. The 200 errors are a separate matter (objectstack#12924 owns it) and are untouched here.The form is copied from the siblings, not invented
grid,card,containerandstackall placeisContainer: truein the registration's third argument, afterdefaultProps. This change matches that placement and value.Where they disagree, and the call made. All four
ui-namespace siblings pairisContainerwithresizable: trueand aresizeConstraintsblock. Thepage:*containers incontainers.tsxdeclareisContainer: trueon its own, with neither. So the flag is independent of designer resize affordances, and onlyisContaineris added here — minting a resize affordance forflexwould be a behaviour change this card did not measure.Reviewer attention: a second consumer, beyond the diagnostic
The card asked that anything altering behaviour past
not-a-containerbe surfaced rather than absorbed. One thing does, and it is measured rather than reasoned about.renderers/layout/react-page.tsx:77builds the JSX scope of everykind:'react'page withif (!tag || cfg.isContainer) continue;, reading the predicate offgetPublicConfigs(). Whileflexomitted the flag it was the one layout primitive of the five still injected there.Measured by rendering real
kind:'react'page source throughSchemaRenderer:b76ca6764FlexGridCardContainerStackButton,Badge,Text,ImageThe leaf-block row is the direction control: without it, "containers are dropped" would be indistinguishable from "everything is dropped".
Two things make this the correcting direction rather than a regression, but a reviewer should still rule on it:
content/docs/guide/react-pages.mdhas documented this behaviour all along and names this component first — "Layout containers are deliberately not injected. The scope builder skips every container ... soflex,grid,cardand friends have no injected wrapper." The prose predates the 2026-08-21 rewrite that added the code citation; onlyflexdid not obey it.({ children: _children, ...props })— it discards children. Measured on base: a react page writing this component around a child rendereddiv data-obj-type="flex" class="flex flex-row ..."with the child gone. After the change the author gets the page-level error panel naming the identifier, and that doc page's stated remedy is real HTML with an inlinestyleobject.So the change moves a react page from silently swallowing content to failing loudly with a documented fix. It is still a user-visible change on a published package, which is why it is called out here rather than left in the diff.
The pin covers the family, not just the one that was missing
The defect's shape is "three declare it and one does not". A pin covering only
flexwould let the next registration rot the same way and stay green while it did, sopackages/components/src/__tests__/layout-containers-declare-containment.test.tsxasserts over all four, throughvalidateTreerather than against the literal property — the property is only interesting because a mechanism reads it.It carries three controls, each closing a way the pin could pass while measuring nothing:
unknown-componentin the result, and the probe node really has children (the containment branch is guarded bynode.children?.length);badge, a genuine leaf, must still drawnot-a-container, so deleting the check or defaulting the flag on turns the file red;A fourth test renders
flexthrough the realSchemaRendererand asserts its children still arrive, which is the scope guard for "nothing else changes".Ablation
Against the committed implementation
e44f941ba, with the restore trapped onEXIT INT TERMusing absolute paths.9cd1720a0...->f5994b334...(a differing hash, not an exit code).flexwith children draws nonot-a-container" (expected [ { severity: 'warning', ... } ] to deeply equal []) and "flexreports as a container on the public tier" (expected undefined to be true). The three sibling cases stayed green, since they legitimately declare the flag.git checkout HEAD --against the absolute path (namingHEAD, so the mutated index cannot be the source), thengit diff HEADempty and restored blob9cd1720a0...equal to that path's HEAD blob hash.No rebuild step is involved and none is owed:
vitest.config.mtsaliases every@object-ui/*specifier topackages/*/src, and the pin reachesflex.tsxby relative import, so the mutated source is what ran. Nothing resolved throughdist.Gates
All run on
e44f941bawith a clean tree, every exit code captured by redirect-then-capture and each result quoted from the tool's own verdict line.pnpm exec vitest run packages/components/ packages/core/src/registry/ examples/schema-catalog/Test Files 228 passed (228)/Tests 3875 passed (3875), exit 0getPublicConfigs/PUBLIC_BLOCKS)Test Files 5 passed (5)/Tests 152 passed (152), exit 0pnpm --filter @object-ui/components type-checknode scripts/check-changeset-presence.mjseslint .inpackages/components(this package's ownlintgate, which is whatturbo run lintinvokes for it)The type-check number is qualified rather than quoted bare:
packages/components/tsconfig.jsonexcludessrc/__tests__, sotsc --noEmitalone says nothing about the new pin. The chainedtsc -p tsconfig.test.jsonis what covers it, and--listFileswas used to confirm the file is actually in that program rather than assumed to be.The repo-wide
pnpm lintisturbo run lint, which fans out to each package's ownlint. Only this package's run was taken, and that is a measurement rather than a skip, on three pieces of evidence: the population is eslint's own (422 files, counted from--format json, not from a guess about what counts); both changed files are inside that population and were linted (the test file clean,flex.tsxcarrying one pre-existingno-explicit-anywarning at line 15 column 94 — the original registration signature, far from the line 146 edit); andeslint.config.jsconfigures noprojectServiceand noparserOptions.project, so type-aware linting is off and each file's verdict is a function of its own source alone — this diff cannot move a verdict in a package it does not touch.CI owns the full farm and runs it regardless.
Generated by Claude Code