fix(runner): type-check the package at all, fix the hidden DataSource violation (#2917) - #2922
Merged
Merged
Conversation
… violation (#2917) @object-ui/runner was the worst-covered package in the repo: `vite build` (transpile only), no type-check script, and uniquely no tsconfig.json at all. Nothing had ever type-checked it, despite it being published. It was NOT broken at runtime — #2917's own risk assessment was wrong and is corrected there. The two bad imports were `import type`, so they were erased before they could fail; the one value import (emulateBatchTransaction) does exist; and MockDataSource is unreferenced anywhere in the repo. What the missing check hid: DataSource and BatchTransactionOperation were imported from @object-ui/core, which does not export them (they live in @object-ui/types). Because that import never resolved, `implements DataSource` was silently a no-op, and three commits maintained the class as if it were verified (62b9ab5, 09d9669, 5527388). A real contract violation survived all three: async find(resource: string, params?: any): Promise<any[]> { return []; } DataSource.find returns a QueryResult envelope, not a bare array. The file's doc comment invites using it as the starting point for a real adapter, so anyone who did would hand every consumer an array whose .data/.total are undefined. - tsconfig.json added, mirroring apps/console (a Vite app) rather than a library: bundler resolution, allowImportingTsExtensions for ./App.tsx, and types: ["vite/client"] for import.meta.glob and the ./index.css import. Standalone, so the root `paths` are never inherited and TS6059 cannot arise. - find() now returns Promise<QueryResult> as { data: [], total: 0 }. - 6 unused params prefixed with _; unused Circle icon dropped from LayoutRenderer. - type-check script added; DEBT entry deleted. Coverage 35 -> 36 of 45, outstanding errors 46 -> 32. With a tsconfig the real count is 10, not 14 — four were artifacts of having none. Proven the gate covers it rather than trusting the green: injecting an error into runner/src/App.tsx yields `Failed: @object-ui/runner#type-check`, which was impossible before. Closes #2917 Co-Authored-By: Claude <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
This was referenced Jul 28, 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
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.
Closes #2917. First follow-up off the #2915 sweep, taking the highest-risk item first.
It is not broken at runtime — #2917's risk assessment was wrong
I wrote that issue, and its "likely broken at runtime" claim does not survive contact with the code. Corrected in a comment there:
import **type** { … }— erased before it can fail.emulateBatchTransaction, does exist (core/src/adapters/index.ts:16, re-exported viacore/src/index.ts:24).MockDataSourceis unreferenced anywhere in the repo;App.tsxhas no data-source wiring at all.So this is a correctness and reference-quality fix, not an outage.
What the missing type-check actually hid
DataSource/BatchTransactionOperationwere imported from@object-ui/core, which does not export them — they live in@object-ui/types. Because that import never resolved,class MockDataSource implements DataSourcewas silently a no-op.Three separate commits then maintained the class as if it were verified —
62b9ab510addedbatchTransaction,09d9669c7madegetObjectSchemarequired,5527388b0added input validation. With theimplementsclause inert, a real contract violation survived all three:DataSource.findreturns aQueryResultenvelope, not a bare array. TS says it plainly once the import is fixed:The file's doc comment invites you to use it as the starting point for your own adapter ("在真实项目中，你会在这里使用 fetch/axios 调用你的 API"), so anyone who did would hand every consumer an array whose
.dataand.totalareundefined. A wrong reference implementation is worse than none — that's the real defect.The tsconfig, and a second correction
#2917 said to mirror
packages/plugin-list. That's a library shape.runneris a Vite app, soapps/consoleis the right peer:bundlerresolution +allowImportingTsExtensions— forimport App from './App.tsx'types: ["vite/client"]— forimport.meta.globinMetadataLoaderand the./index.cssside-effect importpathsare never inherited, workspace deps resolve through built.d.ts, and the TS6059rootDirclass of error cannot arise at all. Nopathsoverride needed, unlike the library packages in fix(plugin-map): drop themaplibre-gl@6default import + gate type-check in CI (#2911) #2915.With a tsconfig in place the real error count is 10, not 14 — four of the original were artifacts of having none.
Changes
packages/runner/tsconfig.jsonmockDataSource.ts@object-ui/types;find()returnsPromise<QueryResult>as{ data: [], total: 0 }; 6 unused params prefixed_LayoutRenderer.tsxCircleicon import droppedpackages/runner/package.json"type-check": "tsc --noEmit"scripts/check-type-check-coverage.mjsrunnerDEBT entry deletedCoverage 35 → 36 of 45; outstanding errors 46 → 32.
Verification
Proven the gate genuinely covers the package rather than trusting the green — injecting a type error into
runner/src/App.tsx:That was impossible before this PR. Restored via
cp(notgit checkout, which would clobber uncommitted work) and confirmed no residual diff.pnpm type-check --force— 71/71, zero cached (70 before; runner's task is new)pnpm build— 43/43pnpm test— 7847 passed, 24 skipped, 663 files, 0 failuresnode scripts/check-type-check-coverage.mjs— green at 36/45No changeset entry for a behaviour change to consumers:
MockDataSourceis unimported, so it is tree-shaken out of the app bundle and no published artifact changes shape.🤖 Generated with Claude Code