Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions .changeset/8648-ui-action-four-undeclared-keys.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.)
42 changes: 42 additions & 0 deletions .changeset/9542-result-dialog-i18nlabel.md
Original file line number Diff line number Diff line change
@@ -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.
42 changes: 36 additions & 6 deletions packages/app-shell/src/views/ActionResultDialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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 {
Expand All @@ -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
Expand Down Expand Up @@ -93,10 +120,10 @@ export function ActionResultDialog({ state, onAcknowledge }: ActionResultDialogP
>
<DialogHeader>
<DialogTitle>
{spec?.title || t('actions.resultDialog.defaultTitle') || 'Save this value now'}
{title || t('actions.resultDialog.defaultTitle') || 'Save this value now'}
</DialogTitle>
{spec?.description ? (
<DialogDescription>{spec.description}</DialogDescription>
{description ? (
<DialogDescription>{description}</DialogDescription>
) : null}
</DialogHeader>

Expand All @@ -113,7 +140,7 @@ export function ActionResultDialog({ state, onAcknowledge }: ActionResultDialogP

<DialogFooter>
<Button onClick={onAcknowledge}>
{spec?.acknowledge || t('actions.resultDialog.acknowledge') || 'I have saved this'}
{acknowledge || t('actions.resultDialog.acknowledge') || 'I have saved this'}
</Button>
</DialogFooter>
</DialogContent>
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<typeof import('@object-ui/i18n')>()),
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() }));
Expand Down Expand Up @@ -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();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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/;
Expand Down Expand Up @@ -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);
});
}
Expand Down
Loading
Loading