test(core,app-shell): type-check the whole test trees, retire both narrow typetests projects (#4040) - #4379
Merged
Conversation
…rrow typetests projects (#4040) `@object-ui/core` and `@object-ui/app-shell` — the last two large TEST_DEBT packages — each gain a `tsconfig.test.json` chained from `type-check`, so their 81 and 350 test files are compiled by something for the first time. Measured at the branch point: core 132 raw / 56 code-tier, app-shell 221 raw / 62 code-tier, against registry entries declaring exactly 56 and 62 — the first tranche where the declared numbers held, because they were the tranche-4 remeasurement rather than the unreliable #2911-era sweep. Config tier: `paths: {}` kills 39 + 121 TS6059 source-leaks; core needs `types: ["node"]` (22 TS2591 + 13 TS2304 for `node:fs` / `__dirname` / `global`); app-shell needs `lib: ES2022` (38 TS2550 for `calls.at(-1)`) plus `vite/client` + `@testing-library/jest-dom`, raised in the TEST project only so the package source keeps compiling against the baseline it ships to. Three source declarations were narrower than their implementation, and only the tests could see it. `ConsoleActionRuntime.actionProviderProps` hand-restated ActionProvider's props and had drifted twice — it omitted `onModal`, which the implementation returns, and typed `handlers` values as one-parameter functions; it is now `Required<Pick<ActionProviderProps, …>>`, an explicit key list with derived types. `apiHandler` declares the `context` parameter it has always taken. `AiChatPage` imported the plugin barrel's minimal legacy `ChatMessage` while producing the enhanced shape — the mismatch survived only because the extra keys are spread conditionally, which defeats excess-property checking. Substantive test fixes: `TransactionManager.test.ts` authored its fixtures in the deprecated `crud.ts` `ActionSchema` while the manager takes `UIActionSchema` (17 of core's 56); `schema-builder.test.ts` spelled columns `{ name, label }`, a dialect `TableColumn` does not have, so the assertion round-tripped its own input; eight app-shell preview suites spread `props as never`, a cast that had never been checked; twelve mocks across both packages carried `ReturnType<typeof vi.fn>` / zero arity, whose `mock.calls` is the empty tuple. Per #4291's ratchet both narrow `tsconfig.typetests.json` projects retire here: `--listFiles` shows the full projects read all 2 + 5 files they named, and a provably-false `Assert` appended to each parity test turns the full project red. app-shell's orphan `type-check:typetests` script goes with it. TEST_DEBT is down to `@object-ui/plugin-dashboard` alone; the gate now reports 39/40 packages compiling their tests and 0 narrow type-assertion projects. Refs #4040, #4291, #3181, #3009, #3968.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
…ad of "some remain" (#4040) `check-type-check-coverage.test.ts` asserted `withNarrow.length > 0` under a comment saying survivors were guaranteed while #4040 burned down. Tranche 5 is where that premise expires: retiring `core`'s and `app-shell`'s narrow projects takes the last two, so the assertion is false by construction. Rewritten to the terminal statement — `expect(withNarrow.map(p => p.name)) .toEqual([])` — which is NOT vacuous in the direction that matters: a `tsconfig.typetests.json` reappearing anywhere turns it red, i.e. #4291's ratchet stated as a test and not only as a gate rule. Proved by driving `collect()` with a synthetic narrow project on `app-shell`: control `[]`, probe `["@object-ui/app-shell"]`. The sibling "holds each survivor" case is now vacuous by design, and says so; it re-arms the moment the set is non-empty again. `@object-ui/core` and `@object-ui/app-shell` join the named `retired` list, which is the case that keeps carrying real weight. Refs #4040, #4291.
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
Collaborator
Author
|
ACCEPT — PM 复核 (session
Flipping ready + arming auto-merge. Landing this opens #4373's gate (its other gate, #4374, already landed) and frees app-shell for #4377. Generated by Claude Code |
yinlianghui
marked this pull request as ready for review
August 12, 2026 00:05
This was referenced Aug 12, 2026
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.
Part of #4040 — tranche 5 (final).
@object-ui/plugin-dashboard's 6 errors are deliberately out of scope (its surface belongs to the #4330 queue candidate's family), so itsTEST_DEBTline stays and this PR does not close the card.@object-ui/coreand@object-ui/app-shell— the two largest debt packages, held back to last by the standing small-first ruling — each gain atsconfig.test.jsonchained fromtype-check, so their 81 and 350 test files are compiled by something for the first time. BothTEST_DEBTlines are removed; per #4291's ratchet both packages' narrowtsconfig.typetests.jsonprojects retire in this same PR.After this,
check-type-check-coverage.mjsreports 39/40 packages compile their tests and 0 narrow type-assertion projects remaining.Measurement — red before, green after
Measured per the discipline adopted at tranche 2: every compiler stream written to a FILE and counted there, never piped through
head/tail(SIGPIPE truncates mid-stream and fabricates a plausible count).@object-ui/core@object-ui/app-shellBoth registry numbers were exactly right — the first tranche where that is true. They are the tranche-4 remeasurement, and the note warning that the older #2911-era numbers were unreliable in both directions is what made them trustworthy; the note is folded into the table header now that the two entries it described are gone.
Raw runs use a temp config that lifts only the test exclusion from each package's own build config; code-tier is against the template config each package graduates with.
core, pre-fix (56):
TS2322×17,TS7006×10,TS2345×7,TS2339×5,TS2304×5,TS2353×4,TS2532×3,TS2352×2,TS6133/TS2538/TS2348×1.core, config-tier (76):
TS6059×39 (source-treepathspulling siblingsrc/in as program inputs →paths: {}),TS2591×22 +TS2304×13 (node:fs/__dirname/global→types: ["node"]),TS7006×2 that fell out with them.app-shell, pre-fix (62):
TS2322×10,TS2698×8,TS2339×8,TS2739×7,TS2493×6,TS2345×6,TS2554×4,TS7006×3,TS2559×3,TS2769×2,TS2352×2,TS2578/TS2353/TS2307×1.app-shell, config-tier (159):
TS6059×121 (samepathsleak) andTS2550×38 —calls.at(-1)and friends against the package build'slib: ES2020. Raised toES2022in the TEST project only, so the package SOURCE keeps compiling against the baseline it ships to.After (both green, and the source projects unchanged):
Config-tier recipe
Both projects extend the repo root, set
noEmit/composite: false/declaration: false, droppathsto{}, and includesrc/**/*.test.ts(x). The two differences:types: ["node"](suites read fixtures throughnode:fs/node:url/__dirname, and the ActionRunner suites stubglobal.fetch);libstays at the root'sES2020/DOM/DOM.Iterable.lib: ["ES2022", "DOM", "DOM.Iterable"]andtypes: ["node", "vite/client", "@testing-library/jest-dom"], exactly the recipe recorded on the card at tranche 4.jsx: react-jsxfor the 212.tsxsuites.Notably NOT added:
vitest/globals. No graduated package in this repo carries it, and the five core files that usedviwithout importing it are outliers — they got the import their siblings all have.Source changes — three declarations narrower than their implementation
Only two source files move, both in
app-shell, and each is a declaration the compile errors proved wrong rather than a behaviour change.1.
ConsoleActionRuntime.actionProviderProps— restatement → derivation (the item the dispatch flagged; it did surface). The interface hand-restated ActionProvider's props and had drifted twice: it omittedonModal, which the implementation has returned all along, and declaredhandlersvalues as one-parameter functions where ActionProvider passes(action, ctx). The neighbouring suite assertstypeof props.onModal === 'function'— it was reading a key the interface said did not exist. Now:The key list stays explicit — it states which props this hook owns — but every type comes from the consumer, so the two cannot drift again.
2.
ConsoleActionRuntime.apiHandlerdeclares its second parameter. The implementation has always been(action, context?)(it readscontext.pageVariablesto resolve{{page.< var >}}tokens), and its three siblings in the same interface already spelled that. Two tests call it with two arguments.3.
AiChatPageimports the chat-message type it actually produces.@object-ui/plugin-chatbotexports TWOChatMessageinterfaces — the enhanced one (asChatbotEnhancedMessage) and a minimal legacy one (id/role/content/…) from its own barrel module. This file imported the legacy one while producing and consuming the enhanced shape; the mismatch compiled only because every construction site spreads the extra keys conditionally (...(x ? { toolInvocations } : {})), which defeats excess-property checking. SohydratedMessagesToChatMessagesreturned a type narrower than every value flowing through it, and its own test could not readtoolInvocationsoff it.Test-side fixes worth naming
TransactionManager.test.ts(17 of 56). The suite importedActionSchemafrom@object-ui/types, which resolves to the deprecatedcrud.tsone (type: 'action', optionalname) — whileexecuteTransactiontakes aUIActionSchemaexecutor. Every fixture was authored in a vocabulary the implementation never sees. Re-pointed atUIActionSchema; theTS2538"undefined cannot be used as an index type" fell out with it, sincenameis required there.schema-builder.test.ts. Column fixtures were{ name, label }— a dialectTableColumndoes not have (its keys areaccessorKey/header). The builder stores whatever it is handed, so the assertion was checking that the builder returns its own input, on a shape no CRUD renderer reads. Re-spelled canonically. Separately,operations.< key >isboolean | CRUDOperation, so.enabledneeded a narrowing helper that throws if the object form ever stops being written — not a cast.ActionRunner.test.ts. The "does not read the retiredexecutealias" case had a comment claimingActionDefis open-ended. It has since been CLOSED (objectstack#4075 step 3), so authoringexecuteis now a compile error too; marked@ts-expect-errorrather than cast, which pins the refusal in both directions.{...(props as never)}spreadnever(TS2698). All eight previews takeMetadataPreviewProps— the cast was pure noise that had never been checked. Replaced with the real props;PagePreview's six render sites gained the requiredtype/name.ReturnType< typeof vi.fn >resolves to the un-instantiatedMock< Procedure | Constructable >, and a zero-arityvi.fn(async () => …)infersMock< () => … >whosemock.callsis the EMPTY tuple — so a good number ofcalls[0][0]assertions were reading element 0 of an empty tuple as far as the compiler was concerned. Each is now typed with the signature the slot it feeds declares (the convention tranche 4 established).deriveBoundPackageId.test.ts. Fixtures were built asChatMessageand cast — and the legacyChatMessagehas NO properties in common with the structural minimum the function declares, so the cast was asserting between unrelated shapes. Now derived:Parameters< typeof deriveBoundPackageId >[0][number].FavoritesProvider.test.tsx.TS2578"unused@ts-expect-error": onenull as anyelement collapsed the whole array literal's element type toany, so the directive documenting a malformed item suppressed nothing. Narrowed the escape hatch toas unknown as FavoriteItem; the suppression is load-bearing again.ContextSelectors.scopeKey.test.tsx. Imported@object-ui/layout/NavigationRenderer— a subpath that resolves through noexportsentry (the package declares only.).resolveHrefis re-exported from the barrel, so this is the same symbol by its public name.locale="en"whereSupportedLocaleis'en-US' | 'zh-CN'.Discrimination proof (both packages)
Per #4291's ratchet, retiring a narrow project requires showing the full project really covers it.
--listFileson each test project, written to a file:dataset-result-field-spec-parity.test.ts,actionDef-closed-surface.test.ts), 2 of 576.spec-symbol-parity.test.ts,resolveActionParams.test.ts,InspectorComboField.naming.types.test.tsx,flow-designer-edge.types.test.ts,flow-sim-edge.types.test.ts), of 3624.Then a provably-false
Assert< Equal< 1, 2 > >appended to each package's parity test, and the FULL project turns red:Both probes removed afterwards (
git diffon those two files is empty in this PR).app-shellalso carried atype-check:typetestsscript that nothing else in the repo referenced; it is removed with the project it ran.For whoever resolves a merge conflict here
Two files in this PR are conflict-prone, and they resolve by different rules:
scripts/check-type-check-coverage.mjs(theTEST_DEBTregistry) — take main's registry, then re-apply the deletion of@object-ui/coreand@object-ui/app-shell. Drop BOTH conflict sides of those lines; do not keep either. Then runnode scripts/check-type-check-coverage.mjs— it must be green with both lines gone.@object-ui/plugin-dashboard's line must SURVIVE.scripts/__tests__/check-type-check-coverage.test.ts(the namedretiredlist) — resolve by UNION: keep both sides' package names,@object-ui/coreand@object-ui/app-shellincluded. Its neighbouring terminal-state case (toEqual([])) is NOT a union merge — take this side, since the set really is empty once this lands.Reminder from tranches 1–4: GitHub silently drops auto-merge when a PR becomes conflicting, with no notification. Re-arm it after resolving.
Verification
All green. Local runs are repo-root vitest with paths relative to the repo root (never
pnpm --filter, never acdinto the package — both of those silently run somebody else's suite), batched under one acquisition of the shared verification lock.CI on this head: all 20 checks completed, every one success or skipped — Type Check, Lint, Test (shard 1–4/4) and Build & E2E all
success.The gate's own self-test needed a second commit — its premise expired here
scripts/__tests__/check-type-check-coverage.test.tsassertedwithNarrow.length > 0, under a comment saying survivors were guaranteed while #4040 burned down. Tranche 5 is the point where that stops being true: retiring these two narrow projects takes the last ones, so the assertion was false by construction and the local run caught it red.Rewritten to the terminal statement,
expect(withNarrow.map(p => p.name)).toEqual([]), which is not vacuous in the direction that matters — atsconfig.typetests.jsonreappearing anywhere turns it red, i.e. #4291's ratchet stated as a test rather than only as a gate rule.@object-ui/coreand@object-ui/app-shelljoin the namedretiredlist, the case that keeps carrying real weight. The sibling "holds each survivor" case is vacuous today by design and says so; it re-arms the moment the set is non-empty.Reverse verification of the registry deletion
Driven through the gate's exported
auditPackages()rather than by editing the file, so it could not race the batched gate run:core+app-shellre-added toTEST_DEBTcorekeeps a narrow typetests projectplugin-dashboard's surviving entry deletedtscinvocation reads"The last row matters: it shows the control's green is not vacuous — the one entry this PR leaves behind is still load-bearing.
And for the rewritten self-test assertion, the same treatment with a synthetic narrow project injected into
collect()'s output:Changeset:
.changeset/core-app-shell-type-check-their-tests.md, empty frontmatter (no published behaviour moves), nevermajor.Refs #4040, #4291, #3181, #3009, #3968.
Generated by Claude Code