Skip to content

Commit 9555b07

Browse files
os-zhuangclaude
andauthored
feat(lint): <ListView searchableFields> on a react page resolves against the bound object's fields (#4329) (#4338)
#4328's searchable-field-unknown gates the metadata surfaces but not the react page surface: ListView declares searchableFields as a dataProp, so a kind:'react' page could ship a stale name nothing resolves — the engine silently narrows the search (or widens it to the auto-default once every entry is stale), and #4254 turns the echoed $searchFields override into a 400 INVALID_FIELD. validate-searchable-fields now exports its core (indexObjectSearchTargets + checkSearchableFieldList) and validate-react-page-props runs it on <ListView> usages with static objectName + searchableFields, under the same rule id and severity. Same three skips, same dotted-path strictness; non-static values and spreads skip silently (ADR-0072 D1). Follow-up: #4340 tracks the remaining unchecked field-bearing React block props centrally. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 3245174 commit 9555b07

4 files changed

Lines changed: 262 additions & 50 deletions

File tree

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
---
2+
"@objectstack/lint": minor
3+
---
4+
5+
feat(lint): `<ListView searchableFields>` on a react page is checked against
6+
the bound object's fields (#4329)
7+
8+
#4328's `searchable-field-unknown` gates a stale `searchableFields` entry on
9+
the metadata surfaces — an object's own ADR-0061 declaration, its built-in
10+
named list views, and a `defineView` aggregate's default `list` / named
11+
`listViews`. It did not cover the react page surface: `ListView` declares
12+
`searchableFields` as a dataProp, so a `kind:'react'` page could write
13+
`<ListView searchableFields={['renamed_field']}>` and nothing resolved the
14+
name. The failure is the one #4328 documents — the engine's
15+
`resolveSearchFields` silently filters the stale name out, so the search scans
16+
a narrower set than the page asked for, or (once every entry is stale) falls
17+
through to the auto-default and scans a wider one; and once the REST read path
18+
validates the `$searchFields` override (#4254), the prop objectui echoes
19+
verbatim becomes a `400 INVALID_FIELD` on that list.
20+
21+
The check lives in `validate-react-page-props` — the gate that already parses
22+
the page's real JSX — and runs on `<ListView>` usages whose `objectName` and
23+
`searchableFields` are static literals, under the same rule id and severity
24+
(`searchable-field-unknown`, `error`) as the metadata surfaces. It is not a
25+
re-implementation: `validate-searchable-fields` now exports its core
26+
(`indexObjectSearchTargets` + `checkSearchableFieldList`), and the react gate
27+
runs that, so the two surfaces agree on what counts as a field by construction
28+
— same three skips (an object this stack does not define, an object with no
29+
authored field map, registry-injected system columns derived from the spec's
30+
own declarations), same dotted-path strictness (search matches the field map
31+
by exact string, so `owner_id.name` is flagged, not exempted).
32+
33+
JSX-specific seams follow the gate's existing rules: a value that comes from a
34+
variable, a call, or a spread is not knowable at build time and is skipped
35+
silently — an unresolvable binding is not a wrong one (ADR-0072 D1).

packages/lint/src/validate-react-page-props.test.ts

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import {
66
REACT_CHART_AGGREGATE_INVALID,
77
REACT_CHART_AXIS_UNKNOWN,
88
} from './validate-react-page-props.js';
9+
import { SEARCHABLE_FIELD_UNKNOWN } from './validate-searchable-fields.js';
910

1011
const page = (source: string) => ({ pages: [{ name: 'p', kind: 'react', source }] });
1112

@@ -240,3 +241,109 @@ describe('validateReactPageProps — <ObjectChart> bindings (#3701)', () => {
240241
expect(f).toEqual([]);
241242
});
242243
});
244+
245+
// ─────────────────────────────────────────────────────────────────────────
246+
// <ListView> searchableFields (#4329) — the react-surface twin of the
247+
// metadata rule `searchable-field-unknown` (#4328). Same core, so the same
248+
// skips and the same dotted-path strictness; these tests pin the JSX-specific
249+
// seams (static-value resolution, spread, where/path shape).
250+
// ─────────────────────────────────────────────────────────────────────────
251+
252+
const account = {
253+
name: 'crm_account',
254+
fields: {
255+
name: { type: 'text' },
256+
billing_email: { type: 'email' },
257+
owner_id: { type: 'lookup', reference: 'sys_user' },
258+
},
259+
};
260+
261+
const listPage = (source: string, objects: unknown[] = [account]) => ({
262+
objects,
263+
pages: [{ name: 'p', kind: 'react', source }],
264+
});
265+
266+
const list = (attrs: string) => `function Page(){ return <ListView ${attrs} />; }`;
267+
268+
describe('validateReactPageProps — <ListView> searchableFields (#4329)', () => {
269+
it('passes a declaration whose every entry resolves', () => {
270+
const f = validateReactPageProps(
271+
listPage(list(`objectName="crm_account" searchableFields={['name', 'billing_email']}`)),
272+
);
273+
expect(f).toEqual([]);
274+
});
275+
276+
it('flags a stale entry, gating, under the metadata rule id', () => {
277+
const f = validateReactPageProps(
278+
listPage(list(`objectName="crm_account" searchableFields={['name', 'email']}`)),
279+
);
280+
281+
expect(f).toHaveLength(1);
282+
expect(f[0].rule).toBe(SEARCHABLE_FIELD_UNKNOWN);
283+
// `error`, like the metadata surface: objectui echoes the prop verbatim as
284+
// the `$searchFields` override, so a stale entry is the same 400-in-waiting.
285+
expect(f[0].severity).toBe('error');
286+
expect(f[0].where).toBe('page "p" › <ListView>');
287+
// The entry index is part of the path so the author can go straight to it.
288+
expect(f[0].path).toBe('pages[0].source › searchableFields[1]');
289+
expect(f[0].message).toContain('"email"');
290+
expect(f[0].message).toContain('crm_account');
291+
expect(f[0].hint).toContain('400 INVALID_FIELD');
292+
});
293+
294+
it('suggests the real field when the stale name is close to one', () => {
295+
const f = validateReactPageProps(
296+
listPage(list(`objectName="crm_account" searchableFields={['biling_email']}`)),
297+
);
298+
expect(f[0].message).toContain('Did you mean "billing_email"?');
299+
});
300+
301+
it('accepts registry-injected system columns absent from authored fields', () => {
302+
const f = validateReactPageProps(
303+
listPage(list(`objectName="crm_account" searchableFields={['name', 'created_at', 'owner_id']}`)),
304+
);
305+
expect(f).toEqual([]);
306+
});
307+
308+
it('flags a dotted path — search cannot resolve the traversal', () => {
309+
const f = validateReactPageProps(
310+
listPage(list(`objectName="crm_account" searchableFields={['owner_id.name']}`)),
311+
);
312+
expect(f).toHaveLength(1);
313+
expect(f[0].hint).toContain("scans this object's own columns");
314+
});
315+
316+
it('skips an object this stack does not define', () => {
317+
const f = validateReactPageProps(
318+
listPage(list(`objectName="pkg_contract" searchableFields={['no_such_field']}`)),
319+
);
320+
expect(f).toEqual([]);
321+
});
322+
323+
it('skips an object with no authored field map (external / introspected)', () => {
324+
const f = validateReactPageProps(
325+
listPage(list(`objectName="external_invoice" searchableFields={['doc_no']}`), [
326+
{ name: 'external_invoice', external: { datasource: 'erp' } },
327+
]),
328+
);
329+
expect(f).toEqual([]);
330+
});
331+
332+
it('skips a value built from a variable (not knowable at build time)', () => {
333+
const f = validateReactPageProps(
334+
listPage(
335+
'function Page(){ const sf = ["nope"]; return <ListView objectName="crm_account" searchableFields={sf} />; }',
336+
),
337+
);
338+
expect(f).toEqual([]);
339+
});
340+
341+
it('skips everything behind a spread (props may come from it)', () => {
342+
const f = validateReactPageProps(
343+
listPage(
344+
'function Page(){ const p = {}; return <ListView objectName="crm_account" {...p} searchableFields={["nope"]} />; }',
345+
),
346+
);
347+
expect(f).toEqual([]);
348+
});
349+
});

