From e44f941ba1bcd79ba1180c8fb6df609e11c654bb Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 29 Aug 2026 07:08:55 +0000 Subject: [PATCH] fix(components): flex declares the containment it renders MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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 Claude-Session: https://claude.ai/code/session_01CRJge11jso9TpXRWFt1Z49 --- .changeset/6740-flex-is-container.md | 40 +++++ ...ut-containers-declare-containment.test.tsx | 160 ++++++++++++++++++ .../components/src/renderers/layout/flex.tsx | 23 ++- 3 files changed, 222 insertions(+), 1 deletion(-) create mode 100644 .changeset/6740-flex-is-container.md create mode 100644 packages/components/src/__tests__/layout-containers-declare-containment.test.tsx diff --git a/.changeset/6740-flex-is-container.md b/.changeset/6740-flex-is-container.md new file mode 100644 index 0000000000..251aae955c --- /dev/null +++ b/.changeset/6740-flex-is-container.md @@ -0,0 +1,40 @@ +--- +'@object-ui/components': patch +--- + +`flex` declares the containment it renders (objectui#6740). + +`flex` has always rendered `schema.children`, but its registration omitted +`isContainer` while `grid`, `card`, `container` and `stack` — same directory, +same `ui` namespace — all declared 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, not inferred from the property. Building the +manifest the way the app builds it (`getKnownTypes()` + `getMeta()` -> +`manifestFromConfigs`) and putting a `flex` node carrying children through +`validateTree` returned `["not-a-container"]`, while `grid` / `card` / +`container` under the identical probe returned `[]` — the control that makes +that reading real. Downstream, objectstack's three shipped +`examples/app-showcase` html pages drew 232 diagnostics, of which every one of +the 32 warnings was `not-a-container` on `flex`, and `flex` was their only +source. `validateTree` is not on objectstack's production gate path today, so +the warnings are currently unobserved — which is why this is worth closing +before that gate goes live rather than after. + +**Second consumer, and the reason this is not purely a declaration change.** +`renderers/layout/react-page.tsx` builds the JSX scope of every `kind:'react'` +page with `if (!tag || cfg.isContainer) continue;`. While `flex` omitted the +flag it was the one layout primitive of the five still injected there, so +`` resolved in react page source — and rendered an EMPTY div, because the +injected wrapper drops `children`. It now behaves like its four siblings and is +not injected, which is what `content/docs/guide/react-pages.md` has documented +all along ("Layout containers are deliberately not injected ... ``, +``, `` and friends have no injected wrapper"). A react page that +wrote `` moves from silently swallowing its children to the page-level +error panel naming the identifier, with that page's documented remedy being +real HTML: `
`. + +Pinned over the family rather than over `flex` alone: the defect's shape was +"three declare it and one does not", and a pin covering only the one that was +missing would let the next registration rot the same way. diff --git a/packages/components/src/__tests__/layout-containers-declare-containment.test.tsx b/packages/components/src/__tests__/layout-containers-declare-containment.test.tsx new file mode 100644 index 0000000000..784458c0a6 --- /dev/null +++ b/packages/components/src/__tests__/layout-containers-declare-containment.test.tsx @@ -0,0 +1,160 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * The `ui` layout primitives DECLARE the containment they render (objectui#6740). + * + * `flex` renders `schema.children` and always has, but its registration omitted + * `isContainer` while `grid`, `card`, `container` and `stack` — same directory, + * same namespace — all declared it. The render path never reads the flag, so + * nothing was broken at runtime; the consumers are elsewhere, and the gap made + * them contradict the renderer. + * + * MEASURED on b76ca6764, 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 `[]`. Downstream, + * objectstack's three shipped `examples/app-showcase` html pages drew 232 + * diagnostics of which 32 were warnings — every one of them `not-a-container` on + * `flex`, and `flex` the only source of them. + * + * WHY THIS FILE PINS FOUR COMPONENTS AND NOT ONE. The defect's shape is "three + * declare it and one does not". A pin covering only `flex` would let the next + * registration rot exactly the same way and stay green while it did, so the + * assertion is over the family. `stack` rides along for the same reason. + * + * WHY THROUGH `validateTree` AND NOT `getConfig(t).isContainer`. The property is + * only interesting because a mechanism reads it; asserting the literal would + * stay green if the containment check were removed, mis-keyed, or fed a manifest + * the tag is missing from — the three ways this fact dies without anyone + * noticing. The reachability controls below exist for the same reason. + */ + +import { describe, it, expect } from 'vitest'; +import { render, waitFor } from '@testing-library/react'; +import { ComponentRegistry } from '@object-ui/core'; +import { SchemaRenderer, AdapterCtx } from '@object-ui/react'; +import { manifestFromConfigs, validateTree } from '@object-ui/sdui-parser'; +import type { Diagnostic, SchemaElement } from '@object-ui/sdui-parser'; + +// Module scope, not a hook: this import IS the registration (AGENTS.md +// §测试纪律 — an unbounded module load must not be billed to a bounded window). +import '../renderers'; + +const CONTAINMENT = 'not-a-container'; + +/** The four `ui` layout registrations whose whole job is to hold children. */ +const LAYOUT_CONTAINERS = ['flex', 'grid', 'card', 'container'] as const; + +/** + * The manifest the running app validates against, built the way the app builds + * it — keyed by every KNOWN registry tag rather than by `getAllConfigs()`, whose + * `.type` is always the namespaced form. Mirrors `getJsxManifest()` in + * `renderers/layout/page.tsx:462` (module-private, hence the four lines here). + * Key it off `getAllConfigs()` instead and the bare `flex` tag authors write is + * absent from the manifest, so every assertion below would pass on + * `unknown-component` without ever reaching the containment check. + */ +const diagnose = (schema: unknown): Diagnostic[] => { + const configs = ComponentRegistry.getKnownTypes().map((t) => { + const meta = ComponentRegistry.getMeta(t); + return { type: t, namespace: meta?.namespace, isContainer: meta?.isContainer, inputs: meta?.inputs }; + }); + const manifest = manifestFromConfigs(configs as unknown as Parameters[0]); + return validateTree(schema as SchemaElement, manifest).diagnostics; +}; + +const withChildren = (type: string) => ({ + type, + children: [{ type: 'text', content: 'child' }], +}); + +describe('the ui layout primitives accept children without warning (objectui#6740)', () => { + it.each(LAYOUT_CONTAINERS)('`%s` with children draws no `not-a-container`', (type) => { + const diagnostics = diagnose(withChildren(type)); + + // Reachability BEFORE absence — an empty result proves nothing if the + // containment branch never ran. Two ways it silently would not: an + // unresolved tag reports `unknown-component` and never reaches the check, + // and the branch is guarded by `node.children?.length`. + expect(diagnostics.filter((d) => d.code === 'unknown-component')).toEqual([]); + expect(withChildren(type).children.length).toBeGreaterThan(0); + + // Filtered by code, not against an empty list: this file owns the + // CONTAINMENT fact only, so a future diagnostic on some other key belongs + // to that key's pin rather than here. + expect(diagnostics.filter((d) => d.code === CONTAINMENT)).toEqual([]); + }); + + it('still reports `not-a-container` for a component that genuinely takes none', () => { + // The control that keeps every assertion above meaningful. Without it the + // suite would stay green if the containment check were deleted outright, or + // if `isContainer` were defaulted on — either of which turns this file into + // a measurement of nothing. `badge` is a leaf: it renders its own label and + // never reads `schema.children`, so children under it ARE an authoring + // mistake the author must still hear about. + // + // If `badge` ever legitimately becomes a container, this goes red — move the + // control to another childless registration rather than deleting it. + const codes = diagnose(withChildren('badge')).map((d) => d.code); + expect(codes).toContain(CONTAINMENT); + }); +}); + +describe('the declaration reaches the consumers that read it (objectui#6740)', () => { + it.each(LAYOUT_CONTAINERS)('`%s` reports as a container on the public tier', (type) => { + // The second consumer, and the reason it is pinned rather than left + // implicit: `renderers/layout/react-page.tsx:77` builds the JSX scope of + // every `kind:'react'` page with `if (!tag || cfg.isContainer) continue;`, + // reading THIS predicate off `getPublicConfigs()` — not off `getMeta()`. + // While `flex` omitted the flag it was the one layout primitive of the five + // still injected there, contradicting + // `content/docs/guide/react-pages.md` ("Layout containers are deliberately + // not injected … ``, ``, `` and friends have no injected + // wrapper"). Declaring it lines `flex` up with its four siblings. + const cfg = ComponentRegistry.getPublicConfigs().find((c: { type: string }) => c.type === type); + expect(cfg, `\`${type}\` is not in the public tier`).toBeTruthy(); + expect((cfg as { isContainer?: boolean }).isContainer).toBe(true); + }); + + it('leaf blocks stay injectable into a react page — containers are the only ones dropped', () => { + // The direction control for the assertion above. If this were empty or all + // falsy, "containers are excluded" would be indistinguishable from + // "everything is excluded", and the pin would be measuring the wrong fact. + for (const leaf of ['badge', 'button', 'image', 'icon']) { + const cfg = ComponentRegistry.getPublicConfigs().find((c: { type: string }) => c.type === leaf); + expect(cfg, `\`${leaf}\` is not in the public tier`).toBeTruthy(); + expect((cfg as { isContainer?: boolean }).isContainer).toBeFalsy(); + } + }); +}); + +describe('flex still renders its children (objectui#6740)', () => { + it('renders children through the real SchemaRenderer, as it always did', async () => { + // The scope guard for acceptance "nothing else about flex changes". The + // render path does not consult `isContainer` — `SchemaRenderer` passes the + // whole node as `schema` and `flex` re-reads `schema.children` itself — + // so declaring the flag must leave this untouched. + const { container } = render( + + + , + ); + + await waitFor(() => expect(container.querySelector('[data-obj-type="flex"]')).toBeTruthy()); + const root = container.querySelector('[data-obj-type="flex"]'); + expect(root?.className).toContain('flex-col'); + expect(container.textContent).toContain('kept'); + }); +}); diff --git a/packages/components/src/renderers/layout/flex.tsx b/packages/components/src/renderers/layout/flex.tsx index 82c320a56f..9cd1720a03 100644 --- a/packages/components/src/renderers/layout/flex.tsx +++ b/packages/components/src/renderers/layout/flex.tsx @@ -146,6 +146,27 @@ ComponentRegistry.register('flex', { type: 'button', label: 'Button 2' }, { type: 'button', label: 'Button 3' } ] - } + }, + // `flex` renders `schema.children` (see `renderChildren` above) but did not + // DECLARE that it does, while `grid`, `card`, `container` and `stack` — the + // same directory, the same `ui` namespace — all do. The flag is not read by + // the render path, so nothing was broken at runtime; its consumers are + // elsewhere, and the gap made them contradict the renderer (objectui#6740). + // + // MEASURED, not inferred. Building the manifest the way the app builds it + // (`getKnownTypes()` + `getMeta()` -> `manifestFromConfigs`) and putting a + // `flex` node WITH children through `validateTree` returned + // `["not-a-container"]`, while `grid` / `card` / `container` under the same + // probe returned `[]` — the control that makes the reading real. Downstream, + // objectstack's three shipped `examples/app-showcase` html pages drew 32 + // `not-a-container` warnings, every one of them on `flex` and `flex` the + // only source of them. + // + // `isContainer` alone, deliberately. The four `ui`-namespace siblings pair + // it with `resizable` / `resizeConstraints`, but the `page:*` containers in + // `containers.tsx` declare `isContainer: true` on its own — so the flag is + // independent of designer resize affordances, and minting one here would be + // a behaviour change this card did not measure. + isContainer: true } );