diff --git a/.changeset/8648-ui-action-four-undeclared-keys.md b/.changeset/8648-ui-action-four-undeclared-keys.md index 9afe96d1e3..ea7f838733 100644 --- a/.changeset/8648-ui-action-four-undeclared-keys.md +++ b/.changeset/8648-ui-action-four-undeclared-keys.md @@ -55,3 +55,12 @@ narrowing assertion to `ActionDef['resultDialog']` at both forward sites — strictly narrower than the `as any` it replaces, since every other member of the forward literal is compiler-checked again — documented as a ledger entry and pinned so it cannot regress to `as any` and cannot outlive its cause in silence. + +⇒ **It did not outlive it, and the paragraph above is superseded in this same +release.** objectui#9542 landed the derivation: `ResultDialogSpec` derives its +label members from the contract, so BOTH narrowing assertions and the ledger +entry that pinned them are gone and the write is compiler-checked with nothing +between it and `ActionDef`. What the pin still refuses is the `as any`. (Noted +here by the objectui#9542 seat, because this body publishes verbatim into the +CHANGELOG and its present tense would otherwise describe a workaround that the +same release removes.) diff --git a/.changeset/9542-result-dialog-i18nlabel.md b/.changeset/9542-result-dialog-i18nlabel.md new file mode 100644 index 0000000000..3bcc60b6ec --- /dev/null +++ b/.changeset/9542-result-dialog-i18nlabel.md @@ -0,0 +1,42 @@ +--- +'@object-ui/core': patch +'@object-ui/app-shell': patch +'@object-ui/components': patch +--- + +Accept an inline per-locale label on an action's `resultDialog`, and resolve it +against the display language (objectui#9542). + +`@object-ui/core`'s `ResultDialogSpec` claimed in its own docblock to mirror +`Action.resultDialog` in `@objectstack/spec` and did not: `title`, +`description`, `acknowledge` and each `fields[].label` were hand-written +`string` where the contract declares every one of them `I18nLabel` — a plain +string **or** an inline per-locale map, both authorized by that type's docblock +and neither deprecated. This repo therefore refused what the platform accepts, +which is the `check:spec-symbols` rule-2 failure class (an alignment CLAIM with +a hand copy behind it), one package over from where that gate matches by name. + +**The consequence was measured, not inferred.** The map arm reached +`ActionResultDialog`'s JSX as a React child and React refuses an object there, +so the dialog did not mis-render — it threw and failed to render at all, on an +action that had **already succeeded**. That dialog is the only place a one-shot +reveal is ever shown (a TOTP secret, a freshly minted OAuth `client_secret`, +regenerated backup codes), so the value was gone. The rendering test in +`packages/app-shell` reproduces the throw: it is red on the unfixed tree with +`Objects are not valid as a React child (found: object with keys {en, zh-CN})`. + +The four members now DERIVE from the contract type instead of restating it, so +the mirror cannot drift again without `tsc` saying so, and the dialog resolves +each through `resolveI18nLabel` from `@objectstack/spec/ui` — the producer's own +resolver for the inline form — against `useObjectTranslation().language`, the +same display locale every other inline-`I18nLabel` caller in `app-shell` +resolves against. A plain-string label is unaffected: the resolver returns it +unchanged, and the existing fallback chain still answers for an absent label or +a map with no usable entry. + +Both action renderers drop the write-side narrowing assertion objectui#8648 had +to leave behind, so the whole `resultDialog` forward is compiler-checked again +with nothing asserted between it and `ActionDef`. The ledger leg that pinned +that workaround is converted rather than deleted outright: the `as any` negative +it carried is not a workaround and still guards a write-side cast that the +read-side matcher walks straight past. diff --git a/packages/app-shell/src/views/ActionResultDialog.tsx b/packages/app-shell/src/views/ActionResultDialog.tsx index 64327b501e..80a62b304c 100644 --- a/packages/app-shell/src/views/ActionResultDialog.tsx +++ b/packages/app-shell/src/views/ActionResultDialog.tsx @@ -14,6 +14,10 @@ * are skipped — the response shape varies per request (e.g. no * `temporaryPassword` when the admin typed one), and a labelled * `undefined` row would be noise. + * - `title`, `description`, `acknowledge` and each `fields[].label` are + * `I18nLabel` on the contract — a plain string OR an inline per-locale map + * — and every one is resolved against the display language before it + * reaches the DOM (objectui#9542). * - The dialog has NO close button — the user must click acknowledge. * This is the whole point: a toast would let them dismiss the value * before reading it. @@ -33,6 +37,12 @@ import { import { useObjectTranslation } from '@object-ui/i18n'; import { Copy, Eye, EyeOff, Check } from 'lucide-react'; import { toCanvas } from 'qrcode'; +// Aliased per PR #4169's convention — app-shell has its OWN `resolveI18nLabel` +// for the KEYED vocabulary, which does not accept the inline per-locale map +// this resolves. ⛔ Not `resolveKeyedI18nLabel`: the two are structurally +// confusable and answer wrongly for each other's input (objectui#4167), and +// `resultDialog`'s label members are the INLINE form. +import { resolveI18nLabel as resolveInlineI18nLabel } from '@objectstack/spec/ui'; import type { ResultDialogSpec, ResultDialogFieldSpec } from '@object-ui/core'; export interface ResultDialogState { @@ -58,9 +68,26 @@ function readPath(root: unknown, path: string): unknown { } export function ActionResultDialog({ state, onAcknowledge }: ActionResultDialogProps) { - const { t } = useObjectTranslation(); + // `language` is the display locale every other inline-`I18nLabel` caller in + // this package resolves against — `AppSidebar` and `UnifiedSidebar` read it + // off this same hook, `resolveActionParams` takes it threaded in by both its + // callers, and metadata-admin's `useMetadataLocale` narrows this same value + // to that designer's two bundled locales. One source, so a title and the + // surrounding chrome cannot disagree about the user's language. + const { t, language } = useObjectTranslation(); const { spec, data } = state; + // Each of these three is an `I18nLabel` on the contract: a plain string, or + // an inline per-locale map. Resolved here, once, on the way INTO JSX — an + // unresolved map is an object, and React refuses an object as a child, so the + // dialog threw rather than mis-rendering and the one-shot value it exists to + // reveal was lost on an action that had already succeeded (objectui#9542). + // `|| fallback` is kept over `??`: the resolver answers `undefined` for a map + // with no usable entry, and an empty string must reach the default too. + const title = resolveInlineI18nLabel(spec?.title, language); + const description = resolveInlineI18nLabel(spec?.description, language); + const acknowledge = resolveInlineI18nLabel(spec?.acknowledge, language); + // Synthesise a single-field render plan when the action did not declare // explicit fields — keeps the dialog body uniform. Declared fields whose // path does not resolve in the payload are dropped: the value is @@ -93,10 +120,10 @@ export function ActionResultDialog({ state, onAcknowledge }: ActionResultDialogP > - {spec?.title || t('actions.resultDialog.defaultTitle') || 'Save this value now'} + {title || t('actions.resultDialog.defaultTitle') || 'Save this value now'} - {spec?.description ? ( - {spec.description} + {description ? ( + {description} ) : null} @@ -113,7 +140,7 @@ export function ActionResultDialog({ state, onAcknowledge }: ActionResultDialogP @@ -130,8 +157,11 @@ function ResultField({ value: unknown; defaultFormat?: FieldFormat; }) { + const { language } = useObjectTranslation(); const format: FieldFormat = field.format ?? defaultFormat ?? 'json'; - const label = field.label; + // Same `I18nLabel` contract as the dialog's own three label members, same + // resolution — a field label is the fourth member the block declares as one. + const label = resolveInlineI18nLabel(field.label, language); // Type-guard each renderer rather than crash if the server returned an // unexpected shape (e.g. format=qrcode but value is undefined). We diff --git a/packages/app-shell/src/views/__tests__/ActionResultDialog.test.tsx b/packages/app-shell/src/views/__tests__/ActionResultDialog.test.tsx index aff2c9320f..555e88b782 100644 --- a/packages/app-shell/src/views/__tests__/ActionResultDialog.test.tsx +++ b/packages/app-shell/src/views/__tests__/ActionResultDialog.test.tsx @@ -5,9 +5,19 @@ import { render, screen } from '@testing-library/react'; import type { ResultDialogSpec } from '@object-ui/core'; import { ActionResultDialog } from '../ActionResultDialog'; +/** + * The display language the dialog resolves an `I18nLabel` against, mutable per + * test. Hoisted so the `vi.mock` factory below can close over it — a plain + * `let` would still be in TDZ when the hoisted factory first runs. + */ +const i18nState = vi.hoisted(() => ({ language: 'en' })); + vi.mock('@object-ui/i18n', async (importOriginal) => ({ ...(await importOriginal()), - useObjectTranslation: () => ({ t: (key: string) => key }), + // `language` is the source every other `resolveI18nLabel` caller in this + // package threads (`AppSidebar`, `UnifiedSidebar`, `resolveActionParams`, and + // metadata-admin's `useMetadataLocale`), so the stub carries it too. + useObjectTranslation: () => ({ t: (key: string) => key, language: i18nState.language }), })); // jsdom has no canvas; the qrcode path is not under test. vi.mock('qrcode', () => ({ toCanvas: vi.fn() })); @@ -57,3 +67,67 @@ describe('ActionResultDialog — unresolved field paths are skipped', () => { expect(screen.getByText(/"anything": 1/)).toBeInTheDocument(); }); }); + +/** + * objectui#9542 — every label member the contract declares as `I18nLabel` + * reaches this dialog in EITHER authorized form: a plain string, or an inline + * per-locale map. `@objectstack/spec`'s `I18nLabel` authorizes both and + * deprecates neither, and `ActionSchema.resultDialog` declares `title`, + * `description`, `acknowledge` and `fields[].label` as `I18nLabel`. + * + * ⭐ Before this card these went red by THROWING, not by mis-rendering: the map + * was passed straight into JSX as a React child, and React refuses an object + * there ("Objects are not valid as a React child"). The dialog therefore failed + * to render AT ALL on an action that had already succeeded — and this dialog is + * the only place its one-shot value is ever shown. The plain-string cases above + * are the control: they passed before and after, so these legs are about the + * map arm and nothing else. + */ +describe('ActionResultDialog — an inline per-locale map resolves against the display language', () => { + it('resolves title, description and acknowledge for the active language', () => { + i18nState.language = 'zh-CN'; + open( + { + title: { en: 'Save this value now', 'zh-CN': '立即保存此值' }, + description: { en: 'Shown once.', 'zh-CN': '仅显示一次。' }, + acknowledge: { en: 'I have saved this', 'zh-CN': '我已保存' }, + fields: [{ path: 'secret', label: { en: 'Secret', 'zh-CN': '密钥' }, format: 'text' }], + }, + { secret: 's3cr3t' }, + ); + + expect(screen.getByText('立即保存此值')).toBeInTheDocument(); + expect(screen.getByText('仅显示一次。')).toBeInTheDocument(); + expect(screen.getByText('我已保存')).toBeInTheDocument(); + expect(screen.getByText('密钥')).toBeInTheDocument(); + // The value itself still reaches the user — the whole reason the dialog exists. + expect(screen.getByText('s3cr3t')).toBeInTheDocument(); + // The other locale's text is resolved AWAY, not rendered alongside. + expect(screen.queryByText('Save this value now')).not.toBeInTheDocument(); + expect(screen.queryByText('Secret')).not.toBeInTheDocument(); + }); + + it('picks the other locale when the display language changes — so the language is really read', () => { + i18nState.language = 'en'; + open( + { + title: { en: 'Save this value now', 'zh-CN': '立即保存此值' }, + fields: [{ path: 'secret', label: { en: 'Secret', 'zh-CN': '密钥' }, format: 'text' }], + }, + { secret: 's3cr3t' }, + ); + + expect(screen.getByText('Save this value now')).toBeInTheDocument(); + expect(screen.getByText('Secret')).toBeInTheDocument(); + expect(screen.queryByText('立即保存此值')).not.toBeInTheDocument(); + }); + + it('falls back to the translated default when the map has no usable entry', () => { + i18nState.language = 'zh-CN'; + // An empty map resolves to `undefined`, which must behave exactly as an + // absent `title` does — the fallback chain, not an empty heading. + open({ title: {}, fields: [{ path: 'secret', format: 'text' }] }, { secret: 's3cr3t' }); + + expect(screen.getByText('actions.resultDialog.defaultTitle')).toBeInTheDocument(); + }); +}); diff --git a/packages/components/src/renderers/action/__tests__/action-undeclared-keys-8648.test.ts b/packages/components/src/renderers/action/__tests__/action-undeclared-keys-8648.test.ts index 41132aa975..6f41cafa08 100644 --- a/packages/components/src/renderers/action/__tests__/action-undeclared-keys-8648.test.ts +++ b/packages/components/src/renderers/action/__tests__/action-undeclared-keys-8648.test.ts @@ -50,16 +50,17 @@ * unwraps the cast) still reports as present. Declaring without un-casting * is an inert declaration that pins green. * - Every negative leg carries a control that varies ONLY the claim. - * - The last block is a LEDGER, not a pin on a good state. Un-casting the + * - The last block WAS a ledger and is not one any more. Un-casting the * `resultDialog` READ made the compiler name a second, separate defect the - * `as any` had been hiding: `@object-ui/core`'s `ResultDialogSpec` is a + * `as any` had been hiding: `@object-ui/core`'s `ResultDialogSpec` was a * hand copy of the contract's block whose `title` / `description` / - * `acknowledge` are `string` where the contract says `I18nLabel`. That fix - * is in another package and needs an i18n-resolution decision, so it is - * filed as objectui#9542 and the WRITE carries a narrowing assertion here - * meanwhile. The ledger leg asserts that workaround is still exactly what - * it says it is — so it cannot quietly become `as any` again, and it goes - * red (as an unused entry) when objectui#9542 lands and the assertions go. + * `acknowledge` read `string` where the contract says `I18nLabel`. That + * repair was filed as objectui#9542 and has LANDED — the mirror derives + * those members from the contract now — so the narrowing assertion the + * WRITE carried meanwhile is deleted, and the ledger entry pinning it with + * it. What replaces the block pins the two things that outlive a + * workaround: the write arriving with nothing asserted between it and + * `ActionDef`, and the `as any` it must never become. */ import { describe, it, expect } from 'vitest'; import { readFileSync } from 'node:fs'; @@ -227,8 +228,11 @@ const maskedSource = (file: string): string => mask(readFileSync(join(RENDERERS, const castBefore = (key: string): RegExp => new RegExp(String.raw`\(\s*schema\s+as\s+\w+\s*\)\s*\.\s*${key}\b`); -/** The write-side narrowing this card had to leave behind — see objectui#9542. */ -const LEDGERED_WRITE_NARROWING = /resultDialog:\s*schema\.resultDialog as ActionDef\['resultDialog'\]/; +/** + * The write, forwarded with NOTHING between it and `ActionDef` — the state + * objectui#9542 left behind when it deleted this card's write-side narrowing. + */ +const UNNARROWED_WRITE = /resultDialog:\s*schema\.resultDialog\s*,/; /** The spelling it must never regress to. */ const WRITE_SIDE_ANY = /resultDialog:\s*schema\.resultDialog as any/; @@ -303,24 +307,39 @@ describe('objectui#8648 — the mirror declaration reaches the read sites (NOT i } }); -describe('objectui#8648 — the `resultDialog` write-side narrowing is a LEDGER entry, not a fix', () => { +/* + * The LEDGER entry this card left behind is GONE, and this block is what it + * became. objectui#9542 made `@object-ui/core`'s `ResultDialogSpec` DERIVE its + * label members from the contract instead of hand-writing them as `string`, so + * the narrowing assertion that stood at both write sites had nothing left to + * narrow and was deleted — the workaround did not outlive its cause. + * + * ⭐ What survives is the half that was never a workaround: the `as any` + * negative. `castBefore` matches a cast between `schema` and the key, which a + * cast on the WRITE side — `schema.resultDialog as any` — walks straight past, + * so this is the only leg that refuses it. It is also the spelling that hid two + * defects at once, which is why the guard outlives the entry. + */ +describe('objectui#9542 — the `resultDialog` write reaches `ActionDef` with nothing between', () => { it('both matchers can fire, so the legs below are readings', () => { + // The controls vary ONLY the claim: the same write, asserted and plain. const narrowed = "resultDialog: schema.resultDialog as ActionDef['resultDialog'],"; - expect(LEDGERED_WRITE_NARROWING.test(narrowed)).toBe(true); - expect(WRITE_SIDE_ANY.test(narrowed)).toBe(false); + expect(UNNARROWED_WRITE.test('resultDialog: schema.resultDialog,')).toBe(true); + expect(UNNARROWED_WRITE.test(narrowed)).toBe(false); expect(WRITE_SIDE_ANY.test('resultDialog: schema.resultDialog as any,')).toBe(true); + expect(WRITE_SIDE_ANY.test('resultDialog: schema.resultDialog,')).toBe(false); }); for (const file of CARD_KEYS.resultDialog) { - it(`${file} narrows the \`resultDialog\` WRITE to \`ActionDef\` and never to \`any\``, () => { + it(`${file} forwards \`resultDialog\` unasserted, and never as \`any\``, () => { const source = maskedSource(file); - // The entry is live. When objectui#9542 lands, `ResultDialogSpec` derives - // from the contract, the assertion is deleted, and THIS goes red — which - // is the point: a workaround must not outlive its cause in silence. - expect(source).toMatch(LEDGERED_WRITE_NARROWING); + // Proof the file was read and masked, so the reading below is about the + // spelling and not about an empty string. + expect(source).toMatch(/UIActionSchema/); + expect(source).toMatch(UNNARROWED_WRITE); // ⛔ The spelling that hid two defects at once must not come back. `as - // any` here would re-swallow the `ResultDialogSpec` drift AND make the - // mirror declaration inert at this site in one stroke. + // any` here would re-swallow a future `ResultDialogSpec` drift AND make + // the mirror declaration inert at this site in one stroke. expect(source).not.toMatch(WRITE_SIDE_ANY); }); } diff --git a/packages/components/src/renderers/action/action-button.tsx b/packages/components/src/renderers/action/action-button.tsx index 83edd648eb..f4aa74c405 100644 --- a/packages/components/src/renderers/action/action-button.tsx +++ b/packages/components/src/renderers/action/action-button.tsx @@ -232,20 +232,15 @@ const ActionButtonRenderer = forwardRef< // backup codes). Without this forward the ActionRunner falls // back to the success toast and the user loses the value. // - // The READ is uncast since objectui#8648: `resultDialog` is declared - // on the mirror, so the compiler types it as the contract's own - // block. What the assertion narrows is the WRITE, and it is a - // ledgered workaround, not a shrug — removing the read-side `as any` - // is what made the compiler name it. `ActionDef['resultDialog']` is - // `@object-ui/core`'s hand-written `ResultDialogSpec`, whose own - // docblock claims it mirrors the contract's block and does not: - // `title` / `description` / `acknowledge` are `string` there and - // `I18nLabel` in the contract, so a contract-valid inline locale map - // reaches the dialog as an object. Filed as objectui#9542; the fix is - // in `@object-ui/core` plus the dialog's own resolver and is outside - // this card's declared surface. ⛔ Never widen this back to `as any` — - // that spelling hid this AND the missing declaration at once. - resultDialog: schema.resultDialog as ActionDef['resultDialog'], + // Both ends uncast: the READ since objectui#8648 (`resultDialog` is + // declared on the mirror, so the compiler types it as the contract's + // own block), and the WRITE since objectui#9542 retired the narrowing + // assertion that stood here — `ActionDef['resultDialog']` now DERIVES + // its label members from the contract instead of hand-writing them as + // `string`, so the whole forward type-checks against one declared + // meaning. ⛔ Never widen either end back to `as any`: that one + // spelling hid the missing declaration AND the drifted mirror at once. + resultDialog: schema.resultDialog, // Declared post-success navigation — spec's closed strict // `{ navigate, openIn }` block, authorable on `ActionSchema` since // @objectstack/spec 17.1.0 (objectui#5328). The runner reads it off diff --git a/packages/components/src/renderers/action/action-icon.tsx b/packages/components/src/renderers/action/action-icon.tsx index 978cac79bf..70e5ad4841 100644 --- a/packages/components/src/renderers/action/action-icon.tsx +++ b/packages/components/src/renderers/action/action-icon.tsx @@ -154,10 +154,11 @@ const ActionIconRenderer = forwardRef< // See action-button.tsx — the one-shot reveal spec (2FA setup, fresh // OAuth secret). Without it the runner falls back to the success // toast and the value the user was meant to copy is gone. The READ is - // uncast since objectui#8648; the write-side assertion is the same - // ledgered `ResultDialogSpec` drift `action-button.tsx` documents - // (filed as objectui#9542). ⛔ Never widen it back to `as any`. - resultDialog: schema.resultDialog as ActionDef['resultDialog'], + // uncast since objectui#8648; the write-side narrowing that stood + // here went with objectui#9542, which made `ResultDialogSpec` derive + // its label members from the contract. ⛔ Never widen either end back + // to `as any`. + resultDialog: schema.resultDialog, // See action-button.tsx — the declared post-success hop // (objectui#5493). The runner reads it off the forwarded def; dropped // here the action succeeds and the authored navigation never runs. diff --git a/packages/core/src/actions/ActionRunner.ts b/packages/core/src/actions/ActionRunner.ts index 65f7481677..319b4c7efd 100644 --- a/packages/core/src/actions/ActionRunner.ts +++ b/packages/core/src/actions/ActionRunner.ts @@ -542,10 +542,30 @@ export type ParamCollectionHandler = ( action?: ActionDef, ) => Promise | null>; +/** + * The contract's own result-dialog block, and one entry of its field list. + * + * ⭐ The two interfaces below DERIVE their label members from these instead of + * restating them, and that is the whole repair of objectui#9542: the three + * label members were hand-written `string` while the producer declares each as + * `I18nLabel` — a plain string **or** an inline per-locale map, both authorized + * and neither deprecated — so this mirror refused what the platform accepts + * while its own docblock claimed the alignment. A hand copy can drift again the + * moment the contract moves; a derived member makes `tsc` re-check the claim on + * every build instead of trusting a sentence written once. + * + * ⛔ Local, deliberately not exported: the two interfaces below stay the only + * names this module publishes for the block. + */ +type SpecResultDialog = NonNullable; +type SpecResultDialogField = NonNullable[number]; + /** * Result dialog spec — declarative description of how to render a * one-shot reveal of an action's API response. Mirrors - * `Action.resultDialog` in @objectstack/spec. + * `Action.resultDialog` in @objectstack/spec — derived from it member by + * member for the labels, so the claim in this sentence is enforced rather + * than asserted. * * When set on an action and the action succeeds, the runner suppresses * the success toast and awaits a ResultDialogHandler instead. Used for @@ -554,13 +574,22 @@ export type ParamCollectionHandler = ( */ export interface ResultDialogFieldSpec { path: string; - label?: string; + /** + * Derived `I18nLabel` — a plain string, or an inline per-locale map. A + * renderer must RESOLVE it (`resolveI18nLabel` in `@objectstack/spec/ui`) + * before it reaches the DOM; passed through as-is, the map arm is an object + * and React refuses an object as a child. + */ + label?: SpecResultDialogField['label']; format?: 'qrcode' | 'code-list' | 'secret' | 'text' | 'json'; } export interface ResultDialogSpec { - title?: string; - description?: string; - acknowledge?: string; + /** Derived `I18nLabel` — see `ResultDialogFieldSpec.label` on resolving it. */ + title?: SpecResultDialog['title']; + /** Derived `I18nLabel` — see `ResultDialogFieldSpec.label` on resolving it. */ + description?: SpecResultDialog['description']; + /** Derived `I18nLabel` — see `ResultDialogFieldSpec.label` on resolving it. */ + acknowledge?: SpecResultDialog['acknowledge']; format?: 'qrcode' | 'code-list' | 'secret' | 'text' | 'json'; fields?: ResultDialogFieldSpec[]; }