packages/lint/src/validate-react-page-props.ts

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,9 @@
1717
// - <ObjectChart>'s data BINDINGS, by reading the attribute VALUES (#3701,
1818
// retargeted to the spec shape in #3729). See the block comment above
1919
// `checkObjectChart`.
20+
// - <ListView>'s `searchableFields` entries, resolved against the bound
21+
// object's declared fields (#4329) — the react-surface twin of the
22+
// metadata rule `searchable-field-unknown`, sharing its core.
2023
//
2124
// Reading values is opt-in per block and per prop: everything below evaluates
2225
// only STATIC literals (`objectName="invoice"`, an `aggregate={{…}}` object
@@ -27,6 +30,10 @@
2730
import { createRequire } from 'node:module';
2831
import type ts from 'typescript';
2932
import { REACT_BLOCKS, chartAggregateResultKeys } from '@objectstack/spec/ui';
33+
import {
34+
checkSearchableFieldList,
35+
indexObjectSearchTargets,
36+
} from './validate-searchable-fields.js';
3037

3138
// The TypeScript compiler must NOT be imported at module top level: it is
3239
// ~9 MB of CJS (~70 ms+ to parse, worse on container cold starts), and
@@ -387,6 +394,11 @@ function checkObjectChart(
387394
export function validateReactPageProps(stack: AnyRec): ReactPropFinding[] {
388395
const findings: ReactPropFinding[] = [];
389396
const objectFields = indexObjectFields(stack);
397+
// A separate index for the searchableFields check, built by the metadata
398+
// rule's own indexer: it keeps `null` for an object with no authored field
399+
// map (external / datasource-introspected), a distinction `indexObjectFields`
400+
// flattens — and one this check must honor so both surfaces skip alike.
401+
const searchTargets = indexObjectSearchTargets(stack);
390402
const pages = asArray(stack.pages);
391403
for (let p = 0; p < pages.length; p++) {
392404
const page = pages[p];
@@ -454,6 +466,24 @@ export function validateReactPageProps(stack: AnyRec): ReactPropFinding[] {
454466
if (tag === 'ObjectChart' && !hasSpread) {
455467
checkObjectChart({ values, where, path }, objectFields, findings);
456468
}
469+
// <ListView searchableFields> names fields on the bound object — the
470+
// react-surface twin of `searchable-field-unknown` (#4329). It runs
471+
// the metadata rule's own core, so the skips (cross-package object,
472+
// no authored field map, system columns) and the dotted-path
473+
// strictness match by construction. A non-static value — either
474+
// attribute — bails inside the checker: unresolvable is not wrong.
475+
if (tag === 'ListView' && !hasSpread) {
476+
findings.push(
477+
...checkSearchableFieldList(
478+
values.get('searchableFields'),
479+
strOf(values.get('objectName')),
480+
searchTargets,
481+
where,
482+
`${path} › searchableFields`,
483+
'searchableFields',
484+
),
485+
);
486+
}
457487
}
458488
}
459489
tsc.forEachChild(node, visit);

packages/lint/src/validate-searchable-fields.ts

Lines changed: 90 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -169,74 +169,114 @@ function distance(a: string, b: string): number {
169169
return prev[n];
170170
}
171171

172+
/**
173+
* object name → declared field names. `null` marks an object with no readable
174+
* field map, so "declared nothing" stays distinguishable from "not in stack".
175+
* Exported alongside `checkSearchableFieldList` so every surface that authors
176+
* a searchable set resolves against the identical index (#4329).
177+
*/
178+
export function indexObjectSearchTargets(
179+
stack: Record<string, unknown>,
180+
): Map<string, Set<string> | null> {
181+
const fieldsByObject = new Map<string, Set<string> | null>();
182+
if (!isRec(stack)) return fieldsByObject;
183+
for (const obj of asArray(stack.objects)) {
184+
const name = strName(obj.name);
185+
if (name) fieldsByObject.set(name, declaredFieldNames(obj));
186+
}
187+
return fieldsByObject;
188+
}
189+
190+
/**
191+
* Check one `searchableFields` array against the field map `fieldsByObject`
192+
* holds for `objectName` — the shared core behind every surface that authors a
193+
* searchable set: the object/list-view metadata walked by
194+
* `validateSearchableFields` below, and the react page surface
195+
* (`<ListView searchableFields={…}>`, `validate-react-page-props`), which
196+
* reuses it so the two surfaces agree on what counts as a field — same three
197+
* skips, same dotted-path strictness (#4329).
198+
*
199+
* `subject` names the declaration for the message, since an object's own set
200+
* and a view's narrowing of it are fixed differently; the entry index is
201+
* appended to `path` so the author can go straight to the stale name.
202+
*/
203+
export function checkSearchableFieldList(
204+
declared: unknown,
205+
objectName: string | undefined,
206+
fieldsByObject: ReadonlyMap<string, Set<string> | null>,
207+
where: string,
208+
path: string,
209+
subject: string,
210+
): SearchableFieldFinding[] {
211+
const findings: SearchableFieldFinding[] = [];
212+
if (!Array.isArray(declared) || declared.length === 0) return findings;
213+
if (!objectName) return findings; // nothing to resolve against
214+
if (!fieldsByObject.has(objectName)) return findings; // ① object from another package
215+
const known = fieldsByObject.get(objectName);
216+
if (!known) return findings; // ② external / introspected — no authored field map
217+
218+
for (let i = 0; i < declared.length; i++) {
219+
const entry = declared[i];
220+
// Pre-parse input may carry junk here; a non-string is a SHAPE error the
221+
// schema owns, not a dangling reference.
222+
const name = strName(entry);
223+
if (!name) continue;
224+
if (known.has(name) || SYSTEM_FIELDS.has(name)) continue; // ③ system column
225+
226+
const dotted = name.includes('.');
227+
findings.push({
228+
severity: 'error',
229+
rule: SEARCHABLE_FIELD_UNKNOWN,
230+
where,
231+
path: `${path}[${i}]`,
232+
message:
233+
`${subject} entry "${name}" is not a field on object "${objectName}". ` +
234+
`The declaration is stale: searching it can never match, and the engine ` +
235+
`silently drops it — leaving a narrower search than declared, or the ` +
236+
`auto-default set once every entry is dropped.` +
237+
(dotted ? '' : suggest(name, known)),
238+
hint:
239+
(dotted
240+
? `'search' scans this object's own columns, so a related record's ` +
241+
`column cannot be a search target — expand the relation and search ` +
242+
`the related object, or copy the value onto a formula field here. `
243+
: `Fix the name, or add "${name}" to ${objectName}.fields. `) +
244+
`Clients echo this declaration verbatim as the '$searchFields' ` +
245+
`override, so a stale entry becomes a 400 INVALID_FIELD on list ` +
246+
`search (#4254), not just a quietly narrowed one.` +
247+
(known.size > 0 ? ` Object fields: ${[...known].sort().join(', ')}.` : ''),
248+
});
249+
}
250+
return findings;
251+
}
252+
172253
/**
173254
* Validate every `searchableFields` declaration in the stack — the object's own
174255
* (the canonical set, ADR-0061) and the list views that narrow it. Returns
175256
* findings (empty = clean).
257+
*
258+
* The react page surface (`<ListView searchableFields={…}>`) is deliberately
259+
* NOT walked here: its declaration lives inside JSX source, and
260+
* `validate-react-page-props` — the gate that already parses that source —
261+
* runs the same `checkSearchableFieldList` core on it (#4329).
176262
*/
177263
export function validateSearchableFields(stack: AnyRec): SearchableFieldFinding[] {
178264
const findings: SearchableFieldFinding[] = [];
179265
if (!isRec(stack)) return findings;
180266

181-
// object name → declared field names. `null` marks an object with no readable
182-
// field map, so "declared nothing" stays distinguishable from "not in stack".
183267
const objects = asArray(stack.objects);
184-
const fieldsByObject = new Map<string, Set<string> | null>();
185-
for (const obj of objects) {
186-
const name = strName(obj.name);
187-
if (name) fieldsByObject.set(name, declaredFieldNames(obj));
188-
}
268+
const fieldsByObject = indexObjectSearchTargets(stack);
189269

190-
/**
191-
* Check one `searchableFields` array against `objectName`'s field map.
192-
* `subject` names the declaration for the message, since an object's own
193-
* set and a view's narrowing of it are fixed differently.
194-
*/
195270
const check = (
196271
declared: unknown,
197272
objectName: string | undefined,
198273
where: string,
199274
path: string,
200275
subject: string,
201276
) => {
202-
if (!Array.isArray(declared) || declared.length === 0) return;
203-
if (!objectName) return; // nothing to resolve against
204-
if (!fieldsByObject.has(objectName)) return; // ① object from another package
205-
const known = fieldsByObject.get(objectName);
206-
if (!known) return; // ② external / introspected — no authored field map
207-
208-
for (let i = 0; i < declared.length; i++) {
209-
const entry = declared[i];
210-
// Pre-parse input may carry junk here; a non-string is a SHAPE error the
211-
// schema owns, not a dangling reference.
212-
const name = strName(entry);
213-
if (!name) continue;
214-
if (known.has(name) || SYSTEM_FIELDS.has(name)) continue; // ③ system column
215-
216-
const dotted = name.includes('.');
217-
findings.push({
218-
severity: 'error',
219-
rule: SEARCHABLE_FIELD_UNKNOWN,
220-
where,
221-
path: `${path}[${i}]`,
222-
message:
223-
`${subject} entry "${name}" is not a field on object "${objectName}". ` +
224-
`The declaration is stale: searching it can never match, and the engine ` +
225-
`silently drops it — leaving a narrower search than declared, or the ` +
226-
`auto-default set once every entry is dropped.` +
227-
(dotted ? '' : suggest(name, known)),
228-
hint:
229-
(dotted
230-
? `'search' scans this object's own columns, so a related record's ` +
231-
`column cannot be a search target — expand the relation and search ` +
232-
`the related object, or copy the value onto a formula field here. `
233-
: `Fix the name, or add "${name}" to ${objectName}.fields. `) +
234-
`Clients echo this declaration verbatim as the '$searchFields' ` +
235-
`override, so a stale entry becomes a 400 INVALID_FIELD on list ` +
236-
`search (#4254), not just a quietly narrowed one.` +
237-
(known.size > 0 ? ` Object fields: ${[...known].sort().join(', ')}.` : ''),
238-
});
239-
}
277+
findings.push(
278+
...checkSearchableFieldList(declared, objectName, fieldsByObject, where, path, subject),
279+
);
240280
};
241281

242282
// ── The object's own canonical set, and its built-in named list views ──

0 commit comments

Comments
 (0)