Uh oh!
There was an error while loading. Please reload this page.
fix(plugin-detail): record:alert CTA action.label accepts the inline locale map - #5572
Merged
Merged
Conversation
…locale map Widens the local, unexported `RecordAlertProps` CTA slot in `packages/plugin-detail/src/renderers/record-alert.tsx` from `label?: string` to `label?: string | I18nLabel` in both copies (`schema.properties.action` and the flat-compat `schema.action` mirror). The renderer already resolves `action.label` through the same inline locale-map `pickLocalized(props.action?.label, language)` call used for `title` / `body` two lines above — the declaration was narrower than the renderer's own runtime behavior, the same declaration-narrower-than-the- renderer contradiction objectui#4970 fixed for `title` / `body` in this same interface. This move was deliberately deferred by #4970 and filed as this issue. Type-only: the block's published authoring surface still declares `action` as a bare `object` with the member shape in prose only (`plugin-detail/src/index.tsx`), so there is no manifest arm to align against yet — that half stays parked on the `ComponentInput` member-shape question (PR #3795) and is out of scope here, per the issue and triage ruling. Adds a test asserting the case the type previously refused: a CTA whose `label` is an inline locale map (`{ en, 'zh-CN' }`), rendered to the resolved string for the active language via a real `I18nProvider`. A plain-string CTA label was already green before this change (existing "CTA label override" test) and cannot distinguish the widened declaration from the narrow one it replaces — reverse-verified: reverting the type widening alone reproduces `tsc` TS2322 at this test's locale-map literal (`type-check` is the gate this change is judged by), confirming the new case is what the old declaration refused. Fixes#4998. Co-authored-by: Claude <noreply@anthropic.com>
Contributor
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
os-sales
marked this pull request as ready for review
August 21, 2026 14:26
Uh oh!
There was an error while loading. Please reload this page.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes#4998
What
record:alert's renderer-local, unexportedRecordAlertPropsCTA slot(
action.label) is widened fromlabel?: stringtolabel?: string | I18nLabelin both copies in
packages/plugin-detail/src/renderers/record-alert.tsx:schema.properties.action.label— line 98schema.action.label(flat-compat mirror) — line 108This is the same move objectui#4970 already landed for
title/bodyin thesame interface: the renderer resolves
action.labelthrough the identicalinline-locale-map
pickLocalized(props.action?.label, language)call used fortitle/bodytwo lines above it, so the bare-stringdeclaration wasnarrower than the renderer's own runtime behavior. #4970 deliberately left this
slot narrow and filed it as this issue (see the doc comment it added, now
updated in this PR to say the widening landed).
Out of scope (per the issue + triage ruling)
The publish-surface half — declaring the
actionmember shape in the manifest— stays out.
plugin-detail/src/index.tsxstill declaresactionas a bareobjectwith the member shape in prose only, so there is no manifest arm toalign the type against yet; that's parked on the
ComponentInputmember-shapequestion in #3795. Not touched here.
Test
Added a case to
record-alert.test.tsxasserting the case the type previouslyrefused: a CTA whose
action.labelis an inline locale map(
{ en: 'Send again', 'zh-CN': '再次发送' }), rendered to the resolved stringfor the active language, via a real
I18nProviderpinning the language so theresolution is deterministic. The existing "CTA label override" test (plain
string label) stays green before and after this change and cannot distinguish
the widened declaration from the narrow one it replaces — this new case can,
because it is a type-check discriminator, not a runtime one:
pickLocalized'sown parameter is
unknown, so it never errors either way; only the callsite's literal (assigning an object to a
string-typed prop) tripstsc.Reverse-verified: with only the type declaration reverted to the pre-fix
label?: string(source only, no other edits),pnpm --filter @object-ui/plugin-detail type-checkfails exactly at the new test's locale-mapliteral:
Restoring the fix returns
type-checkto exit 0. This confirms the new case —not the pre-existing plain-string CTA test — is what the narrow declaration
refused.
Gates (all at
dc14b6488)pnpm --filter @object-ui/plugin-detail type-checktsc --noEmit && tsc -p tsconfig.test.json, no errors)pnpm exec vitest run packages/plugin-detail/src/renderers/__tests__/record-alert.test.tsxpnpm exec vitest run packages/plugin-detail/pnpm --filter @object-ui/plugin-detail lintno-explicit-any/ hook warnings, none on touched lines)check:control-bytesnode scripts/check-control-bytes.mjscheck-changeset-presence.mjscheck-changeset-no-major.mjscheck-changeset-fixed.mjsChangeset:
.changeset/record-alert-cta-label-i18n-4998.md(@object-ui/plugin-detail: patch).Generated by Claude Code
Generated by Claude Code