fix(plugin-grid,plugin-form,cli,+2): type-check the last five unchecked packages, and fix the two runtime bugs hiding there (#2919) - #2936
Merged
Conversation
…ed packages, and fix the two runtime bugs hiding there (#2919) Closes the remaining `DEBT` entries from the #2911 sweep. Each package gains `"type-check": "tsc --noEmit"` and loses its entry in the coverage guard; coverage goes 36 -> 41 of 45, outstanding errors 25 -> 5 (only #2916 and #2918 remain). Two were real bugs, not type noise: - cli: `objectui validate` could never report a validation failure. Zod 4 removed `ZodError.errors`, so it read undefined and `.forEach` threw a TypeError that the enclosing catch reported as "Error reading or parsing schema file" — swallowing the errors the command exists to print. Now reads `.issues`. - plugin-grid: grouping by a boolean column rendered the raw key `grid.booleanTrue`. That key exists in no bundle, and the English fallback was passed as a bare second argument, which the no-provider translator reads as an options object. Switched to the `grid.yes`/`grid.no` keys the cell renderer and BulkActionDialog already use, with `defaultValue`. Covered by a regression test confirmed to fail against the old code. The rest are type-only and preserve runtime behaviour: `scorePair`'s closure-mutated `let`s folded into one record so the TS2367 type gate narrows correctly (gate unchanged), `SectionFieldsContext.fieldLabel` aligned with its `@object-ui/i18n` producer (cleared six errors), `MasterDetailFormSchema.recordId` widened to `string | number` with `String()` only at the batch-transaction boundary, an explicit `fillPriority` for the optional `GridColumn.type`, an unused parameter prefixed `_`, and a stale `@ts-expect-error` dropped. vscode-extension migrates off the deprecated node10 `moduleResolution: "node"` to `node16`; its count was under-reported as 1 because that TS5107 config error masked four missing Node globals — `@types/node` added with an explicit `types`. plugin-grid/plugin-form/plugin-designer gain the `baseUrl` + `paths` override their type-checked peers carry (cli an empty `paths`), without which the inherited root `paths` produce the spurious TS6059 noise described in #2915. Verified by injecting a type error into each of the five: `pnpm type-check` now fails for every one, which was impossible before. Refs #2911, #2915 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 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 #2919. Clears the remaining
DEBTentries from the #2911 sweep:plugin-grid(4),plugin-form(10),cli(4),plugin-designer(1),object-ui(1).Each package gains
"type-check": "tsc --noEmit"and loses itsDEBTentry inscripts/check-type-check-coverage.mjs. Coverage 36 → 41 of 45; outstanding errors 25 → 5 — only #2916 (plugin-view, 3) and #2918 (layout, 2) remain.@object-ui/sitewas already out of scope per the #2924 measurement, and@object-ui/runnerwas already done in #2922.Two of these were real, user-visible bugs
objectui validatecould never report a validation failureZodError.errorswas removed in Zod 4 (this repo is on 4.4.3). Soresult.error.errorsreadundefined,.forEachthrew aTypeError, and the enclosingcatchreported it as✗ Error reading or parsing schema file:— swallowing the very errors the command exists to print. Now reads.issues.Verified against the built CLI on an invalid schema:
exit 1. Previously:
✗ Error reading or parsing schema file: Cannot read properties of undefined (reading 'forEach').Grouping a grid by a boolean column rendered a raw i18n key
t('grid.booleanTrue', 'Yes')asked for a key present in neitherGRID_DEFAULT_TRANSLATIONSnor any locale bundle, and passed the English fallback as a bare second argument — whichcreateSafeTranslation's no-provider translator reads as an options object, so the fallback never applied. Group headers rendered the literalgrid.booleanTrue/grid.booleanFalse.Switched to the
grid.yes/grid.nokeys that the boolean cell renderer (ObjectGrid.tsx:1999) andBulkActionDialog.tsx:215already use, with the fallback passed asdefaultValue. New regression test, confirmed to fail against the old code:Type-only, runtime preserved
importParsers.ts—score/reasonfolded into onebestrecord. They were capturedlets mutated only inside thebumpclosure, which TS's control-flow analysis doesn't track, so it still believedreasonwas'none'and flagged the comparisons as non-overlapping (TS2367). The type gate itself is unchanged — as plugin-grid / plugin-form / plugin-designer / cli / object-ui: remaining type-check debt from the #2911 sweep #2919 warned, deleting the branch would silently disable the guard that stops a text column being mapped onto a number field. Its two dedicated tests (uses content type to gate a fuzzy match,discounts a name match when the content type is incompatible) still pass.SectionFieldsContext.fieldLabelnow requiresfallback, matching theuseSafeFieldLabelproducer in@object-ui/i18n; an omitted fallback could not satisfy the=> stringreturn, and all four call sites already pass one. One signature cleared six errors.MasterDetailFormSchema.recordIdwidens tostring | numberto matchObjectFormSchemaand the five envelopes that forward into it, narrowed withString()only at the batch boundary whereBatchTransactionOperation.idis a string by protocol (theisEditguard already proves it non-null).deriveMasterDetailgets an explicitfillPriorityhelper —GridColumn.typeis optional, and a typeless column keeps sorting at priority 5 exactly asTYPE_FILL_PRIORITY[undefined] ?? 5put it.indexparameter prefixed_, matching the_entrybeside it.@ts-expect-errorremoved;viteConfigis typedany, so the line it guarded had stopped erroring.object-ui(vscode-extension) was under-reportedIts count of 1 was wrong: the TS5107 config error masked four more. Confirmed by A/B — the extra errors reproduce under the old
moduleResolution: "node"too.Migrated to
node16+module: "node16"(the real fix, notignoreDeprecations): no"type": "module", so node16 resolves it as the CommonJS tsup emits, and it gains theexports-map awareness node10 lacks. The package usesconsole/Bufferbut setslib: ["ES2020"]with no DOM and never declared@types/node— added, with an explicittypes: ["node", "vscode"].tsconfig
pathsplugin-grid/plugin-form/plugin-designergain thebaseUrl+pathsoverride their type-checked plugin peers already carry;cligets an emptypaths. Without it the inherited rootpathspoint@object-ui/*at siblingsrc/, outside each project'srootDir, producing the ~104 spurious TS6059 errors described in #2915. Workspace deps instead resolve through node_modules to built.d.ts, whichtype-check'sdependsOn: ["^build"]guarantees exist.Verification
node scripts/check-type-check-coverage.mjs→✅ 41/45 via type-check, 1 via their own build, 2 known-broken (5 errors outstanding), 1 not compiledpnpm type-check→ 76/76 tasks successfulpnpm build→ 43/43 successful;dist/index.d.tsintact for all three packages that gainedcompositepnpm exec vitest runover the five packages → 63 files, 574 tests passedeslint→ 0 errors in all fivepnpm type-check --filter <pkg>fail. All five ✅; impossible before this change.Refs #2911, #2915