Skip to content

Commit 785b8a5

Browse files
os-zhuangclaude
andauthored
fix(fields)!: FieldWidgetComponentProps stops claiming to have every key (#3221) (#3230)
`FieldWidgetComponentProps` ended in `[key: string]: any`. That is the objectstack#4075 mechanism: a type that claims to have every key can never be reported as missing one. Three consequences, all closed here: - `props.required` and `props.error` — both declared by the spec's `FieldWidgetPropsSchema`, neither declared here — were legal reads typed `any` and `undefined` at runtime forever; - a misspelled prop (`readOnly` for `readonly`) compiled and did nothing; - any structural/parity comparison against the type was useless in principle, which is why objectui#3161's batch-7 symbol guard was the only detector. The index signature is replaced by a closed set derived from the real call sites, not guessed: the controlled-input contract, the host plumbing the form renderer forwards (`schema`, `dataSource`, `dependentValues`, `dependsOn`, `emptyHint`, `compact`, `onSelectRecord`, `onCreateNew`), and DOM pass-through (`id`, `name`, `autoFocus`, `tabIndex`, focus/click handlers, `aria-*`, and `data-*` as a template-literal key so `keyof` stays finite). Every consumer in the monorepo compiles unchanged. Also read through the type instead of around it: ~20 `(props as any).x` reads of keys the type now declares. Leaving them would have kept the "a typo compiles" half of the defect alive at exactly the sites that matter. The three batch-7 tripwires written to go red on this change (`_IndexSignatureStillThere` / `_RequiredSilentlyReadsAsAny` / `_ErrorSilentlyReadsAsAny`) are replaced by their inverse, plus a new `__tests__/widget-props-contract.test.tsx` that pins the closed contract with `@ts-expect-error` and proves the pass-through behaviour still renders. Deliberately NOT resolved here: the `error` / `errorMessage` divergence (objectui#3222). This change only makes it visible to the compiler. Claude-Session: https://claude.ai/code/session_01PRJtkgUAaVG11FsJQbvZWA Co-authored-by: Claude <noreply@anthropic.com>
1 parent b06f78a commit 785b8a5

16 files changed

Lines changed: 337 additions & 56 deletions
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
---
2+
"@object-ui/fields": minor
3+
---
4+
5+
`FieldWidgetComponentProps` stops claiming to have every key (objectui#3221).
6+
7+
**Breaking for widget authors**: the exported `FieldWidgetComponentProps` no
8+
longer ends in `[key: string]: any`. FROM: any prop name at all type-checked and
9+
read as `any`. TO: the type declares a closed set — the controlled-input
10+
contract (`value` / `onChange` / `field` / `readonly` / `disabled` /
11+
`className` / `errorMessage` / `onUploadingChange`), the host plumbing a
12+
renderer forwards (`schema`, `dataSource`, `dependentValues`, `dependsOn`,
13+
`emptyHint`, `compact`, `onSelectRecord`, `onCreateNew`), and DOM pass-through
14+
(`id`, `name`, `autoFocus`, `tabIndex`, `onBlur`/`onFocus`/`onClick`, every
15+
`aria-*`, and `data-*` via a template-literal key). A custom widget reading
16+
anything else now fails `tsc`; the fix is to read it off `field` (the metadata)
17+
or to add the key here with its producer named.
18+
19+
Scored `minor`, not `major`, per this repo's fixed-group rule — objectui's major
20+
tracks `@objectstack`, so breaking changes of our own ship as minor with the
21+
semantics spelled out (see AGENTS.md §版本号策略). The practical blast radius is
22+
small: every call site in this monorepo — `plugin-detail`'s inline editor,
23+
`plugin-grid`'s cell editor, `app-shell`'s metadata inspectors, the form
24+
renderer — compiles unchanged, because the closed set was derived from them.
25+
26+
Why it mattered: an index signature is the objectstack#4075 mechanism — **a type
27+
that claims to have every key can never be reported as missing one**. Three
28+
things followed, and all three are now fixed:
29+
30+
- `props.required` and `props.error`, both declared by the spec's
31+
`FieldWidgetPropsSchema` and both absent here, were legal reads typed `any`
32+
and `undefined` at runtime forever. They are compile errors now, which is what
33+
makes the `error` / `errorMessage` divergence (objectui#3222) decidable by the
34+
compiler instead of by a symbol guard. This change deliberately does **not**
35+
resolve that divergence — only make it visible.
36+
- A misspelled prop (`readOnly` for `readonly`, `onchange` for `onChange`)
37+
compiled and silently did nothing.
38+
- Any structural / parity comparison against the type was useless *in
39+
principle*, which is why objectui#3161's batch-7 symbol guard was the only
40+
detector that could see the collision at all.
41+
42+
Also cleaned up inside the package: ~20 `(props as any).x` reads of keys the
43+
type now declares (`compact`, `dataSource`, `disabled`, `name`, `id`,
44+
`onCreateNew`, `onSelectRecord`, `contextRecord`, `dependentValues`) read
45+
through the type instead — leaving them would have kept the "a typo compiles"
46+
half of the defect alive at exactly the sites that matter. The three batch-7
47+
tripwires that existed to go red on this change
48+
(`_IndexSignatureStillThere` / `_RequiredSilentlyReadsAsAny` /
49+
`_ErrorSilentlyReadsAsAny`) are replaced by their inverse, so re-widening the
50+
type fails a test rather than passing one.

packages/fields/src/FieldEditWidget.tsx

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,8 @@ export function FieldEditWidget({
184184
const resolved = field?.type ? resolveInlineEditType(field.type) : undefined;
185185
const Widget = resolved ? EDIT_WIDGETS[resolved] : undefined;
186186
if (!Widget) return null;
187+
// `compact` is a declared widget prop (objectui#3221 closed this type), so
188+
// the spread no longer needs an `any` escape hatch to get past it.
187189
const compactProps = resolved && COMPACT_EDIT_TYPES.has(resolved) ? { compact: true } : {};
188-
return <Widget field={field} value={value} onChange={onChange} readonly={readonly} {...(compactProps as any)} />;
190+
return <Widget field={field} value={value} onChange={onChange} readonly={readonly} {...compactProps} />;
189191
}

packages/fields/src/__tests__/spec-symbol-batch7.test.ts

Lines changed: 21 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,9 @@
2222
* - `FieldWidgetProps` is now `FieldWidgetComponentProps`. The spec owns that
2323
* name for the DECLARED widget-plugin props contract; this package's is the
2424
* React interface its widgets implement. The pins record the divergence that
25-
* made re-exporting impossible, and the index signature that made the
26-
* divergence invisible.
25+
* made re-exporting impossible — and, since objectui#3221 closed the type,
26+
* that the divergence is now *visible* rather than swallowed by a
27+
* `[key: string]: any` index signature.
2728
*
2829
* The other direction — "the spec must not start owning the NEW names" — is not
2930
* asserted here because `scripts/check-spec-symbol-derivation.mjs` already
@@ -87,19 +88,24 @@ describe('FieldWidgetComponentProps is the RENDERED layer of the spec contract',
8788
type _SpecNamesItError = Assert<HasKey<SpecFieldWidgetProps, 'error'>>;
8889
type _LocalNamesItErrorMessage = Assert<HasKey<FieldWidgetComponentProps, 'errorMessage'>>;
8990

90-
// …and the reason nobody noticed. The index signature answers `any` for
91-
// every key this type does not declare, so BOTH of the spec's keys already
92-
// "exist" here — `props.required` and `props.error` are legal reads that
93-
// give `any` and are always `undefined` at runtime. No compiler check and
94-
// no structural comparison can report a key as missing from a type that
95-
// claims to have every key (objectstack#4075). That is what makes a parity
96-
// test useless for this symbol and the guard the only detector.
97-
//
98-
// These three assertions exist to be DELETED by objectui#3221, which
99-
// removes the index signature; when that lands they go red on purpose.
100-
type _IndexSignatureStillThere = Assert<Extends<string, keyof FieldWidgetComponentProps>>;
101-
type _RequiredSilentlyReadsAsAny = Assert<IsAny<FieldWidgetComponentProps['required']>>;
102-
type _ErrorSilentlyReadsAsAny = Assert<IsAny<FieldWidgetComponentProps['error']>>;
91+
// …and the reason nobody noticed used to sit right here: three pins
92+
// asserting that `[key: string]: any` was still present, and that
93+
// `props.required` / `props.error` therefore read as `any`. objectui#3221
94+
// removed that index signature, so those pins have gone red on purpose and
95+
// are gone with it. What replaces them is the inverse claim — the type is
96+
// now CLOSED, so the two keys the spec declares and this one does not can
97+
// finally be reported as missing. That is what makes objectui#3222 (the
98+
// `error` / `errorMessage` divergence) decidable by the compiler instead of
99+
// by a symbol guard.
100+
type _NoStringIndexSignature = Assert<Equal<Extends<string, keyof FieldWidgetComponentProps>, false>>;
101+
type _RequiredIsAbsent = Assert<Equal<HasKey<FieldWidgetComponentProps, 'required'>, false>>;
102+
type _ErrorIsAbsent = Assert<Equal<HasKey<FieldWidgetComponentProps, 'error'>, false>>;
103+
104+
// `data-*` stays open (it is open in HTML too), but as a template-literal
105+
// key — the distinction that keeps `keyof` finite above. A pin, because
106+
// widening it back to `[key: string]` would silently restore the defect
107+
// while every assertion here still read as "closed".
108+
type _DataAttributesStayOpen = Assert<Extends<'data-testid', keyof FieldWidgetComponentProps>>;
103109

104110
expect(true).toBe(true);
105111
});
Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
1+
/**
2+
* ObjectUI
3+
* Copyright (c) 2024-present ObjectStack Inc.
4+
*
5+
* This source code is licensed under the MIT license found in the
6+
* LICENSE file in the root directory of this source tree.
7+
*/
8+
9+
/**
10+
* `FieldWidgetComponentProps` is a CLOSED contract (objectui#3221).
11+
*
12+
* It used to end in `[key: string]: any`, which made every check over it
13+
* vacuous: a type claiming to have every key can never be reported as missing
14+
* one (objectstack#4075). Three consequences, one test each below:
15+
*
16+
* 1. a key the type does not declare is a compile error, not a silent `any`
17+
* — which is what makes the `error` / `errorMessage` divergence
18+
* (objectui#3222) decidable by the compiler at all;
19+
* 2. a MISSPELLED prop (`readOnly` for `readonly`) is rejected;
20+
* 3. the pass-through keys hosts genuinely forward still type-check, and
21+
* still reach the rendered control at runtime — closing the type must not
22+
* have been paid for by dropping real behaviour.
23+
*
24+
* (1) and (2) are compile-time: a violation fails `pnpm --filter
25+
* @object-ui/fields type-check`, not this run. `@ts-expect-error` inverts
26+
* that — the line fails to compile if the error it expects ever STOPS
27+
* happening, i.e. if the index signature comes back.
28+
*/
29+
30+
import { describe, it, expect } from 'vitest';
31+
import { render, screen } from '@testing-library/react';
32+
33+
import { TextField } from '../widgets/TextField';
34+
import { BooleanField } from '../widgets/BooleanField';
35+
import type { FieldWidgetComponentProps } from '../widgets/types';
36+
import type { FieldMetadata } from '@object-ui/types';
37+
38+
type Assert<T extends true> = T;
39+
type Equal<A, B> = (<T>() => T extends A ? 1 : 2) extends <T>() => T extends B ? 1 : 2 ? true : false;
40+
type Extends<A, B> = [A] extends [B] ? true : false;
41+
42+
const field = { name: 'title', label: 'Title', type: 'text' } as unknown as FieldMetadata;
43+
44+
describe('FieldWidgetComponentProps is closed (objectui#3221)', () => {
45+
it('has no `[key: string]` index signature', () => {
46+
// The single assertion the whole change turns on. While the index
47+
// signature existed this was `true`, and every check below was vacuous.
48+
type _NotOpen = Assert<Equal<Extends<string, keyof FieldWidgetComponentProps>, false>>;
49+
expect(true).toBe(true);
50+
});
51+
52+
it('rejects keys it does not declare', () => {
53+
const props = {} as FieldWidgetComponentProps<string>;
54+
55+
// The spec's `FieldWidgetPropsSchema` declares both; this type declares
56+
// neither. Reading them used to give `any` and `undefined` forever.
57+
// Whether to ADD them is objectui#3222 — deliberately not decided here.
58+
// @ts-expect-error `required` is not part of this contract
59+
void props.required;
60+
// @ts-expect-error `error` is not part of this contract — this type's slot is `errorMessage`
61+
void props.error;
62+
63+
// …and the class of bug the index signature hid best: a typo.
64+
// @ts-expect-error the prop is `readonly`, not React's DOM `readOnly`
65+
void props.readOnly;
66+
// @ts-expect-error the prop is `onChange`
67+
void props.onchange;
68+
69+
expect(true).toBe(true);
70+
});
71+
72+
it('still accepts the pass-through keys hosts actually forward', () => {
73+
// Closing the type is only correct if the real call sites still compile.
74+
// Each key here has a named producer: the form renderer, the inline-edit
75+
// host, or the line-item grid. Deleting one from the type breaks this.
76+
const passThrough: FieldWidgetComponentProps<string> = {
77+
value: '',
78+
onChange: () => {},
79+
field,
80+
// form renderer (`renderFieldComponent`)
81+
schema: field,
82+
dataSource: {},
83+
dependentValues: { country: 'cn' },
84+
dependsOn: ['country'],
85+
emptyHint: 'Select country first',
86+
name: 'title',
87+
// inline-edit hosts / line-item grid
88+
compact: true,
89+
autoFocus: true,
90+
onSelectRecord: () => {},
91+
onCreateNew: () => {},
92+
// DOM / a11y
93+
id: 'title-input',
94+
'aria-label': 'Title',
95+
'data-testid': 'field-title',
96+
};
97+
expect(passThrough.compact).toBe(true);
98+
});
99+
});
100+
101+
describe('closing the type kept the pass-through behaviour', () => {
102+
it('forwards a11y and data attributes to the rendered input', () => {
103+
render(
104+
<TextField
105+
value="hello"
106+
onChange={() => {}}
107+
field={field}
108+
aria-label="Title"
109+
data-testid="title-input"
110+
/>,
111+
);
112+
const input = screen.getByTestId('title-input');
113+
expect(input).toHaveAttribute('aria-label', 'Title');
114+
expect(input).toHaveValue('hello');
115+
});
116+
117+
it('forwards `disabled` — a declared key, not an index-signature accident', () => {
118+
render(
119+
<BooleanField
120+
value={false}
121+
onChange={() => {}}
122+
field={{ name: 'active', label: 'Active', type: 'boolean' } as unknown as FieldMetadata}
123+
disabled
124+
data-testid="active-switch"
125+
/>,
126+
);
127+
expect(screen.getByTestId('active-switch')).toBeDisabled();
128+
});
129+
});

packages/fields/src/widgets/CapabilityMultiSelectField.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,8 +90,8 @@ export function CapabilityMultiSelectField({
9090
const { t } = useFieldTranslation();
9191
const ctx = React.useContext(SchemaRendererContext);
9292
const dataSource: DataSource | null =
93-
(props as any).dataSource ?? (ctx as any)?.dataSource ?? null;
94-
const disabled = (props as any).disabled as boolean | undefined;
93+
(props.dataSource as any) ?? (ctx as any)?.dataSource ?? null;
94+
const disabled = props.disabled;
9595

9696
const [caps, setCaps] = React.useState<Capability[] | null>(null);
9797

packages/fields/src/widgets/CheckboxesField.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ export function CheckboxesField({
3535
const rawOptions: Option[] = config?.options || [];
3636
const selected: string[] = Array.isArray(value) ? value : value == null ? [] : [value as unknown as string];
3737
const groupId = useId();
38-
const fieldName = (props as any).name || config?.name || (props as any).id || '';
38+
const fieldName = props.name || config?.name || props.id || '';
3939

4040
const dependsOn = config?.dependsOn ?? dependsOnProp;
4141
const { options, gated, dependsOnFields } = useCascadingOptions<Option>(
@@ -107,7 +107,7 @@ export function CheckboxesField({
107107
id={id}
108108
checked={selected.includes(value)}
109109
onCheckedChange={(checked) => toggle(value, !!checked)}
110-
disabled={(props as any).disabled}
110+
disabled={props.disabled}
111111
data-testid={`checkboxes-option-${value}`}
112112
/>
113113
<Label htmlFor={id} className="font-normal">{opt.label}</Label>

packages/fields/src/widgets/FilterConditionField.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -301,7 +301,7 @@ export function FilterConditionField({
301301
}: FieldWidgetComponentProps<string | object>) {
302302
const ctx = React.useContext(SchemaRendererContext);
303303
const { t } = useFieldTranslation();
304-
const dataSource: any = (props as any).dataSource ?? (ctx as any)?.dataSource ?? null;
304+
const dataSource: any = props.dataSource ?? (ctx as any)?.dataSource ?? null;
305305
const dependentValues: Record<string, any> = (props as any).dependentValues ?? {};
306306
const objectName = String(dependentValues.object_name ?? '');
307307

packages/fields/src/widgets/GridField.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -327,7 +327,7 @@ export function GridField({
327327
const cfg = (field || (props as any).schema || {}) as any;
328328
const allColumns: GridColumn[] = cfg.columns || [];
329329
const rows: Row[] = Array.isArray(value) ? value : [];
330-
const contextRecord = (props as any).contextRecord as Record<string, unknown> | undefined;
330+
const contextRecord = props.contextRecord;
331331

332332
// Per-cell CEL rule state (B2 in grids). A column with no readonlyWhen/
333333
// requiredWhen resolves to its static flags (cheap fast-path — no engine

packages/fields/src/widgets/LookupField.tsx

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -280,13 +280,13 @@ export function LookupField({ value, onChange, field, readonly, ...props }: Fiel
280280
}, [fieldMeta?.depends_on, fieldMeta?.dependsOn]);
281281

282282
// Resolve dependent field values from explicit prop or SchemaRendererContext.data
283-
const dependentValuesProp = (props as any).dependentValues as Record<string, any> | undefined;
283+
const dependentValuesProp = props.dependentValues;
284284

285285
// Resolve DataSource: explicit prop > field-level > wrapper field > SchemaRendererContext > none
286286
const ctx = useContext(SchemaRendererContext);
287287
const contextDataSource = ctx?.dataSource ?? null;
288288
const dataSource: DataSource | null =
289-
(props as any).dataSource ?? lookupField?.dataSource ?? fieldMeta?.dataSource ?? contextDataSource;
289+
(props.dataSource as DataSource | null | undefined) ?? lookupField?.dataSource ?? fieldMeta?.dataSource ?? contextDataSource;
290290

291291
/** Resolve dependent values from the explicit prop (preferred), the form-data
292292
* context provided by @object-ui/react, or finally `ctx.data` (record scope). */
@@ -369,7 +369,7 @@ export function LookupField({ value, onChange, field, readonly, ...props }: Fiel
369369

370370
// Optional create-new callback
371371
const onCreateNew: ((searchQuery: string) => void) | undefined =
372-
(props as any).onCreateNew ?? lookupField?.onCreateNew;
372+
props.onCreateNew ?? lookupField?.onCreateNew;
373373

374374
// State for the full Record Picker dialog (Level 2)
375375
const [isPickerOpen, setIsPickerOpen] = useState(false);
@@ -657,7 +657,7 @@ export function LookupField({ value, onChange, field, readonly, ...props }: Fiel
657657
// auto-fill sibling fields from it — e.g. a line-item grid copying a product's
658658
// unit_price/description when the item is chosen. When provided (single
659659
// select), it drives the update and the host owns the resulting value change.
660-
const onSelectRecord = (props as any).onSelectRecord as ((record: LookupOption) => void) | undefined;
660+
const onSelectRecord = props.onSelectRecord;
661661

662662
const handleSelect = useCallback(
663663
(option: LookupOption) => {
@@ -906,7 +906,7 @@ export function LookupField({ value, onChange, field, readonly, ...props }: Fiel
906906
// Compact mode (e.g. inside a line-item grid cell): show the selected value
907907
// INSIDE a borderless trigger on a single line — no chip stacked above a
908908
// separate "Select…" button (which double-stacks and wastes the row height).
909-
const compact = !!(props as any).compact;
909+
const compact = !!props.compact;
910910
const singleSelectedLabel = selectedOptions[0]?.label || selectedOptions[0]?.[displayField];
911911

912912
// Shared field trigger — the anchor for either the inline PeoplePicker
@@ -920,8 +920,8 @@ export function LookupField({ value, onChange, field, readonly, ...props }: Fiel
920920
compact && 'h-8 rounded-none border-0 bg-transparent px-2 shadow-none focus-visible:ring-1 focus-visible:ring-ring/60',
921921
)}
922922
type="button"
923-
disabled={dependenciesMissing || (props as any).disabled}
924-
data-testid={dependenciesMissing ? 'lookup-trigger-gated' : (((props as any).name || lookupField?.name) ? `lookup-trigger-${(props as any).name || lookupField.name}` : 'lookup-trigger')}
923+
disabled={dependenciesMissing || props.disabled}
924+
data-testid={dependenciesMissing ? 'lookup-trigger-gated' : ((props.name || lookupField?.name) ? `lookup-trigger-${props.name || lookupField.name}` : 'lookup-trigger')}
925925
title={dependenciesMissing
926926
? t('lookup.selectFirst', { fields: dependsOn.map(d => d.field).join(', ') })
927927
: undefined}
@@ -1214,7 +1214,7 @@ export function LookupField({ value, onChange, field, readonly, ...props }: Fiel
12141214
type="button"
12151215
// Gated exactly like the main trigger (#2215) — pre-fix this button
12161216
// opened the full unscoped table while the dependency was missing.
1217-
disabled={dependenciesMissing || (props as any).disabled}
1217+
disabled={dependenciesMissing || props.disabled}
12181218
onClick={() => setIsPickerOpen(true)}
12191219
aria-label={t('lookup.browseAll')}
12201220
title={dependenciesMissing

packages/fields/src/widgets/MultiSelectField.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ export function MultiSelectField({
3636
const config = (field || schema) as any;
3737
const rawOptions: Option[] = config?.options || [];
3838
const selected: string[] = Array.isArray(value) ? value : value == null ? [] : [value as unknown as string];
39-
const fieldName = (props as any).name || config?.name || (props as any).id || '';
39+
const fieldName = props.name || config?.name || props.id || '';
4040

4141
const dependsOn = config?.dependsOn ?? dependsOnProp;
4242
const { options, gated, dependsOnFields } = useCascadingOptions<Option>(
@@ -107,7 +107,7 @@ export function MultiSelectField({
107107
type="button"
108108
key={value}
109109
onClick={() => toggle(value)}
110-
disabled={(props as any).disabled}
110+
disabled={props.disabled}
111111
aria-pressed={active}
112112
data-testid={`multiselect-option-${opt.value}`}
113113
className={cn(

0 commit comments

Comments
 (0)