Uh oh!
There was an error while loading. Please reload this page.
The I18nLabel inverted pin now watches the premise it claims to, and ui-action.ts drops the unused import - #5634
Merged
Conversation
… forms The `it(...)` case that called itself an inverted pin on the spec's `I18nLabel` asserted `const label: SpecI18nLabel = 'Priority'` — type-correct under both the narrow and the wide shape, so it could only fire if the plain-string form were removed, the opposite of the widening it was written to catch. The widening had already landed (`@objectstack/spec@17.0.0` declares `z.ZodUnion<[z.ZodString, z.ZodRecord<z.ZodString, z.ZodString>]>`) and the pin stayed green. Retarget it at the premise that holds the decision up: BOTH authorized forms stay assignable, on `SpecI18nLabel` and on the inherited `ActionParam['label']` / `options[].label`. It now fails when either form is withdrawn. Retitled, and the comment rewritten against the spec's own two-forms doc block. Also drop the unused `I18nLabel` type import from `ui-action.ts` — residue of the removed local `label` override, with no type position left — and update the `NOT guarded` paragraph it sits under, which asserted the absence of the guard this commit adds. Fixes#5612Fixes#5613
…ed import Scored `patch`, not empty frontmatter: the published surface really does move. Measured with the package's real `tsc`, both legs from a cleared `dist/` and cleared composite build info — 108 emitted files on both sides, exactly one differing (`dist/ui-action.d.ts`, 31,026 -> 31,117 bytes, JSDoc prose only), `dist/ui-action.js` byte-identical. Same shape as the precedent on this block.
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 21:02
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
1 participant
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#5612
Fixes#5613
Folded per the claim on #5612: both cards are residue of the same removed local
label/options[].labeloverride inpackages/types, each is independently checkable, and one changeset covers both. Precedent: #4559 + #4966 landed as one PR in round 7.Part 1 (#5612) — the inverted pin that could not fire
The
it(...)case inpackages/types/src/__tests__/page-nav-misc-spec-parity.test.tscalled itself an inverted pin on the spec'sI18nLabeland rested on one assertion:A plain string is assignable under the narrow shape and under the wide one, so that assignment could only ever fail if the plain-string form were removed — the opposite of the event the comment said it was watching. Re-verified against the installed pin rather than taken from the card:
@objectstack/spec@17.0.0,node_modules/@objectstack/spec/dist/ui/index.d.ts:614i.e. the widening had already landed and the pin stayed green through it. Doc block at
dist/ui/index.d.ts:571-613— both card line numbers confirmed accurate.Retargeted at the premise that actually holds the decision up. Not which single form the spec has, but that both authorized forms stay assignable — on
SpecI18nLabeland on the inheritedActionParam['label']/options[].label. It now fails when either form is withdrawn, and deliberately does not fail on a further widening, since inheriting by reference is precisely what stays correct as the authorized set moves. Retitled, and the comment rewritten against the schema's own doc block ("Both are real; neither is deprecated by this schema", objectstack#5728, maintainer ruling 2026-08-06) instead of the false premise.Evidence the new pin can fail
⛔ A rewritten pin still green under both shapes would have fixed nothing, so this is shown, not asserted.
Standalone 2×2 truth table — the real spec type vs. a locally constructed
type I18nLabel = string:@objectstack/spec@17.0.0)string)const label: SpecI18nLabel = 'Priority'tscexit 0tscexit 0 — cannot discriminatetscexit 0tscexit 2 — firesThe wide column carries its own negative control, so it is not vacuously green — assigning a number there resolves the real union and fails, and the error text names the union, proving the leg did not silently fall back to
any:In-repo reverse verification, both legs mutation-confirmed on disk and restored to a byte-identical tree (
git diff --quiet HEAD→ 0):SpecI18nLabelinside the real test file →tsc -p tsconfig.test.jsonexit 2,TS2344+TS2322.label?: string) → exit 2, four errors, including bothActionParamlegs:Sibling pins in the same file — probed, none defective
The mechanical test for this defect class: a pin that cannot fail is one whose asserted value can be flipped without turning the compile red. Every type-level pin in the file was flipped one at a time, each flip confirmed on disk and restored; baseline green:
IsAny( SpecNavigationArea['navigation'][number] ) = false:251trueTS2322SpecThemeInput = authored(mutual assignability) :293TS2353ModeOfParsed = 'required':309'optional'TS2322ModeOfLocal = 'optional':313'required'TS2322RequiredOf = 'optional':346'required'TS2322_MapForm/_StringForm:389-390TS2344x2_ParamLabelMap/_OptionLabelMap:400-401TS2344x2The file's other 15 compile-time guards are
@ts-expect-errordirectives, which cannot be blind by construction: a directive suppressing nothing is itself an error. Measured rather than assumed — injecting a directive over an error-free line givesso with the baseline green, all 15 are live. No sibling pin carries the #5612 defect.
Part 2 (#5613) — the unused import
packages/types/src/ui-action.ts:27imported the typeI18nLabelwith no type position using it. Re-verified on currentorigin/main: the import line number :27 is accurate, but the card's occurrence count is stale — it recorded 4 (import + 3 in JSDoc), and the file now holds 6 word-boundary occurrences (import at :27 plus five in JSDoc prose at :317, :323, :335, :336, :350), because #5617 landed the prose rewrite after the card was filed. None is a type position; the symbol is not re-exported. Deleted.One bounded in-place fix, named here rather than left silent
The same JSDoc block carried a paragraph opening
NOT guarded:that recorded this very pin as absent coverage and pointed at #5612. Landing the guard in the same PR would have shipped a published comment asserting the absence of the thing the PR adds, so the paragraph is corrected to say what is now true. Same defect class (stale prose asserting a false premise about this exact pin), same file this card already claims, same gate family, no new verification surface.Changeset —
patch, not empty frontmatterLet the gate decide and measure
dist/rather than reasoning from an assumed zero delta — and the delta is not zero.Measured with the package's real
tsc, both legs built from a cleareddist/and cleared composite build info (tsconfig.tsbuildinfo, so neither leg skipped emit), comparing sha256 per file rather than byte counts:dist/ui-action.d.ts— 31,026 to 31,117 bytes. Content diff is JSDoc prose only; no declaration changed.dist/ui-action.jsbyte-identical (1b24e8f5…, 3,480 bytes) — the comment documents aninterface, erased at emit with its declaration.exclude: **/*.test.ts).So the published surface really does move, in the declaration file. That is the same shape and the same scoring as the precedent on this exact block (#5617 used
'@object-ui/types': patch), which is why the empty-frontmatter "publishes nothing" form would have been wrong here.Gates — run at
eca724225, the final commitThe repo-wide
pnpm lintran in full — no narrowing is being declared. Both changed files lint clean of new findings: the test file reports 0 errors / 0 warnings, andui-action.ts's 7no-explicit-anywarnings are pre-existing, at :600-642, far from either edit.Not run locally, and left to CI by the standing scope rule:
pnpm check(the CLI self-check, which needs the CLI build closure). Known-broken gauges noted, not touched:check-eager-closure-budgetexits 2,check-doc-snippet-typesexits 1.Out-of-scope finding — already filed, no new card
#5613's body also observes that
tsconfig.base.jsonsetsnoUnusedLocals: truewhilepackages/types/tsconfig.jsonextends the root config, which sets itfalse. Confirmed on currentorigin/main, and extended: nothing underpackages/orapps/extendstsconfig.base.jsonat all — the only consumer isexamples/byo-backend-console/tsconfig.json, plustsconfig.node.jsonandtsconfig.react.json, which nothing in turn extends.Searching before filing, per the file-first discipline, this is already open as #4806, which states both halves — the
noUnusedLocalsgap and the uncappedno-unused-varswarning. No duplicate card was created; today's re-measurement was added there as a comment instead.Generated by Claude Code