Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 79 additions & 0 deletions .changeset/typecheck-debt-2919.md
Original file line number Diff line number Diff line change
@@ -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.
3 changes: 2 additions & 1 deletion packages/cli/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@
"build": "tsup",
"dev": "tsup --watch",
"lint": "eslint src",
"test": "vitest run"
"test": "vitest run",
"type-check": "tsc --noEmit"
},
"keywords": [
"objectstack",
Expand Down
1 change: 0 additions & 1 deletion packages/cli/src/commands/dev.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
19 changes: 11 additions & 8 deletions packages/cli/src/commands/validate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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}`));
}
});

Expand Down
8 changes: 7 additions & 1 deletion packages/cli/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
Expand Down
1 change: 1 addition & 0 deletions packages/plugin-designer/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
"build": "vite build",
"clean": "rm -rf dist",
"test": "vitest run",
"type-check": "tsc --noEmit",
"lint": "eslint ."
},
"peerDependencies": {
Expand Down
2 changes: 1 addition & 1 deletion packages/plugin-designer/src/components/HistoryPanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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)`;
Expand Down
8 changes: 8 additions & 0 deletions packages/plugin-designer/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"],
Expand Down
1 change: 1 addition & 0 deletions packages/plugin-form/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
"scripts": {
"build": "vite build",
"test": "vitest run",
"type-check": "tsc --noEmit",
"lint": "eslint ."
},
"dependencies": {
Expand Down
11 changes: 9 additions & 2 deletions packages/plugin-form/src/MasterDetailForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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>;
Expand Down Expand Up @@ -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,
Expand Down
11 changes: 10 additions & 1 deletion packages/plugin-form/src/deriveMasterDetail.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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);
Expand Down
11 changes: 9 additions & 2 deletions packages/plugin-form/src/sectionFields.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

/**
Expand Down
11 changes: 10 additions & 1 deletion packages/plugin-form/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
Expand Down
1 change: 1 addition & 0 deletions packages/plugin-grid/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
"scripts": {
"build": "vite build",
"test": "vitest run",
"type-check": "tsc --noEmit",
"lint": "eslint ."
},
"dependencies": {
Expand Down
9 changes: 6 additions & 3 deletions packages/plugin-grid/src/ObjectGrid.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};
Expand Down
65 changes: 65 additions & 0 deletions packages/plugin-grid/src/__tests__/groupedBooleanLabel.test.tsx
Original file line number Diff line number Diff line change
@@ -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\./);
}
});
});
17 changes: 10 additions & 7 deletions packages/plugin-grid/src/importParsers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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; }
Expand All @@ -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 };
}

/**
Expand Down
Loading
Loading