Repository navigation
fix(core/app-shell): accept and resolve an inline I18nLabel on an action's resultDialog (objectui#9542) - #9826
Merged
Conversation
…ontract and resolve them (objectui#9542) `@object-ui/core`'s `ResultDialogSpec` claimed 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 and neither deprecated. All four now derive from the contract type, so the alignment is re-checked by `tsc` instead of asserted in prose. Measured consequence: the map arm reached `ActionResultDialog`'s JSX as a React child, React refused the object, and the dialog threw rather than mis-rendering — the one-shot reveal failed to render at all on an action that had already succeeded. The dialog now resolves all four through `resolveI18nLabel` from `@objectstack/spec/ui` (the producer's own resolver for the inline form, aliased per this package's convention) against `useObjectTranslation().language`. Both action renderers drop the write-side narrowing assertion objectui#8648 left behind, and its ledger leg is converted to pin the unasserted write — keeping only the `as any` negative, which was never the workaround. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Xm4WFhEe5mwcgyqHjxR2hn
…it describes The derived local aliases had been inserted BETWEEN the "Mirrors `Action.resultDialog` in @objectstack/spec" docblock and the interface it describes, which detached the claim: `attachedDoc` in `scripts/check-spec-symbol-derivation.mjs` reads only the LAST leading block, so `pnpm check:spec-symbols` reported the claim as gone and asked for its ledger entry to be deleted. Moving the aliases above the docblock puts the claim back on the exported mirror, where the ledger already tracks it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Xm4WFhEe5mwcgyqHjxR2hn
github-actions
Bot
added
package: core
package: components
tests
package: app-shell
labels
Contributor
|
changeset-claim-re-read
|
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
|
…ded by this change `.changeset/8648-ui-action-four-undeclared-keys.md` is pending, so its body publishes verbatim into the CHANGELOG at the next release. Its last paragraph states in the present tense that the `resultDialog` WRITE carries a narrowing assertion at both forward sites and that a ledger entry pins it — which this change removes, in the same release. A supersession note is appended rather than the original claims rewritten: every word the objectui#8648 author wrote stands, and the final state is stated after it. The other three pending changesets naming files this branch edits were checked and are unmoved: 6306's `type: schema` population is byte-identical on the base and this branch and no line of it is in this diff; 8318's "`ActionRunner.ts` does not read `level`" still holds (every `level` in that file is prose); 5642's argument rests on `ActionDef` carrying no index signature and on the interface living in `ActionRunner.ts`, neither of which this diff touches. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Xm4WFhEe5mwcgyqHjxR2hn
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
|
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 free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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 #9542
Clause-②: no
@object-ui/core'sResultDialogSpecclaimed in its own docblock to mirrorAction.resultDialogin@objectstack/specand did not. Four of its members —title,description,acknowledgeandfields[].label— were hand-writtenstringwhere the contract declares every one of themI18nLabel: a plain string or an inline per-locale map, both authorized by that type's own docblock and neither deprecated. So this repo refused what the platform accepts, which ischeck:spec-symbols' rule-2 failure class — an alignment CLAIM with a hand copy behind it.The three steps: derive the members from the contract, do the resolution at the dialog, then delete the workaround objectui#8648 left behind.
The runtime severity: MEASURED, not restated
The card filed this reading as unverified and asked for confirmation. Confirmed — and it is a throw, not a mis-render. A rendering test with a locale-map
titleon the unfixed tree:Nothing upstream coerces or guards the value first. React 19.2.8 throws at the first such member reached, so the dialog never mounts — on an action that has already succeeded, and this dialog is the only place a one-shot reveal is ever shown. Three rendering legs failed this way before the change; the three plain-string legs in the same file passed unchanged, which is the control that makes this a reading about the map arm and nothing else.
client_secret/ backup-code examples are the contract's own illustrations of whatresultDialogis for. They are not a measured population and are not restated as one here.What changed
Derive, do not hand-copy.
@object-ui/corealready depends on@objectstack/specand already importsActionfrom@objectstack/spec/ui, so the contract type is reachable — checked rather than assumed, and no hand-written fallback was needed. The four label members are now indexed-access derivations off the contract's own result-dialog block through two module-local aliases. ⛔ No new exported symbol: the two interfaces stay the only names this module publishes for the block, so the clause-②nostands on its terms.Resolve at the dialog. Through
@objectstack/spec/ui'sresolveI18nLabel, imported under this package's establishedresolveInlineI18nLabelalias. ⛔ NotresolveKeyedI18nLabel— that takes the KEYED vocabulary and these members are the INLINE form. 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 (the||chain is kept over??for exactly that reason).The workaround is out. Both action renderers drop
as ActionDefOPEN-BRACKET'resultDialog'CLOSE-BRACKET, so the whole forward is compiler-checked with nothing between it andActionDefagain. (Spelled in words: GitHub's body sanitizer eats tag-shaped fragments, and an index into a type is what this sentence is about.)Zone-2 mechanism assumptions, each tested
The locale source — one source, no fork. All six in-package
resolveInlineI18nLabelsites trace to the same value,useObjectTranslation().languagefrom@object-ui/i18n:AppSidebarandUnifiedSidebardestructurelanguageoff that hook and pass it directly;resolveActionParamstakes it threaded in and says so in its own docblock; the three metadata-admin sites take alocaleprop fed byuseMetadataLocale, which reads that samelanguageand clamps it to that designer's two bundled catalogue locales. The clamp is about the designer's own copy catalogue, not a second source — so the un-clampedlanguageis what a runtime surface like this dialog wants, matching the three runtime sites. The dialog already called that hook fort.The sibling type carries the same drift, and repairing it is FORCED by ruling 5 — measured.
ResultDialogFieldSpec.labelis the contract's fourthI18nLabelmember on this block. It is not a scope judgement: withlabelreverted tostringand everything else in place,@object-ui/components' type-check goes red at BOTH forward sites withTS2322 … Types of property 'label' are incompatible, because the narrowing assertion that used to absorb it is gone. Deleting the assertions and leaving the sibling drifted do not compose. The ablation was run on disk and restored (git diff HEADempty afterwards).Tests
A case that is red before and green after, plus a converted pin that is red under ablation.
packages/app-shell/src/views/__tests__/ActionResultDialog.test.tsx— three new legs. Before:Tests 3 failed | 3 passed (6), all three failing with the React throw above. After: green. One leg varies only the display language and asserts the other locale's text is resolved AWAY, so the legs cannot pass by rendering both.packages/components/src/renderers/action/__tests__/action-undeclared-keys-8648.test.ts— the objectui#8648 ledger leg is converted, not merely deleted. Itsas anynegative was never the workaround:castBeforeonly matches a cast betweenschemaand the key, so a write-sideas anywalks straight past it and this is the only leg that refuses it. Ablation: re-injecting the narrowing assertion at both sites turns the converted legs red (Tests 2 failed | 14 passed); restored,git diff HEADempty.Commands and exit codes, each captured before any pipe, on
7321c6e3:pnpm exec vitest run packages/core/ packages/components/436 passed (436)files,6022 passed (6022)testspnpm exec turbo run build --filter=@object-ui/app-shell --filter=@object-ui/components --filter=@object-ui/corepnpm exec turbo run type-check(same three filters)pnpm check:spec-symbolspnpm check:action-forward-paritypnpm check:control-bytespnpm check:new-line-citationspnpm check:phantom-deps·check:unused-deps·check:self-import·check:esm-specifierspnpm check:published-dist·check:published-tsconfig-exclude·check:readme-exports·check:unreferenced-sourcespnpm check:comment-mask-corpus·check:i18n-keysnode scripts/check-changeset-presence.mjspnpm exec turbo run lint(same three filters)pnpm check:eager-closurecheck:eager-closureis a cached-artifact reading. The gauge needsapps/console/dist/eager-closure.json; the console build it comes from was a 100% turbo cache hit (35 cached / 35, 1s), and this checkout shares one turbo cache with every sibling worktree. So the gate's exit 0 does not prove anything about THIS tree's closure. It is reported as it is rather than greened over. The diff adds no new module edge into the console's eager graph —@objectstack/spec/uiis already imported eagerly byAppSidebarin this same package — so the expected movement is zero, but that is reasoning, not the measurement.The build had to run before the type-checks: a package type-check ahead of its dependency closure produces
TS6305false reds. Both readings were taken in that order and both were green.Deviations from the declared file surface — two, both declared
packages/app-shell/src/views/__tests__/ActionResultDialog.test.tsxis not on the declared surface, and the dispatch also requires a case that fails before the change and passes after. The card's own example is a locale-maptitlereaching this dialog, and its existing sibling test file is where that lives. Extended rather than stopped, and named here.attachedDocinscripts/check-spec-symbol-derivation.mjsreads only the LAST leading comment block — andpnpm check:spec-symbolswent red asking for the ledger entry to be deleted. Moving the aliases above the docblock restores the attachment. ⛔ No gate script was edited.Acceptance notes
check:spec-symbols' rule-2 ledger has no state for "claim now BACKED by derivation". Measured: with the claim re-attached and the members derived, deletingResultDialogFieldSpecfromCLAIM_DEBTmakes the gate red —a spec-alignment claim has nothing behind it— so the entry must stay although the debt it records is paid.CLAIM_DEBTmeans unbacked andCLAIM_ALLOWmeans deliberate divergence; a repaired symbol fits neither, so it can never leave the ledger, and the ledger's own burn-down instruction cannot be followed for this class. Left as a finding; a gate change is a maintainer-floor item and ⛔ not this card's.@object-ui/coreand had this very symbol ledgered asCLAIM_DEBTthe whole time. Extending rule 1 is therefore a smaller idea than the card implies — it was the CLAIM half, not the coverage half, that was missing.fieldsfallback entry spellslabel: undefinedexplicitly where omission would read the same; nothing carries it.What this PR does not touch
Out of scope:
#9572is not addressed here, and#8941remains open — both are neighbours in the same directories on different files, and neither is touched. objectui#8648 is referenced only as the card that left the workaround behind.This PR was drafted by an agent seat; the session reference is
session_01Xm4WFhEe5mwcgyqHjxR2hn.Generated by Claude Code
Generated by Claude Code