diff --git a/.changeset/one-schemanode-one-label-vocabulary-4580.md b/.changeset/one-schemanode-one-label-vocabulary-4580.md new file mode 100644 index 0000000000..9f05357152 --- /dev/null +++ b/.changeset/one-schemanode-one-label-vocabulary-4580.md @@ -0,0 +1,21 @@ +--- +'@object-ui/types': minor +'@object-ui/core': minor +'@object-ui/react': minor +'@object-ui/components': minor +'@object-ui/plugin-dashboard': minor +--- + +One `SchemaNode`, and one label vocabulary — the union wins, and labels resolve where the locale lives + +Two packages published a type called `SchemaNode` and they were not the same type. `@object-ui/core` hand-declared `interface SchemaNode { type: string; … [key: string]: any }`; `@object-ui/types` exported `type SchemaNode = BaseSchema | string | number | boolean | null | undefined`, whose own doc comment names `'Plain string'` a valid node. Both were exported under one name from packages the same consumers import together, so which declaration a call site got depended on which package it happened to import from — #4548's canary measured 19 of 35 errors as exactly that collision. Core's declaration is now a re-export of types', so there is one declaration left to disagree with. Core's entry surface is unchanged: `dist/index.d.ts` is byte-identical across the change. + +Reconciling it exposed a real defect rather than a mechanical narrowing, which is why the first attempt was withdrawn instead of forced. The spec bridges write `spec.label` — the spec's `I18nLabel`, an INLINE locale map like `{ en: 'Owner', 'zh-CN': '负责人' }` — into `node.label`, and `BaseSchema.label` declared `string`. Under core's old index signature that assignment was invisibly `any`; under one honest `SchemaNode` it is a type error. `BaseSchema.label` and `.description` therefore now accept `string | I18nLabel`, and the two bridge assignments compile with their expressions untouched. + +Resolution happens at READ time, in the renderer, against the display locale — not at the bridge. Resolving at the bridge was measured unimplementable: it is a plain class method that cannot call a hook, `BridgeContext` declares no locale, and `updateContext()` has zero callers, so a bridge-resolved label would freeze one audience's language into the node tree with no re-translation channel. React's own invalidation re-translates for free at the read site. + +The widening turned every blind `schema.label`-as-string read into a named compiler error, and that inventory is the audit: it named four sites repo-wide, all one class — the label reaching a React child position, where a map does not render as `[object Object]` but THROWS `Objects are not valid as a React child`, failing the whole subtree. Three are `@object-ui/components` renderers (`filter-builder`, `sidebar-group`, `dropdown-menu`), which now resolve with the spec's own `resolveI18nLabel` against `useDisplayLocale()`. The fourth is `plugin-dashboard`'s `DashboardGridLayout` heading, which resolves with `pickLocalized` against the active UI language — matching the widget-title resolution already in that same component rather than putting two resolvers and two disagreeing locale channels in one render; the two resolvers are limb-for-limb twins with a parity test pinning them. + +One interface now carries both label vocabularies two properties apart — `label`/`description` are the spec's INLINE map, `ariaLabel` is the KEYED bundle reference — and each accepts the other's shape vacuously. That confusability is objectui#4167's known hazard, inherent to the spec's `I18nLabel` design; both shapes are named with cross-referenced doc comments stating which resolver owns which slot, and a pin asserts the two unions do not collapse into each other. + +Finally, the spec bridges declare their return type as `BaseSchema` instead of the union. Both bridges end in a single `return node` on an object literal, so the union described nothing real while forcing a narrowing at every read — 272 mechanical errors across five suites in the first round. That change is a type annotation only; the emitted JavaScript is byte-identical. diff --git a/packages/components/src/__tests__/inline-locale-label-read-sites.test.tsx b/packages/components/src/__tests__/inline-locale-label-read-sites.test.tsx new file mode 100644 index 0000000000..62358a9f50 --- /dev/null +++ b/packages/components/src/__tests__/inline-locale-label-read-sites.test.tsx @@ -0,0 +1,185 @@ +/** + * 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. + */ + +/** + * Renderers that read `schema.label` resolve the INLINE locale map (objectui#4580). + * + * `BaseSchema.label` and `.description` accept `string | I18nLabel` since + * #4580's revised Q1 ruling (comment 5284973826) — `I18nLabel` being the spec's + * INLINE locale MAP (`string | Record` in `@objectstack/spec` + * 17.0.0-rc.6), the shape a spec producer already writes: `bridgeListView` + * assigns `node.label = spec.label` at `list-view.ts:180`. + * + * Resolution happens at READ time, here, against the display locale — NOT at + * the spec bridge. PR #4603 measured the bridge unable to do it: it is a plain + * class method that cannot call a hook, `BridgeContext` declares no locale, and + * `updateContext()` has zero callers, so a bridge-resolved label would freeze + * one audience's language into the node tree (the spec's own resolver doc + * records that defect class as objectstack#6761). + * + * ## The site class, and why these three are ONE case + * + * The widening turned every blind `schema.label`-as-string read into a named + * TS2322 — that compiler inventory IS the audit, and it named exactly four + * sites repo-wide, all of them the SAME class: the label reaching a React child + * position. Three are in this package; the fourth is + * `plugin-dashboard/src/DashboardGridLayout.tsx` and is pinned in that package. + * + * ## Red-first — measured BEFORE the fix, verbatim + * + * Each of the three renderers below was invoked with + * `label: { en: 'Owner', 'zh-CN': '负责人' }` against the unfixed source. All + * three THREW, with byte-identical messages: + * + * ``` + * Objects are not valid as a React child (found: object with keys {en, zh-CN}). + * If you meant to render a collection of children, use an array instead. + * ``` + * + * Not `[object Object]` — a throw. A text node is one of the positions React + * refuses outright rather than stringifying, so the pre-fix harm is the whole + * subtree failing to render, not a cosmetic mis-render. + * + * ⚠️ **`dropdown-menu`'s harm is invisible unless the menu is OPEN.** Radix + * mounts `DropdownMenuContent` lazily, so the first probe of that renderer + * returned an EMPTY container and no throw — a case that would have shipped + * looking green while proving nothing. `defaultOpen: true` is what makes the + * label reachable, and it is load-bearing in the case below for that reason. + * + * ## Why these invoke the registered renderer DIRECTLY + * + * Same reason PR #4603's toggle case does: `SchemaRenderer` injects its own + * props around a renderer, and a test driven through it can be green in both + * directions. `ComponentRegistry.get(name)` returns the component the registry + * actually renders (`React.createElement`, `SchemaRenderer.tsx:621`) — which is + * also why calling `useDisplayLocale()` inside these renderers is legal. + */ + +import { describe, it, expect } from 'vitest'; +import { render, screen } from '@testing-library/react'; +import { I18nProvider, LocalizationProvider } from '@object-ui/i18n'; +import { ComponentRegistry } from '@object-ui/core'; +// Registers the renderers at module scope, NOT inside a `beforeAll` — there the +// cold transform is billed to `hookTimeout`. See +// object-ui/no-dynamic-import-in-test-hook (objectui#3010/#3021). +import '../renderers'; + +/** The inline locale map an author writes; `zh-CN` exists so a switch is observable. */ +const INLINE_MAP = { en: 'Owner', 'zh-CN': '负责人' } as const; + +/** + * Render a registered renderer directly, with the display locale pinned. + * + * `useDisplayLocale()` is `tenantLocale || uiLanguage || 'en'`, so driving + * `LocalizationProvider` sets it deterministically rather than depending on the + * ambient react-i18next instance. + */ +function renderDirect( + name: string, + schema: Record, + tenantLocale?: string, +) { + const C = ComponentRegistry.get(name) as React.ComponentType; + return render( + + + + + , + ); +} + +describe('label read sites resolve the inline locale map (objectui#4580)', () => { + /* ── filter-builder ──────────────────────────────────────────────────── */ + + describe('filter-builder', () => { + it('resolves the map for the display locale', () => { + renderDirect('filter-builder', { type: 'filter-builder', label: INLINE_MAP, fields: [] }, 'zh-CN'); + expect(screen.getByText('负责人')).toBeInTheDocument(); + }); + + it('resolves the same map differently for a different locale', () => { + renderDirect('filter-builder', { type: 'filter-builder', label: INLINE_MAP, fields: [] }, 'en'); + expect(screen.getByText('Owner')).toBeInTheDocument(); + }); + + it('passes a plain string through unchanged', () => { + renderDirect('filter-builder', { type: 'filter-builder', label: 'Filters', fields: [] }, 'zh-CN'); + expect(screen.getByText('Filters')).toBeInTheDocument(); + }); + }); + + /* ── sidebar-group ───────────────────────────────────────────────────── */ + + describe('sidebar-group', () => { + it('resolves the map for the display locale', () => { + renderDirect('sidebar-group', { type: 'sidebar-group', label: INLINE_MAP }, 'zh-CN'); + expect(screen.getByText('负责人')).toBeInTheDocument(); + }); + + it('passes a plain string through unchanged', () => { + renderDirect('sidebar-group', { type: 'sidebar-group', label: 'Reports' }, 'zh-CN'); + expect(screen.getByText('Reports')).toBeInTheDocument(); + }); + }); + + /* ── dropdown-menu ───────────────────────────────────────────────────── */ + + describe('dropdown-menu', () => { + // `defaultOpen` is load-bearing: Radix mounts the content lazily, so without + // it this case renders an empty container and proves nothing (measured). + it('resolves the map for the display locale', () => { + renderDirect( + 'dropdown-menu', + { type: 'dropdown-menu', label: INLINE_MAP, items: [], defaultOpen: true }, + 'zh-CN', + ); + expect(screen.getByText('负责人')).toBeInTheDocument(); + }); + + it('passes a plain string through unchanged', () => { + renderDirect( + 'dropdown-menu', + { type: 'dropdown-menu', label: 'Actions', items: [], defaultOpen: true }, + 'zh-CN', + ); + expect(screen.getByText('Actions')).toBeInTheDocument(); + }); + }); + + /* ── The resolver's documented fallback ──────────────────────────────── */ + + /** + * The spec's `resolveI18nLabel` documents its fallback order as exact match → + * base/region (`zh-CN` ↔ `zh`) → last resort (any remaining entry), returning + * `undefined` when nothing matched. These pin the two limbs past "exact", + * because a resolver that only ever hits the exact limb is indistinguishable + * from a lookup that ignores the locale entirely. + */ + describe("the spec resolver's documented fallback", () => { + it('falls back from a region tag to its base language', () => { + // Author wrote only `zh`; viewer is `zh-CN`. + renderDirect( + 'filter-builder', + { type: 'filter-builder', label: { en: 'Owner', zh: '负责人' }, fields: [] }, + 'zh-CN', + ); + expect(screen.getByText('负责人')).toBeInTheDocument(); + }); + + it('falls back to a remaining entry when no limb matches', () => { + // Author wrote only `ja-JP`; viewer is `fr` — the doc's last-resort limb. + renderDirect( + 'filter-builder', + { type: 'filter-builder', label: { 'ja-JP': '所有者' }, fields: [] }, + 'fr', + ); + expect(screen.getByText('所有者')).toBeInTheDocument(); + }); + }); +}); diff --git a/packages/components/src/renderers/complex/filter-builder.tsx b/packages/components/src/renderers/complex/filter-builder.tsx index b6f98e6dfa..bf51f235a5 100644 --- a/packages/components/src/renderers/complex/filter-builder.tsx +++ b/packages/components/src/renderers/complex/filter-builder.tsx @@ -8,10 +8,20 @@ import { ComponentRegistry } from '@object-ui/core'; import type { FilterBuilderSchema, FilterGroup } from '@object-ui/types'; +import { useDisplayLocale } from '@object-ui/i18n'; +// Aliased on import, following PR #4169's convention (and `AppSchemaRenderer`'s +// use of it): this repo has its OWN `resolveKeyedI18nLabel` over a DIFFERENT +// vocabulary, and neither resolver accepts the other's shape. `schema.label` is +// the spec's INLINE locale map — see `BaseSchema.label` (objectui#4580). +import { resolveI18nLabel as resolveInlineI18nLabel } from '@objectstack/spec/ui'; import { FilterBuilder } from '../../custom/filter-builder'; -ComponentRegistry.register('filter-builder', +ComponentRegistry.register('filter-builder', ({ schema, className, onChange, ...props }: { schema: FilterBuilderSchema; className?: string; onChange?: (event: any) => void; [key: string]: any }) => { + // Read-time resolution against the display locale (objectui#4580 revised + // Q1-A). `BaseSchema.label` accepts `string | I18nLabel`; rendering the map + // straight into a text node THREW "Objects are not valid as a React child". + const locale = useDisplayLocale(); const handleChange = (value: any) => { if (onChange) { onChange({ @@ -26,7 +36,9 @@ ComponentRegistry.register('filter-builder', return (
{schema.label && ( - + )} ( - - {schema.label && {schema.label}} - - {renderChildren(schema.body)} - - - ), + ({ schema, ...props }: { schema: BaseSchema; [key: string]: any }) => { + // Read-time resolution against the display locale (objectui#4580 revised + // Q1-A). `BaseSchema.label` accepts `string | I18nLabel`; rendering the map + // straight into a text node THREW "Objects are not valid as a React child". + // The body became a block only to host this hook — the registry renders its + // entries with `React.createElement` (`SchemaRenderer.tsx:621`), so hooks + // are legal here, as `elements.tsx`'s own `useDisplayLocale()` already relies on. + const locale = useDisplayLocale(); + return ( + + {schema.label && ( + {resolveInlineI18nLabel(schema.label, locale)} + )} + + {renderChildren(schema.body)} + + + ); + }, { namespace: 'ui', label: 'Sidebar Group', diff --git a/packages/components/src/renderers/overlay/dropdown-menu.tsx b/packages/components/src/renderers/overlay/dropdown-menu.tsx index 4a7ae06170..69558ad8ed 100644 --- a/packages/components/src/renderers/overlay/dropdown-menu.tsx +++ b/packages/components/src/renderers/overlay/dropdown-menu.tsx @@ -8,6 +8,12 @@ import { ComponentRegistry } from '@object-ui/core'; import type { DropdownMenuSchema } from '@object-ui/types'; +import { useDisplayLocale } from '@object-ui/i18n'; +// Aliased on import, following PR #4169's convention: this repo has its OWN +// `resolveKeyedI18nLabel` over a DIFFERENT vocabulary, and neither resolver +// accepts the other's shape. `schema.label` is the spec's INLINE locale map — +// see `BaseSchema.label` (objectui#4580). +import { resolveI18nLabel as resolveInlineI18nLabel } from '@objectstack/spec/ui'; import { DropdownMenu, DropdownMenuTrigger, @@ -52,18 +58,28 @@ const renderMenuItems = (items: any[]) => { }; ComponentRegistry.register('dropdown-menu', - ({ schema, className, ...props }: { schema: DropdownMenuSchema; className?: string; [key: string]: any }) => ( - - - {renderChildren(schema.trigger)} - - - {schema.label && {schema.label}} - {schema.label && } - {renderMenuItems(schema.items)} - - - ), + ({ schema, className, ...props }: { schema: DropdownMenuSchema; className?: string; [key: string]: any }) => { + // Read-time resolution against the display locale (objectui#4580 revised + // Q1-A). `BaseSchema.label` accepts `string | I18nLabel`; rendering the map + // straight into a text node THREW "Objects are not valid as a React child" + // — observable only with the menu OPEN, since Radix mounts + // `DropdownMenuContent` lazily. The body became a block only to host this hook. + const locale = useDisplayLocale(); + return ( + + + {renderChildren(schema.trigger)} + + + {schema.label && ( + {resolveInlineI18nLabel(schema.label, locale)} + )} + {schema.label && } + {renderMenuItems(schema.items)} + + + ); + }, { namespace: 'ui', label: 'Dropdown Menu', diff --git a/packages/core/src/types/index.ts b/packages/core/src/types/index.ts index 29fe2b9256..a060b496fa 100644 --- a/packages/core/src/types/index.ts +++ b/packages/core/src/types/index.ts @@ -6,15 +6,50 @@ * LICENSE file in the root directory of this source tree. */ -export interface SchemaNode { - type: string; - id?: string; - className?: string; - data?: any; - body?: SchemaNode | SchemaNode[]; - [key: string]: any; -} +/** + * One `SchemaNode` (objectui#4580). + * + * This package used to hand-declare `interface SchemaNode { type: string; … + * [key: string]: any }` here, while `@object-ui/types` exported + * `type SchemaNode = BaseSchema | string | number | boolean | null | undefined` + * — two different types under one name, from two packages the same consumers + * import together. Which declaration a call site got depended on which package + * it happened to import from, and #4548's option-A canary measured **19 of 35** + * errors as exactly that collision: + * + * ``` + * Type 'import(".../packages/types/dist/base").SchemaNode' is not assignable to + * type 'import(".../packages/core/dist/types/index").SchemaNode' + * ``` + * + * PR #4578 sidestepped it — `SchemaRenderer` states its own component-level + * union rather than picking a side — and left the reconciliation to this card. + * + * **`@object-ui/types`' union wins**, per #4580's ruling 1: it is the spec of + * record (its own doc comment names `'Plain string'` a valid node), and it is + * the side the measured collisions resolve toward. The declaration is RE-EXPORTED + * rather than restated, so there is exactly one declaration left to disagree + * with — a structural copy would reproduce the defect the moment either side + * moved. This package already depends on `@object-ui/types`, so the edge exists + * and adds no cycle, and core's own entry surface is unchanged (`dist/index.d.ts` + * is byte-identical across the change — measured, both rounds). + * + * The collision is only observable from a package that resolves BOTH through + * `node_modules`; the pin therefore lives in `@object-ui/react` + * (`src/__tests__/SchemaNode.reconciliation.test.ts`), not here. + */ +export type { SchemaNode } from '@object-ui/types'; +import type { SchemaNode } from '@object-ui/types'; + +/** + * ⛔ Deliberately NOT reconciled with `@object-ui/types`' `ComponentRendererProps` + * (objectui#4594). The two declarations differ — types' is generic + * (`< TSchema extends BaseSchema = BaseSchema >`), this one is not — but that + * card measured **zero consumers** of this declaration, so reconciling it here + * would be an unmeasured change riding a card that was scoped to `SchemaNode`. + * It stays dual-declared until #4594 is dispatched on its own evidence. + */ export interface ComponentRendererProps { schema: SchemaNode; [key: string]: any; diff --git a/packages/plugin-dashboard/src/DashboardGridLayout.tsx b/packages/plugin-dashboard/src/DashboardGridLayout.tsx index 784c788a94..c57a86ed04 100644 --- a/packages/plugin-dashboard/src/DashboardGridLayout.tsx +++ b/packages/plugin-dashboard/src/DashboardGridLayout.tsx @@ -325,7 +325,23 @@ export const DashboardGridLayout: React.FC = ({
{hasDndProvider && }
-

{schema.title || schema.label || 'Dashboard'}

+ {/* + `schema.label` accepts the spec's INLINE locale map since objectui#4580's + revised Q1-A ruling, and rendering the map straight into this text node + THREW "Objects are not valid as a React child (found: object with keys + {en, zh-CN})". Resolved with `pickLocalized` against the ACTIVE UI + LANGUAGE, matching the widget-title resolution ~70 lines below rather + than introducing a second resolver and a second locale channel into one + component: `pickLocalized` is objectui's limb-for-limb twin of the spec's + `resolveI18nLabel` (objectstack#6765), differing only in how it spells a + miss (`''` vs `undefined`) — pinned in + `plugin-list/src/__tests__/i18nLabel-resolver-parity.test.ts`. The `||` + chain is preserved exactly: a miss yields `''`, which is falsy, so + `'Dashboard'` still backstops it. + */} +

+ {schema.title || pickLocalized(schema.label, language) || 'Dashboard'} +

{editMode ? ( <> diff --git a/packages/plugin-dashboard/src/__tests__/DashboardGridLayout.inlineLocaleLabel.test.tsx b/packages/plugin-dashboard/src/__tests__/DashboardGridLayout.inlineLocaleLabel.test.tsx new file mode 100644 index 0000000000..393fe2da1d --- /dev/null +++ b/packages/plugin-dashboard/src/__tests__/DashboardGridLayout.inlineLocaleLabel.test.tsx @@ -0,0 +1,103 @@ +/** + * 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 dashboard heading resolves an INLINE locale map label (objectui#4580). + * + * `BaseSchema.label` accepts `string | I18nLabel` since #4580's revised Q1 + * ruling. `DashboardGridLayout` renders `schema.label` into an `

` text + * node, which is the fourth and last site the widening's compiler inventory + * named repo-wide (the other three are `@object-ui/components` renderers, + * pinned in that package's `inline-locale-label-read-sites.test.tsx`). + * + * ## Red-first — measured BEFORE the fix, verbatim + * + * Rendering with `label: { en: 'Owner', 'zh-CN': '负责人' }` against the + * unfixed source THREW: + * + * ``` + * Objects are not valid as a React child (found: object with keys {en, zh-CN}). + * If you meant to render a collection of children, use an array instead. + * ``` + * + * ## Why `pickLocalized`, not the spec's `resolveI18nLabel` + * + * This component ALREADY resolves the sibling slot — `widget.title`, the same + * inline-map vocabulary — with `pickLocalized(widget.title, language)` against + * `useObjectTranslation().language`. Adding the spec resolver plus + * `useDisplayLocale()` here would put two resolvers AND two locale channels in + * one component for one vocabulary, and those channels genuinely disagree: + * `useDisplayLocale()` prefers the tenant's regional default over the active UI + * language, so a heading and the widget titles beneath it could resolve to + * different languages in the same render. + * + * That is not a tolerant fallback standing in for the real resolver: + * `pickLocalized` is objectui's limb-for-limb twin of `resolveI18nLabel` + * (objectstack#6765 aligned them deliberately), and the ONLY difference is how + * each spells a miss — `''` here for a text node, `undefined` there for a + * producer's `?? name` chain — pinned in + * `plugin-list/src/__tests__/i18nLabel-resolver-parity.test.ts`. + */ + +import { describe, it, expect } from 'vitest'; +import { render, screen } from '@testing-library/react'; +import { I18nProvider } from '@object-ui/i18n'; +import { DashboardGridLayout } from '../DashboardGridLayout'; + +const INLINE_MAP = { en: 'Owner', 'zh-CN': '负责人' } as const; + +/** + * The heading reads the ACTIVE UI LANGUAGE channel (`useObjectTranslation`), + * which is seeded by `config.defaultLanguage` — NOT by a `language` prop. + * `persistLanguage={false}` keeps a stored preference from outranking the seed, + * following `fields/src/__tests__/PercentCellRenderer.locale.test.tsx`. + */ +function renderGrid(schema: Record, language: string) { + return render( + + + , + ); +} + +describe('DashboardGridLayout heading — inline locale map label (objectui#4580)', () => { + it('resolves the map for the active language', () => { + renderGrid({ label: INLINE_MAP }, 'zh-CN'); + expect(screen.getByRole('heading', { level: 2 })).toHaveTextContent('负责人'); + }); + + it('resolves the same map differently for another language', () => { + renderGrid({ label: INLINE_MAP }, 'en'); + expect(screen.getByRole('heading', { level: 2 })).toHaveTextContent('Owner'); + }); + + it('passes a plain string label through unchanged', () => { + renderGrid({ label: 'Sales' }, 'zh-CN'); + expect(screen.getByRole('heading', { level: 2 })).toHaveTextContent('Sales'); + }); + + /** + * The `||` chain around the label is preserved exactly, in both directions. + * A resolver miss yields `''` (falsy), so the `'Dashboard'` backstop still + * fires — if the resolution had been spelled with the spec resolver's + * `undefined` miss it would behave the same here, but a `?? ''` written in the + * wrong place would have swallowed the backstop. + */ + it('keeps `title` ahead of `label` in the precedence chain', () => { + renderGrid({ title: 'Pipeline', label: INLINE_MAP }, 'zh-CN'); + expect(screen.getByRole('heading', { level: 2 })).toHaveTextContent('Pipeline'); + }); + + it("falls back to 'Dashboard' when the map resolves to nothing", () => { + renderGrid({ label: {} }, 'zh-CN'); + expect(screen.getByRole('heading', { level: 2 })).toHaveTextContent('Dashboard'); + }); +}); diff --git a/packages/react/src/__tests__/SchemaNode.reconciliation.test.ts b/packages/react/src/__tests__/SchemaNode.reconciliation.test.ts new file mode 100644 index 0000000000..c7c8be211b --- /dev/null +++ b/packages/react/src/__tests__/SchemaNode.reconciliation.test.ts @@ -0,0 +1,116 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * One `SchemaNode` (objectui#4580). + * + * Two packages published a type of this name and they were not the same type: + * + * `@object-ui/core` `interface SchemaNode { type: string; … [key: string]: any }` + * `@object-ui/types` `type SchemaNode = BaseSchema | string | number | boolean | null | undefined` + * + * Both are exported under one name from packages the same consumers import + * together, so which declaration a call site got depended on which package it + * happened to import from. #4548's option-A canary measured **19 of 35** errors + * as exactly this collision. PR #4578 sidestepped it — `SchemaRenderer` states + * its own component-level union instead of picking a side — and left the + * reconciliation to this card. + * + * `@object-ui/types`' union wins: it is the spec of record (its own doc comment + * names `'Plain string'` a valid node), and it is the side the measured + * collisions resolve toward. Core's hand-declared interface becomes a + * re-export, so there is exactly one declaration left to disagree with. + * + * ## Why this file lives in `@object-ui/react` + * + * It has to be a CONSUMER of both packages, resolving each through + * `node_modules` — that is the only place the collision is observable. This + * package's `tsconfig.test.json` sets `"paths": {}` precisely so `@object-ui/*` + * resolve through the workspace dependency's built `.d.ts` rather than pulling + * sibling sources in as program inputs, which is what makes the two `dist` + * identities in the error text below real. + * + * ## Predictions, written before the first run (red-first) + * + * Against `origin/main` (`92250d648`), i.e. with core's interface still + * hand-declared, `tsc -p packages/react/tsconfig.test.json` must report: + * + * 1. `assertion1` — `Equal< CoreSchemaNode, TypesSchemaNode >` resolves + * `false`, so `Expect< … >` fails its `extends true` constraint (TS2344). + * 2. `acceptsCoreNode(typesNode)` — TS2322, naming both `dist` identities: + * `Type 'import(".../packages/types/dist/base").SchemaNode' is not + * assignable to type 'import(".../packages/core/dist/types/index").SchemaNode'`. + * 3. `plainStringIsANode` — a plain string is a node per types' doc comment, + * and core's interface requires `type: string`, so this too is TS2322. + * + * After the fix all three compile clean. The assertions are deliberately + * type-level and INVARIANT (`Equal`, not `extends`): a one-way `extends` — or a + * bare `satisfies` — would stay green if core's declaration were merely made + * assignable rather than made THE SAME TYPE, which is the whole defect. + */ + +import { describe, it, expect } from 'vitest'; +import type { SchemaNode as CoreSchemaNode } from '@object-ui/core'; +import type { SchemaNode as TypesSchemaNode } from '@object-ui/types'; + +/* ── Type-level helpers ──────────────────────────────────────────────────── */ + +/** Invariant equality — `extends` both ways would accept a narrowing. */ +type Equal< A, B > = + (< T >() => T extends A ? 1 : 2) extends (< T >() => T extends B ? 1 : 2) ? true : false; +type Expect< T extends true > = T; + +/* ── 1. The two names are now one type ───────────────────────────────────── */ + +export type assertion1 = Expect< Equal< CoreSchemaNode, TypesSchemaNode > >; + +/* ── 2. The measured collision, pinned at a call boundary ────────────────── */ + +declare function acceptsCoreNode(node: CoreSchemaNode): void; +declare const typesNode: TypesSchemaNode; + +/** + * Assignment position — this is the spelling #4580 quotes, and it reports + * TS2322. The call-boundary spelling below is the same defect at an argument, + * where the compiler reports TS2345 instead; both are pinned because both are + * how the collision reached real call sites. + * + * ⚠️ Both live INSIDE never-called functions, and that is load-bearing rather + * than stylistic. `typesNode` and `acceptsCoreNode` are `declare`d — they are + * type-level fictions with no runtime existence — so a top-level + * `export const coreNodeHoldsTypesNode: CoreSchemaNode = typesNode;` type-checks + * exactly the same but **throws `ReferenceError: typesNode is not defined` the + * moment vitest imports the module**, failing the whole suite before a single + * case runs. Round 1 of this card only ever type-checked this file (the + * reconciliation was withdrawn before it reached a vitest run), so the defect + * was latent in the preserved pin and surfaced the first time both checkers saw + * it. A function body is checked by `tsc` just as thoroughly and never + * executes, which is what lets one file be read by both tools. + */ +export function assignmentPositionCompiles(): void { + const coreNodeHoldsTypesNode: CoreSchemaNode = typesNode; + void coreNodeHoldsTypesNode; +} + +export function crossPackageHandoffCompiles(): void { + // Pre-fix this is the 19-of-35 error class from #4548, verbatim. + acceptsCoreNode(typesNode); +} + +/* ── 3. The union's members are nodes through core's name too ────────────── */ + +export const plainStringIsANode: CoreSchemaNode = 'Plain string'; +export const nullishIsANode: CoreSchemaNode = null; +export const objectIsStillANode: CoreSchemaNode = { type: 'text', value: 'Hello' }; + +/* ── Runtime companion ───────────────────────────────────────────────────── */ + +describe('SchemaNode is declared once (objectui#4580)', () => { + it('type-level: core and types name the same type', () => { + // The assertions above are erased at runtime — `tsc -p tsconfig.test.json` + // is what checks them, and this package chains that from `type-check`. + // This case documents that the pin is compile-time, so a reader does not + // mistake a green vitest run for the proof. + const witness: CoreSchemaNode = 'Plain string'; + expect(witness).toBe('Plain string'); + }); +}); diff --git a/packages/react/src/spec-bridge/SpecBridge.ts b/packages/react/src/spec-bridge/SpecBridge.ts index 245bb57674..573dba828b 100644 --- a/packages/react/src/spec-bridge/SpecBridge.ts +++ b/packages/react/src/spec-bridge/SpecBridge.ts @@ -6,7 +6,7 @@ * LICENSE file in the root directory of this source tree. */ -import type { SchemaNode } from '@object-ui/core'; +import type { BaseSchema } from '@object-ui/types'; import type { BridgeContext, BridgeFn } from './types'; import { bridgeListView } from './bridges/list-view'; import { bridgeFormView } from './bridges/form-view'; @@ -25,8 +25,14 @@ export class SpecBridge { this.bridges.set(specType, bridge); } - /** Transform a spec schema into a SchemaNode tree */ - transform(specType: string, spec: any): SchemaNode { + /** + * Transform a spec schema into a schema node tree. + * + * Returns `BaseSchema` rather than `SchemaNode` — see {@link BridgeFn} for + * the ruling (objectui#4580 Q4-B) and the measurement behind it. A registered + * bridge always produces an object; the union only made callers narrow. + */ + transform(specType: string, spec: any): BaseSchema { const bridge = this.bridges.get(specType); if (!bridge) { throw new Error(`No bridge registered for spec type: ${specType}`); @@ -35,12 +41,12 @@ export class SpecBridge { } /** Transform a ListView spec */ - transformListView(spec: any): SchemaNode { + transformListView(spec: any): BaseSchema { return this.transform('list', spec); } /** Transform a FormView spec */ - transformFormView(spec: any): SchemaNode { + transformFormView(spec: any): BaseSchema { return this.transform('form', spec); } diff --git a/packages/react/src/spec-bridge/bridges/form-view.ts b/packages/react/src/spec-bridge/bridges/form-view.ts index c996486f90..d6f332a66a 100644 --- a/packages/react/src/spec-bridge/bridges/form-view.ts +++ b/packages/react/src/spec-bridge/bridges/form-view.ts @@ -6,7 +6,7 @@ * LICENSE file in the root directory of this source tree. */ -import type { SchemaNode } from '@object-ui/core'; +import type { BaseSchema } from '@object-ui/types'; import type { BridgeContext, BridgeFn } from '../types'; interface FormField { @@ -161,7 +161,7 @@ const PASSTHROUGH_KEYS = [ export const bridgeFormView: BridgeFn = ( spec: FormViewSpec, _context: BridgeContext, -): SchemaNode => { +): BaseSchema => { // Spec defines `groups` as a legacy alias of `sections`; normalize here so // downstream renderers only ever see `sections` (ObjectForm never reads a // `groups` key — before this normalization a groups-only spec silently @@ -169,7 +169,7 @@ export const bridgeFormView: BridgeFn = ( const sections = (spec.sections ?? spec.groups ?? []).map(mapSection); const formType = mapFormType(spec.type); - const node: SchemaNode = { + const node: BaseSchema = { type: 'object-form', id: `form-${spec.type ?? 'default'}`, sections, diff --git a/packages/react/src/spec-bridge/bridges/list-view.ts b/packages/react/src/spec-bridge/bridges/list-view.ts index efb1c95fc8..433e54bb38 100644 --- a/packages/react/src/spec-bridge/bridges/list-view.ts +++ b/packages/react/src/spec-bridge/bridges/list-view.ts @@ -6,7 +6,7 @@ * LICENSE file in the root directory of this source tree. */ -import type { SchemaNode } from '@object-ui/core'; +import type { BaseSchema } from '@object-ui/types'; import type { ListViewExportFormat, ListViewExportOptions } from '@object-ui/types'; import type { BridgeContext, BridgeFn } from '../types'; import type { ListView, ListColumn, RowHeight } from '@objectstack/spec/ui'; @@ -166,11 +166,11 @@ function liftExportOptions( export const bridgeListView: BridgeFn = ( spec: ListViewSpec, _context: BridgeContext, -): SchemaNode => { +): BaseSchema => { const columns = (spec.columns ?? []).map(mapColumn); const density = mapDensity(spec.rowHeight); - const node: SchemaNode = { + const node: BaseSchema = { type: 'object-grid', id: spec.name, columns, diff --git a/packages/react/src/spec-bridge/types.ts b/packages/react/src/spec-bridge/types.ts index a50cbb2fbe..b1f574d435 100644 --- a/packages/react/src/spec-bridge/types.ts +++ b/packages/react/src/spec-bridge/types.ts @@ -6,7 +6,7 @@ * LICENSE file in the root directory of this source tree. */ -import type { SchemaNode } from '@object-ui/core'; +import type { BaseSchema } from '@object-ui/types'; /** Context passed to all bridge functions */ export interface BridgeContext { @@ -31,8 +31,29 @@ export interface ObjectDefLite { }>; } -/** A bridge function transforms a spec schema into a SchemaNode tree */ +/** + * A bridge function transforms a spec schema into a schema node tree. + * + * Returns **`BaseSchema`**, not `SchemaNode` (objectui#4580, ruling Q4-B). + * Once core's `SchemaNode` became `@object-ui/types`' union + * (`BaseSchema | string | number | boolean | null | undefined`), declaring the + * return as `SchemaNode` made every caller re-ask a question no bridge has ever + * answered "yes" to: *is this a bare string / null?* Both shipped bridges end + * in a single `return node` on an object literal — `bridges/list-view.ts:226` + * and `bridges/form-view.ts:195` — so the wider declaration described nothing + * real while forcing a narrowing at every read. + * + * That cost was measured, not assumed: round 1 of this card recorded **272** + * mechanical errors across five spec-bridge suites, all of them TS18049 + * (`'node' is possibly 'null' or 'undefined'`) and TS2339 + * (`Property 'formType' does not exist on type + * 'string | number | boolean | BaseSchema'`) — the union being destructured by + * tests reading properties off a node the bridge always produces. + * + * This is a type ANNOTATION only: no runtime behaviour changes, and the emitted + * JavaScript is byte-identical (proven by bundle sha256 on this PR). + */ export type BridgeFn = ( spec: T, context: BridgeContext, -) => SchemaNode; +) => BaseSchema; diff --git a/packages/types/src/__tests__/base-schema-label-vocabulary.test.ts b/packages/types/src/__tests__/base-schema-label-vocabulary.test.ts index a74930fde6..efce1d3df1 100644 --- a/packages/types/src/__tests__/base-schema-label-vocabulary.test.ts +++ b/packages/types/src/__tests__/base-schema-label-vocabulary.test.ts @@ -52,14 +52,33 @@ * `disabledOn?: string` sibling exists for the same reason. The asymmetry with * `visible` was accidental, not deliberate. * - * ## `label` / `description` STAY `string` (ruling Q1-B) — must-not-change + * ## `label` / `description` — PIN MOVED (objectui#4580 revised Q1) * - * The two spec bridges hand these slots the spec's INLINE `I18nLabel`, which is - * a real defect (#4593's canary: TS2322 at `list-view.ts:180` and `:224`). The - * ruling resolves it at the BRIDGE, not by widening these declarations. The two - * assertions below are therefore the ruling written down: they are the only - * pins in this file expected GREEN pre-fix, and a future card that "fixes" the - * bridge defect by widening `BaseSchema.label` turns them red on purpose. + * ⚠️ **This section records a ruling that has since been REVISED, and the two + * pins below moved with it. The history is kept rather than rewritten, because + * the pins did exactly what they were built to do.** + * + * As written for #4581, these two slots STAYED `string` under ruling Q1-B: the + * two spec bridges hand them the spec's INLINE `I18nLabel` (#4593's canary: + * TS2322 at `list-view.ts:180` and `:224`), and Q1-B resolved that at the + * BRIDGE rather than by widening the declarations. The note below read: *"a + * future card that 'fixes' the bridge defect by widening `BaseSchema.label` + * turns them red on purpose."* + * + * That is what happened, on the very next card. PR #4603's seat measured + * Q1-B's premise FALSE on every leg — the bridge is a plain class method that + * cannot call `useDisplayLocale()`, `BridgeContext` declares no locale, + * `updateContext()` has zero callers, and `SpecBridge` has zero in-repo + * production consumers — so resolving there would freeze one audience's + * language into the node tree. The PM revised the ruling to **option A** + * (comment 5284973826): `label`/`description` widen to `string | I18nLabel` + * and resolution happens at READ time against the display locale. + * + * So these two pins turned red **on purpose, as designed**, and they are moved + * here to pin the widened unions. Their value as a pin is unchanged: they are + * still the ruling written down, just a different ruling — and the confusability + * they now guard is larger, because `label`/`description` (INLINE map) and + * `ariaLabel` (KEYED ref) now sit two properties apart on one interface. * * ## Predictions, written before the first run (red-first) * @@ -84,7 +103,8 @@ * not independent evidence. They are listed for completeness, and the * load-bearing pre-fix reds are 1-4. * 6. `assertionLabel` / `assertionDescription` — NO error, pre-fix and - * post-fix. See must-not-change above. + * post-fix. See the `label`/`description` section above. + * ⚠️ SUPERSEDED by #4580's revised Q1 — see the PIN MOVED record below. * * MEASURED (`52d878a3b`, before the fix) — 1, 2, 3, 4 and 6 held exactly: * @@ -119,6 +139,9 @@ import { describe, it, expect } from 'vitest'; import type { BaseSchema, KeyedI18nLabel } from '../base'; +// The INLINE vocabulary, bound from the spec by reference (never re-declared +// locally) — the same binding `packages/types/src/index.ts` re-exports. +import type { I18nLabel } from '@objectstack/spec/ui'; /* ── Type-level helpers ──────────────────────────────────────────────────── */ @@ -148,10 +171,62 @@ export type assertionDisabled = Expect< Equal< BaseSchema['disabled'], boolean | string | undefined > >; -/* ── must-not-change: the Q1-B ruling, written as a pin ──────────────────── */ +/* ── PIN MOVED (objectui#4580 revised Q1) ────────────────────────────────── */ + +/** + * **PIN MOVED (objectui#4580 revised Q1).** These two pinned + * `BaseSchema['label' | 'description'] === string | undefined` under ruling + * Q1-B, and turned red exactly as this file predicted when the revised ruling + * (comment 5284973826) widened them instead. Measured red before the move, + * verbatim: + * + * ``` + * base-schema-label-vocabulary.test.ts(153,38): error TS2344: Type 'false' does not satisfy the constraint 'true'. + * base-schema-label-vocabulary.test.ts(154,44): error TS2344: Type 'false' does not satisfy the constraint 'true'. + * ``` + * + * They now pin the widened unions. `I18nLabel` is the spec's INLINE locale map + * (`string | Record` in `@objectstack/spec` 17.0.0-rc.6), + * resolved at READ time by `resolveI18nLabel(label, locale)` — NOT + * `ariaLabel`'s keyed `resolveKeyedI18nLabel`, two properties away on this same + * interface. + * + * `Equal` is INVARIANT here for the reason PR #4593 spelled out and this file + * inherits: a one-way `extends` is vacuous for a widening in BOTH directions — + * the narrow `string` is assignable to the wide `string | I18nLabel`, so a + * widening that never happened AND one that overshot to `any` would both stay + * green. `BaseSchema`'s `[key: string]: any` makes the overshoot live. + */ +export type assertionLabel = Expect< + Equal< BaseSchema['label'], string | I18nLabel | undefined > +>; +export type assertionDescription = Expect< + Equal< BaseSchema['description'], string | I18nLabel | undefined > +>; + +/** + * The two vocabularies do NOT collapse into each other. Without this, a future + * edit that "simplified" `label` to `ariaLabel`'s union — or vice versa — would + * leave both `Equal` pins above green while silently swapping which resolver + * owns the slot. This is objectui#4167's confusability hazard, pinned. + */ +export type assertionVocabulariesAreDistinct = Expect< + Equal< Equal< BaseSchema['label'], BaseSchema['ariaLabel'] >, false > +>; -export type assertionLabel = Expect< Equal< BaseSchema['label'], string | undefined > >; -export type assertionDescription = Expect< Equal< BaseSchema['description'], string | undefined > >; +/** The inline map an author writes into `label` — the spec producer's shape. */ +export const inlineLocaleMapLabelIsAuthorable: BaseSchema = { + type: 'test-widget', + label: { en: 'Owner', 'zh-CN': '负责人' }, + description: { en: 'Record owner', 'zh-CN': '记录所有者' }, +}; + +/** A plain string stays authorable in both slots — the widening ADDS, never replaces. */ +export const plainStringLabelIsStillAuthorable: BaseSchema = { + type: 'test-widget', + label: 'Owner', + description: 'Record owner', +}; /* ── Authorable fixtures ─────────────────────────────────────────────────── */ diff --git a/packages/types/src/base.ts b/packages/types/src/base.ts index cc59f11031..b002befa67 100644 --- a/packages/types/src/base.ts +++ b/packages/types/src/base.ts @@ -16,6 +16,8 @@ * @packageDocumentation */ +import type { I18nLabel } from '@objectstack/spec/ui'; + /** * A KEYED i18n label — a reference INTO a translation bundle (objectui#4581). * @@ -86,14 +88,73 @@ export interface BaseSchema { /** * Display label for the component. * Often used in forms, cards, and other UI elements. + * + * Accepts the spec's INLINE LOCALE MAP as well as a plain string + * (objectui#4580, revised Q1 ruling — option A), because that is what a spec + * producer already writes into this slot: `bridgeListView` assigns + * `node.label = spec.label` at + * `packages/react/src/spec-bridge/bridges/list-view.ts:180`, and `ListView`'s + * own `label` is the spec's `I18nLabel`. Under the old `string` declaration + * that assignment was a type error the moment `SchemaNode` stopped being + * core's index-signature interface — the defect this widening resolves, not a + * capability being invented here. + * + * ## Which vocabulary this is, and who resolves it + * + * This slot — and {@link BaseSchema.description} two lines down — carries the + * spec's INLINE form: `I18nLabel` = `string | Record`, a + * locale MAP like `{ en: 'Owner', 'zh-CN': '负责人' }`, resolved against a + * BCP-47 locale by the spec's own `resolveI18nLabel(label, locale)` from + * `@objectstack/spec/ui`. Its documented fallback order is exact match → + * base/region (`zh-CN` ↔ `zh`) → last resort (any remaining entry), and it + * returns `undefined` when nothing matched, so read sites pair it with + * `?? someDefault`. + * + * ⚠️ {@link BaseSchema.ariaLabel}, two properties below, carries the OTHER + * vocabulary — the KEYED form {@link KeyedI18nLabel} (`{ key, defaultValue?, + * params? }`), a reference INTO a translation bundle, resolved by + * `resolveKeyedI18nLabel`. One interface now carries both, two properties + * apart, and they are structurally confusable: a keyed ref typed into this + * slot is accepted only *vacuously*, as a locale map whose "locales" are + * named `key` and `defaultValue`. That is objectui#4167's hazard, inherent to + * the spec's `I18nLabel` design and present on every spec surface using it; + * naming both shapes with cross-referenced docs is the accepted mitigation + * (#4580's revised Q1 ruling states this cost and accepts it). + * + * ## Resolution happens at READ time, not at the bridge + * + * The renderer resolves this against the display locale — `useDisplayLocale()` + * where the read site is in an i18n-reachable package, a locale threaded + * through props/context where it is not (`packages/layout` carries no i18n + * dependency). Resolving at the spec bridge was ruled out and measured + * unimplementable in PR #4603: the bridge is a plain class method that cannot + * call a hook, `BridgeContext` declares no locale, and `updateContext()` has + * zero callers — so a bridge-resolved label would freeze one audience's + * language into the node tree with no re-translation channel, the defect the + * spec's own resolver doc records as #6761. + * + * @example "Submit" + * @example { en: 'Submit', 'zh-CN': '提交' } */ - label?: string; + label?: string | I18nLabel; /** * Descriptive text providing additional context. * Typically rendered as help text below the component. + * + * Accepts the spec's INLINE LOCALE MAP as well as a plain string on exactly + * the {@link BaseSchema.label} evidence one slot over (objectui#4580, revised + * Q1 ruling): `bridgeListView` assigns `node.description = spec.description` + * at `packages/react/src/spec-bridge/bridges/list-view.ts:224`, where the + * spec's `ListView.description` is an `I18nLabel`. Same vocabulary, same + * resolver (`resolveI18nLabel` against the display locale), same + * confusability warning against {@link BaseSchema.ariaLabel}'s keyed form — + * see {@link BaseSchema.label} for the full statement. + * + * @example "Shown below the field" + * @example { en: 'Shown below the field', 'zh-CN': '显示在字段下方' } */ - description?: string; + description?: string | I18nLabel; /** * Placeholder text for input components. @@ -235,6 +296,16 @@ export interface BaseSchema { * returns `undefined` for it, rendering an EMPTY aria-label. The two * vocabularies are structurally confusable — objectui#4167's exact hazard. * + * ⚠️ That hazard is now LIVE ON THIS INTERFACE, not just adjacent to it: + * since #4580's revised Q1 ruling, {@link BaseSchema.label} and + * {@link BaseSchema.description} declare the spec's INLINE map (`I18nLabel`, + * resolved by `resolveI18nLabel(label, locale)`), while this slot declares + * the KEYED ref (resolved by `resolveKeyedI18nLabel`). Two properties apart, + * both spelled `string | {object}`, and each accepts the other's shape + * vacuously. Check which resolver owns a slot before writing an object into + * it; the ruling accepted this cost with exactly this naming + cross- + * referencing as the mitigation. + * * @example "Close dialog" * @example { key: 'dialog.close', defaultValue: 'Close dialog' } */