fix(plugin-grid,plugin-form,cli,+2): type-check the last five unchecked packages, and fix the two runtime bugs hiding there (#2919) (#2936)

os-zhuang · claude · web-flow · commit dc7a79867c56 · 2026-07-29T00:41:49.000+08:00
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: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com>
diff --git a/.changeset/typecheck-debt-2919.md b/.changeset/typecheck-debt-2919.md
@@ -0,0 +1,79 @@
+---
+"@object-ui/plugin-grid": patch
+"@object-ui/plugin-form": patch
+"@object-ui/plugin-designer": patch
+"@object-ui/cli": patch
+"object-ui": patch
+---
+
+fix(plugin-grid,plugin-form,plugin-designer,cli,vscode-extension): type-check the last five unchecked packages, and fix the two runtime bugs that hid there (#2919)
+
+Closes the remaining `DEBT` entries from the #2911 sweep. Each package gains
+`"type-check": "tsc --noEmit"` and loses its entry in
+`scripts/check-type-check-coverage.mjs`; coverage goes 36 -> 41 of 45 and
+outstanding errors 25 -> 5 (only #2916 `plugin-view` and #2918 `layout` remain).
+
+**Two of these were real bugs, not just type noise.**
+
+`@object-ui/cli` — `objectui validate` could never report a validation failure.
+`ZodError.errors` was removed in Zod 4 (the repo is on 4.4.3), so `.errors` read
+`undefined` and `.forEach` threw a `TypeError` that the enclosing `catch`
+reported as `✗ Error reading or parsing schema file: Cannot read properties of
+undefined` — swallowing the very errors the command exists to print. Now reads
+`.issues`. Verified against the built CLI: an invalid schema now prints
+`1. Invalid input / Code: invalid_union` and exits 1.
+
+`@object-ui/plugin-grid` — grouping a grid by a boolean column showed the raw
+i18n key. `t('grid.booleanTrue', 'Yes')` asked for a key present in neither
+`GRID_DEFAULT_TRANSLATIONS` nor any locale bundle, and passed the English
+fallback as a bare second argument — which `createSafeTranslation`'s no-provider
+translator reads as an *options object*, so the fallback never applied and the
+header rendered the literal `grid.booleanTrue`. Switched to the `grid.yes` /
+`grid.no` keys the boolean cell renderer (`ObjectGrid.tsx`) and
+`BulkActionDialog` already use, with the fallback passed as `defaultValue`.
+Covered by a new regression test, confirmed to fail against the old code.
+
+The rest are type-only corrections that preserve runtime behaviour exactly:
+
+- **plugin-grid** `importParsers.ts` — `scorePair`'s `score`/`reason` moved into
+  one `best` record. They were captured `let`s mutated only inside the `bump`
+  closure, which TypeScript's control-flow analysis does not track, so it still
+  believed `reason` was `'none'` at the type gate and flagged the comparisons as
+  non-overlapping (TS2367). The gate — which stops a text column being mapped
+  onto a number field — is unchanged; its two dedicated tests still pass.
+- **plugin-form** — `SectionFieldsContext.fieldLabel` now requires `fallback`,
+  matching the `useSafeFieldLabel` producer in `@object-ui/i18n` (an omitted
+  fallback could not satisfy the `=> string` return, and all four call sites
+  already pass one). This one signature cleared six errors.
+  `MasterDetailFormSchema.recordId` widens to `string | number`, matching
+  `ObjectFormSchema` and the five envelopes that forward straight into it;
+  it is narrowed with `String()` only at the batch-transaction boundary, whose
+  `BatchTransactionOperation.id` is a string by protocol (the `isEdit` guard
+  already proves it non-null there). `deriveMasterDetail`'s column sort gets an
+  explicit `fillPriority` helper — `GridColumn.type` is optional, and a column
+  without one keeps sorting at priority 5 exactly as the old
+  `TYPE_FILL_PRIORITY[undefined] ?? 5` lookup put it.
+- **plugin-designer** — unused `index` parameter prefixed `_`, matching the
+  `_entry` beside it.
+- **cli** — a stale `@ts-expect-error` removed; `viteConfig` is typed `any`, so
+  the line it guarded had stopped erroring.
+- **vscode-extension** (`object-ui`) — migrated off `moduleResolution: "node"`,
+  which is deprecated and stops working in TypeScript 7, to `node16` paired with
+  `module: "node16"` (the package has no `"type": "module"`, so node16 resolves
+  it as the CommonJS that tsup emits, and it gains the `exports`-map awareness
+  node10 lacks). Its error count was under-reported as 1: that TS5107 config
+  error masked four more. The package uses `console`/`Buffer` but sets
+  `lib: ["ES2020"]` with no DOM and never declared `@types/node` — added, with an
+  explicit `types: ["node", "vscode"]`.
+
+Also: `plugin-grid`, `plugin-form` and `plugin-designer` gain the `baseUrl` +
+`paths` override their type-checked plugin peers already carry, and `cli` an
+empty `paths`. Without it the inherited root `paths` point `@object-ui/*` at
+sibling `src/`, which is outside each project's `rootDir` and produces the ~104
+spurious TS6059 errors noted in #2915; workspace deps instead resolve through
+node_modules to built `.d.ts`, which `type-check`'s `dependsOn: ["^build"]`
+guarantees exist.
+
+Verified the gate genuinely covers all five rather than trusting the green:
+injecting a type error into each package makes `pnpm type-check --filter <pkg>`
+fail, which was impossible before this change.
diff --git a/packages/cli/package.json b/packages/cli/package.json
@@ -33,7 +33,8 @@
     "build": "tsup",
     "dev": "tsup --watch",
     "lint": "eslint src",
-    "test": "vitest run"
+    "test": "vitest run",
+    "type-check": "tsc --noEmit"
   },
   "keywords": [
     "objectstack",
diff --git a/packages/cli/src/commands/dev.ts b/packages/cli/src/commands/dev.ts
@@ -197,7 +197,6 @@ export async function dev(schemaPath: string, options: DevOptions) {
       // We might get the cjs entry, but for aliasing usually fine. 
       // Better yet, if we can find the package root, but require.resolve gives file.
       // Let's just use what require.resolve gives.
-      // @ts-expect-error - lucidePath is dynamically resolved
       viteConfig.resolve.alias['lucide-react'] = lucidePath;
     } catch (e) {
       console.warn('⚠️ Could not resolve lucide-react automatically:', e);
diff --git a/packages/cli/src/commands/validate.ts b/packages/cli/src/commands/validate.ts
@@ -83,15 +83,18 @@ export async function validate(schemaPath: string) {
       console.error(chalk.red('✗ Schema validation failed!\n'));
       console.error(chalk.bold('Validation Errors:'));
       
-      // Format Zod errors nicely
-      const errors = result.error.errors;
-      errors.forEach((error, index) => {
-        console.error(chalk.red(`\n${index + 1}. ${error.message}`));
-        if (error.path && error.path.length > 0) {
-          console.error(chalk.gray(`   Path: ${error.path.join(' → ')}`));
+      // Format Zod errors nicely. `.issues` is the only accessor on a Zod 4
+      // ZodError — the `.errors` alias was removed, so reading it yielded
+      // undefined and this loop threw a TypeError that the catch below
+      // reported as "Error reading or parsing schema file", hiding the very
+      // errors this command exists to print.
+      result.error.issues.forEach((issue, index) => {
+        console.error(chalk.red(`\n${index + 1}. ${issue.message}`));
+        if (issue.path && issue.path.length > 0) {
+          console.error(chalk.gray(`   Path: ${issue.path.join(' → ')}`));
         }
-        if (error.code) {
-          console.error(chalk.gray(`   Code: ${error.code}`));
+        if (issue.code) {
+          console.error(chalk.gray(`   Code: ${issue.code}`));
         }
       });
       
diff --git a/packages/cli/tsconfig.json b/packages/cli/tsconfig.json
@@ -7,7 +7,13 @@
     "target": "ES2020",
     "lib": ["ES2020"],
     "moduleResolution": "bundler",
-    "resolveJsonModule": true
+    "resolveJsonModule": true,
+    "baseUrl": ".",
+    // Drop the repo-root `paths` (which point `@object-ui/*` at sibling
+    // `src/`): with `rootDir: ./src` those sources are outside this project
+    // and every one of them reports TS6059. Workspace deps resolve through
+    // node_modules to their built types, which `type-check` depends on.
+    "paths": {}
   },
   "include": ["src/**/*"],
   "exclude": ["node_modules", "dist"]
diff --git a/packages/plugin-designer/package.json b/packages/plugin-designer/package.json
@@ -24,6 +24,7 @@
     "build": "vite build",
     "clean": "rm -rf dist",
     "test": "vitest run",
+    "type-check": "tsc --noEmit",
     "lint": "eslint ."
   },
   "peerDependencies": {
diff --git a/packages/plugin-designer/src/components/HistoryPanel.tsx b/packages/plugin-designer/src/components/HistoryPanel.tsx
@@ -54,7 +54,7 @@ export function HistoryPanel<T>({
   const { timeline, currentIndex, undo, redo, jumpTo, canUndo, canRedo } = history;
 
   const defaultRenderLabel = React.useCallback(
-    (_entry: T, index: number, position: number) => {
+    (_entry: T, _index: number, position: number) => {
       if (position === 0) return 'Current';
       if (position < 0) return `Earlier (${Math.abs(position)} step${Math.abs(position) > 1 ? 's' : ''} back)`;
       return `Later (${position} step${position > 1 ? 's' : ''} forward)`;
diff --git a/packages/plugin-designer/tsconfig.json b/packages/plugin-designer/tsconfig.json
@@ -3,6 +3,14 @@
   "compilerOptions": {
     "outDir": "dist",
     "jsx": "react-jsx",
+    "baseUrl": ".",
+    "paths": {
+      "@/*": ["src/*"]
+    },
+    "noEmit": false,
+    "declaration": true,
+    "composite": true,
+    "declarationMap": true,
     "skipLibCheck": true
   },
   "include": ["src"],
diff --git a/packages/plugin-form/package.json b/packages/plugin-form/package.json
@@ -17,6 +17,7 @@
   "scripts": {
     "build": "vite build",
     "test": "vitest run",
+    "type-check": "tsc --noEmit",
     "lint": "eslint ."
   },
   "dependencies": {
diff --git a/packages/plugin-form/src/MasterDetailForm.tsx b/packages/plugin-form/src/MasterDetailForm.tsx
@@ -75,7 +75,14 @@ export interface MasterDetailFormSchema {
   /** Parent object name, e.g. 'expense_claim'. */
   objectName: string;
   mode?: 'create' | 'edit';
-  recordId?: string;
+  /**
+   * `string | number` to match `ObjectFormSchema` and the drawer/modal/split/
+   * tabbed/wizard envelopes that hand a record straight through to this form —
+   * a numeric primary key is a real backend shape. Narrowed to a string only at
+   * the batch-transaction boundary, whose `BatchTransactionOperation.id` is a
+   * string by protocol.
+   */
+  recordId?: string | number;
   /** Prefilled parent header values (create mode) — seeds the parent form's
    *  initial values, e.g. a conversion wizard carrying the lead/account over. */
   initialValues?: Record<string, any>;
@@ -554,7 +561,7 @@ export const MasterDetailForm: React.FC<MasterDetailFormProps> = ({
       const ops = isEdit
         ? buildMasterDetailEditBatch(
             schema.objectName,
-            schema.recordId!,
+            String(schema.recordId),
             parentData,
             details.filter((d) => d.relationshipField).map((d, i) => ({
               childObject: d.childObject,
diff --git a/packages/plugin-form/src/deriveMasterDetail.ts b/packages/plugin-form/src/deriveMasterDetail.ts
@@ -149,6 +149,15 @@ const TYPE_FILL_PRIORITY: Record<string, number> = {
   text: 4,
 };
 
+/**
+ * Fill priority for a column. `GridColumn.type` is optional, and a column
+ * without one sorts with the unknown types at the back of the budget — the
+ * same place the bare `TYPE_FILL_PRIORITY[undefined] ?? 5` lookup put it.
+ */
+function fillPriority(col: GridColumn): number {
+  return (col.type ? TYPE_FILL_PRIORITY[col.type] : undefined) ?? 5;
+}
+
 /**
  * Choose the default-visible subset of `max` columns — always keeping the
  * primary (name-like) column and every required column, then filling the
@@ -166,7 +175,7 @@ function curateColumns(cols: GridColumn[], max: number): GridColumn[] {
   const remaining = cols
     .map((c, i) => ({ c, i }))
     .filter(({ c }) => !visible.has(c.field))
-    .sort((a, b) => (TYPE_FILL_PRIORITY[a.c.type] ?? 5) - (TYPE_FILL_PRIORITY[b.c.type] ?? 5) || a.i - b.i);
+    .sort((a, b) => fillPriority(a.c) - fillPriority(b.c) || a.i - b.i);
   for (const { c } of remaining) {
     if (visible.size >= max) break;
     visible.add(c.field);
diff --git a/packages/plugin-form/src/sectionFields.ts b/packages/plugin-form/src/sectionFields.ts
@@ -37,8 +37,15 @@ export interface SectionFieldsContext {
   readOnly?: boolean;
   /** Form mode — `view` forces every field disabled. */
   mode?: 'create' | 'edit' | 'view';
-  /** Translation-aware label resolver (from `useSafeFieldLabel`). */
-  fieldLabel: (objectName: string, fieldName: string, fallback?: string) => string;
+  /**
+   * Translation-aware label resolver (from `useSafeFieldLabel`).
+   *
+   * `fallback` is required, matching the producer in `@object-ui/i18n`: the
+   * resolver returns the fallback when no translation exists, so an omitted
+   * one could not satisfy the `=> string` return. Every call site already
+   * passes it.
+   */
+  fieldLabel: (objectName: string, fieldName: string, fallback: string) => string;
 }
 
 /**
diff --git a/packages/plugin-form/tsconfig.json b/packages/plugin-form/tsconfig.json
@@ -2,7 +2,16 @@
   "extends": "../../tsconfig.json",
   "compilerOptions": {
     "outDir": "dist",
-    "jsx": "react-jsx"
+    "jsx": "react-jsx",
+    "baseUrl": ".",
+    "paths": {
+      "@/*": ["src/*"]
+    },
+    "noEmit": false,
+    "declaration": true,
+    "composite": true,
+    "declarationMap": true,
+    "skipLibCheck": true
   },
   "include": ["src"],
   "exclude": ["node_modules", "dist", "**/*.test.ts", "**/*.test.tsx"]
diff --git a/packages/plugin-grid/package.json b/packages/plugin-grid/package.json
@@ -17,6 +17,7 @@
   "scripts": {
     "build": "vite build",
     "test": "vitest run",
+    "type-check": "tsc --noEmit",
     "lint": "eslint ."
   },
   "dependencies": {
diff --git a/packages/plugin-grid/src/ObjectGrid.tsx b/packages/plugin-grid/src/ObjectGrid.tsx
@@ -672,10 +672,13 @@ export const ObjectGrid: React.FC<ObjectGridProps> = ({
         if (label !== undefined) return label;
       }
       // Boolean fields: render as Yes/No. We use the toolbar i18n bundle so
-      // grids without an objectName still produce a readable label.
+      // grids without an objectName still produce a readable label — the same
+      // `grid.yes`/`grid.no` keys the boolean cell renderer and the bulk-action
+      // dialog use, passed as `defaultValue` (a bare string second argument is
+      // read as an options object, so the fallback never applied).
       if (meta.type === 'boolean' || typeof value === 'boolean') {
-        if (value === true || value === 'true') return t('grid.booleanTrue', 'Yes');
-        if (value === false || value === 'false') return t('grid.booleanFalse', 'No');
+        if (value === true || value === 'true') return t('grid.yes', { defaultValue: 'Yes' });
+        if (value === false || value === 'false') return t('grid.no', { defaultValue: 'No' });
       }
       return undefined;
     };
diff --git a/packages/plugin-grid/src/__tests__/groupedBooleanLabel.test.tsx b/packages/plugin-grid/src/__tests__/groupedBooleanLabel.test.tsx
@@ -0,0 +1,65 @@
+/**
+ * Grouped boolean headers fall back to readable Yes/No.
+ *
+ * Regression coverage for the group header rendering the raw i18n key
+ * (`grid.booleanTrue`) instead of a label: the formatter asked for keys that
+ * exist in neither the grid's default bundle nor the locale files, and passed
+ * the English fallback as a bare second argument — which the no-provider
+ * translator reads as an options object, so the fallback never applied.
+ *
+ * These grids render with no `I18nProvider`, which is exactly the path the
+ * defaults map exists to serve (standalone usage / tests).
+ */
+import { describe, it, expect } from 'vitest';
+import { render, waitFor } from '@testing-library/react';
+import '@testing-library/jest-dom';
+import React from 'react';
+
+import { ObjectGrid } from '../ObjectGrid';
+import { registerAllFields } from '@object-ui/fields';
+import { ActionProvider } from '@object-ui/react';
+
+registerAllFields();
+
+const rows = [
+  { id: '1', name: 'Row 1', active: true },
+  { id: '2', name: 'Row 2', active: false },
+  { id: '3', name: 'Row 3', active: true },
+];
+
+function renderBooleanGroupedGrid() {
+  const schema: any = {
+    type: 'object-grid' as const,
+    objectName: 'test_object',
+    columns: [
+      { field: 'name', label: 'Name' },
+      { field: 'active', label: 'Active', type: 'boolean' },
+    ],
+    data: { provider: 'value', items: rows },
+    grouping: { fields: [{ field: 'active' }] },
+  };
+  return render(
+    <ActionProvider>
+      <ObjectGrid schema={schema} />
+    </ActionProvider>
+  );
+}
+
+const groupLabels = () =>
+  Array.from(document.querySelectorAll('.group-label')).map((el) => el.textContent);
+
+describe('Grouped boolean group headers', () => {
+  it('labels the groups Yes / No', async () => {
+    renderBooleanGroupedGrid();
+    await waitFor(() => expect(groupLabels().length).toBeGreaterThan(0));
+    expect(groupLabels()).toEqual(expect.arrayContaining(['Yes', 'No']));
+  });
+
+  it('never leaks a raw `grid.*` translation key into a group header', async () => {
+    renderBooleanGroupedGrid();
+    await waitFor(() => expect(groupLabels().length).toBeGreaterThan(0));
+    for (const label of groupLabels()) {
+      expect(label).not.toMatch(/^grid\./);
+    }
+  });
+});
diff --git a/packages/plugin-grid/src/importParsers.ts b/packages/plugin-grid/src/importParsers.ts
@@ -324,9 +324,12 @@ function scorePair(header: string, field: MappableField, inferred: InferredType)
   const targets = [field.name, field.label].filter((s): s is string => !!s);
   const hNorm = normalizeKey(header);
   const hTokens = tokenize(header);
-  let score = 0;
-  let reason: MappingReason = 'none';
-  const bump = (s: number, r: MappingReason) => { if (s > score) { score = s; reason = r; } };
+  // Held as one record rather than two captured `let`s: `bump` only ever
+  // mutates them from inside a closure, which TypeScript's control-flow
+  // analysis does not track — it would still believe `reason` is `'none'` at
+  // the type gate below and report those comparisons as non-overlapping.
+  const best: { score: number; reason: MappingReason } = { score: 0, reason: 'none' };
+  const bump = (s: number, r: MappingReason) => { if (s > best.score) { best.score = s; best.reason = r; } };
 
   for (const target of targets) {
     if (header.trim() === target.trim()) { bump(1, 'exact'); continue; }
@@ -349,11 +352,11 @@ function scorePair(header: string, field: MappableField, inferred: InferredType)
   // Type gate: for softer (non-exact) matches, reward a compatible inferred
   // type and heavily discount an incompatible one so we don't confidently map
   // a text column onto a number field just because the names rhyme.
-  if (reason !== 'exact' && reason !== 'normalized' && inferred !== 'text' && score > 0) {
-    if (isTypeCompatible(inferred, field.type)) score = Math.min(1, score + 0.05);
-    else score *= 0.5;
+  if (best.reason !== 'exact' && best.reason !== 'normalized' && inferred !== 'text' && best.score > 0) {
+    if (isTypeCompatible(inferred, field.type)) best.score = Math.min(1, best.score + 0.05);
+    else best.score *= 0.5;
   }
-  return { score, reason };
+  return { score: best.score, reason: best.reason };
 }
 
 /**
diff --git a/packages/plugin-grid/tsconfig.json b/packages/plugin-grid/tsconfig.json
diff --git a/packages/vscode-extension/package.json b/packages/vscode-extension/package.json
diff --git a/packages/vscode-extension/tsconfig.json b/packages/vscode-extension/tsconfig.json
diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml
diff --git a/scripts/check-type-check-coverage.mjs b/scripts/check-type-check-coverage.mjs
