diff --git a/.changeset/react-listview-searchable-fields-lint.md b/.changeset/react-listview-searchable-fields-lint.md new file mode 100644 index 0000000000..3314fe7586 --- /dev/null +++ b/.changeset/react-listview-searchable-fields-lint.md @@ -0,0 +1,35 @@ +--- +"@objectstack/lint": minor +--- + +feat(lint): `` on a react page is checked against +the bound object's fields (#4329) + +#4328's `searchable-field-unknown` gates a stale `searchableFields` entry on +the metadata surfaces — an object's own ADR-0061 declaration, its built-in +named list views, and a `defineView` aggregate's default `list` / named +`listViews`. It did not cover the react page surface: `ListView` declares +`searchableFields` as a dataProp, so a `kind:'react'` page could write +`` and nothing resolved the +name. The failure is the one #4328 documents — the engine's +`resolveSearchFields` silently filters the stale name out, so the search scans +a narrower set than the page asked for, or (once every entry is stale) falls +through to the auto-default and scans a wider one; and once the REST read path +validates the `$searchFields` override (#4254), the prop objectui echoes +verbatim becomes a `400 INVALID_FIELD` on that list. + +The check lives in `validate-react-page-props` — the gate that already parses +the page's real JSX — and runs on `` usages whose `objectName` and +`searchableFields` are static literals, under the same rule id and severity +(`searchable-field-unknown`, `error`) as the metadata surfaces. It is not a +re-implementation: `validate-searchable-fields` now exports its core +(`indexObjectSearchTargets` + `checkSearchableFieldList`), and the react gate +runs that, so the two surfaces agree on what counts as a field by construction +— same three skips (an object this stack does not define, an object with no +authored field map, registry-injected system columns derived from the spec's +own declarations), same dotted-path strictness (search matches the field map +by exact string, so `owner_id.name` is flagged, not exempted). + +JSX-specific seams follow the gate's existing rules: a value that comes from a +variable, a call, or a spread is not knowable at build time and is skipped +silently — an unresolvable binding is not a wrong one (ADR-0072 D1). diff --git a/packages/lint/src/validate-react-page-props.test.ts b/packages/lint/src/validate-react-page-props.test.ts index 71f194f115..9e36f73c6b 100644 --- a/packages/lint/src/validate-react-page-props.test.ts +++ b/packages/lint/src/validate-react-page-props.test.ts @@ -6,6 +6,7 @@ import { REACT_CHART_AGGREGATE_INVALID, REACT_CHART_AXIS_UNKNOWN, } from './validate-react-page-props.js'; +import { SEARCHABLE_FIELD_UNKNOWN } from './validate-searchable-fields.js'; const page = (source: string) => ({ pages: [{ name: 'p', kind: 'react', source }] }); @@ -240,3 +241,109 @@ describe('validateReactPageProps — bindings (#3701)', () => { expect(f).toEqual([]); }); }); + +// ───────────────────────────────────────────────────────────────────────── +// searchableFields (#4329) — the react-surface twin of the +// metadata rule `searchable-field-unknown` (#4328). Same core, so the same +// skips and the same dotted-path strictness; these tests pin the JSX-specific +// seams (static-value resolution, spread, where/path shape). +// ───────────────────────────────────────────────────────────────────────── + +const account = { + name: 'crm_account', + fields: { + name: { type: 'text' }, + billing_email: { type: 'email' }, + owner_id: { type: 'lookup', reference: 'sys_user' }, + }, +}; + +const listPage = (source: string, objects: unknown[] = [account]) => ({ + objects, + pages: [{ name: 'p', kind: 'react', source }], +}); + +const list = (attrs: string) => `function Page(){ return ; }`; + +describe('validateReactPageProps — searchableFields (#4329)', () => { + it('passes a declaration whose every entry resolves', () => { + const f = validateReactPageProps( + listPage(list(`objectName="crm_account" searchableFields={['name', 'billing_email']}`)), + ); + expect(f).toEqual([]); + }); + + it('flags a stale entry, gating, under the metadata rule id', () => { + const f = validateReactPageProps( + listPage(list(`objectName="crm_account" searchableFields={['name', 'email']}`)), + ); + + expect(f).toHaveLength(1); + expect(f[0].rule).toBe(SEARCHABLE_FIELD_UNKNOWN); + // `error`, like the metadata surface: objectui echoes the prop verbatim as + // the `$searchFields` override, so a stale entry is the same 400-in-waiting. + expect(f[0].severity).toBe('error'); + expect(f[0].where).toBe('page "p" › '); + // The entry index is part of the path so the author can go straight to it. + expect(f[0].path).toBe('pages[0].source › searchableFields[1]'); + expect(f[0].message).toContain('"email"'); + expect(f[0].message).toContain('crm_account'); + expect(f[0].hint).toContain('400 INVALID_FIELD'); + }); + + it('suggests the real field when the stale name is close to one', () => { + const f = validateReactPageProps( + listPage(list(`objectName="crm_account" searchableFields={['biling_email']}`)), + ); + expect(f[0].message).toContain('Did you mean "billing_email"?'); + }); + + it('accepts registry-injected system columns absent from authored fields', () => { + const f = validateReactPageProps( + listPage(list(`objectName="crm_account" searchableFields={['name', 'created_at', 'owner_id']}`)), + ); + expect(f).toEqual([]); + }); + + it('flags a dotted path — search cannot resolve the traversal', () => { + const f = validateReactPageProps( + listPage(list(`objectName="crm_account" searchableFields={['owner_id.name']}`)), + ); + expect(f).toHaveLength(1); + expect(f[0].hint).toContain("scans this object's own columns"); + }); + + it('skips an object this stack does not define', () => { + const f = validateReactPageProps( + listPage(list(`objectName="pkg_contract" searchableFields={['no_such_field']}`)), + ); + expect(f).toEqual([]); + }); + + it('skips an object with no authored field map (external / introspected)', () => { + const f = validateReactPageProps( + listPage(list(`objectName="external_invoice" searchableFields={['doc_no']}`), [ + { name: 'external_invoice', external: { datasource: 'erp' } }, + ]), + ); + expect(f).toEqual([]); + }); + + it('skips a value built from a variable (not knowable at build time)', () => { + const f = validateReactPageProps( + listPage( + 'function Page(){ const sf = ["nope"]; return ; }', + ), + ); + expect(f).toEqual([]); + }); + + it('skips everything behind a spread (props may come from it)', () => { + const f = validateReactPageProps( + listPage( + 'function Page(){ const p = {}; return ; }', + ), + ); + expect(f).toEqual([]); + }); +}); diff --git a/packages/lint/src/validate-react-page-props.ts b/packages/lint/src/validate-react-page-props.ts index a2b052d4fd..392746f1df 100644 --- a/packages/lint/src/validate-react-page-props.ts +++ b/packages/lint/src/validate-react-page-props.ts @@ -17,6 +17,9 @@ // - 's data BINDINGS, by reading the attribute VALUES (#3701, // retargeted to the spec shape in #3729). See the block comment above // `checkObjectChart`. +// - 's `searchableFields` entries, resolved against the bound +// object's declared fields (#4329) — the react-surface twin of the +// metadata rule `searchable-field-unknown`, sharing its core. // // Reading values is opt-in per block and per prop: everything below evaluates // only STATIC literals (`objectName="invoice"`, an `aggregate={{…}}` object @@ -27,6 +30,10 @@ import { createRequire } from 'node:module'; import type ts from 'typescript'; import { REACT_BLOCKS, chartAggregateResultKeys } from '@objectstack/spec/ui'; +import { + checkSearchableFieldList, + indexObjectSearchTargets, +} from './validate-searchable-fields.js'; // The TypeScript compiler must NOT be imported at module top level: it is // ~9 MB of CJS (~70 ms+ to parse, worse on container cold starts), and @@ -387,6 +394,11 @@ function checkObjectChart( export function validateReactPageProps(stack: AnyRec): ReactPropFinding[] { const findings: ReactPropFinding[] = []; const objectFields = indexObjectFields(stack); + // A separate index for the searchableFields check, built by the metadata + // rule's own indexer: it keeps `null` for an object with no authored field + // map (external / datasource-introspected), a distinction `indexObjectFields` + // flattens — and one this check must honor so both surfaces skip alike. + const searchTargets = indexObjectSearchTargets(stack); const pages = asArray(stack.pages); for (let p = 0; p < pages.length; p++) { const page = pages[p]; @@ -454,6 +466,24 @@ export function validateReactPageProps(stack: AnyRec): ReactPropFinding[] { if (tag === 'ObjectChart' && !hasSpread) { checkObjectChart({ values, where, path }, objectFields, findings); } + // names fields on the bound object — the + // react-surface twin of `searchable-field-unknown` (#4329). It runs + // the metadata rule's own core, so the skips (cross-package object, + // no authored field map, system columns) and the dotted-path + // strictness match by construction. A non-static value — either + // attribute — bails inside the checker: unresolvable is not wrong. + if (tag === 'ListView' && !hasSpread) { + findings.push( + ...checkSearchableFieldList( + values.get('searchableFields'), + strOf(values.get('objectName')), + searchTargets, + where, + `${path} › searchableFields`, + 'searchableFields', + ), + ); + } } } tsc.forEachChild(node, visit); diff --git a/packages/lint/src/validate-searchable-fields.ts b/packages/lint/src/validate-searchable-fields.ts index e5197ed9e9..39e8126570 100644 --- a/packages/lint/src/validate-searchable-fields.ts +++ b/packages/lint/src/validate-searchable-fields.ts @@ -169,29 +169,104 @@ function distance(a: string, b: string): number { return prev[n]; } +/** + * object name → declared field names. `null` marks an object with no readable + * field map, so "declared nothing" stays distinguishable from "not in stack". + * Exported alongside `checkSearchableFieldList` so every surface that authors + * a searchable set resolves against the identical index (#4329). + */ +export function indexObjectSearchTargets( + stack: Record, +): Map | null> { + const fieldsByObject = new Map | null>(); + if (!isRec(stack)) return fieldsByObject; + for (const obj of asArray(stack.objects)) { + const name = strName(obj.name); + if (name) fieldsByObject.set(name, declaredFieldNames(obj)); + } + return fieldsByObject; +} + +/** + * Check one `searchableFields` array against the field map `fieldsByObject` + * holds for `objectName` — the shared core behind every surface that authors a + * searchable set: the object/list-view metadata walked by + * `validateSearchableFields` below, and the react page surface + * (``, `validate-react-page-props`), which + * reuses it so the two surfaces agree on what counts as a field — same three + * skips, same dotted-path strictness (#4329). + * + * `subject` names the declaration for the message, since an object's own set + * and a view's narrowing of it are fixed differently; the entry index is + * appended to `path` so the author can go straight to the stale name. + */ +export function checkSearchableFieldList( + declared: unknown, + objectName: string | undefined, + fieldsByObject: ReadonlyMap | null>, + where: string, + path: string, + subject: string, +): SearchableFieldFinding[] { + const findings: SearchableFieldFinding[] = []; + if (!Array.isArray(declared) || declared.length === 0) return findings; + if (!objectName) return findings; // nothing to resolve against + if (!fieldsByObject.has(objectName)) return findings; // ① object from another package + const known = fieldsByObject.get(objectName); + if (!known) return findings; // ② external / introspected — no authored field map + + for (let i = 0; i < declared.length; i++) { + const entry = declared[i]; + // Pre-parse input may carry junk here; a non-string is a SHAPE error the + // schema owns, not a dangling reference. + const name = strName(entry); + if (!name) continue; + if (known.has(name) || SYSTEM_FIELDS.has(name)) continue; // ③ system column + + const dotted = name.includes('.'); + findings.push({ + severity: 'error', + rule: SEARCHABLE_FIELD_UNKNOWN, + where, + path: `${path}[${i}]`, + message: + `${subject} entry "${name}" is not a field on object "${objectName}". ` + + `The declaration is stale: searching it can never match, and the engine ` + + `silently drops it — leaving a narrower search than declared, or the ` + + `auto-default set once every entry is dropped.` + + (dotted ? '' : suggest(name, known)), + hint: + (dotted + ? `'search' scans this object's own columns, so a related record's ` + + `column cannot be a search target — expand the relation and search ` + + `the related object, or copy the value onto a formula field here. ` + : `Fix the name, or add "${name}" to ${objectName}.fields. `) + + `Clients echo this declaration verbatim as the '$searchFields' ` + + `override, so a stale entry becomes a 400 INVALID_FIELD on list ` + + `search (#4254), not just a quietly narrowed one.` + + (known.size > 0 ? ` Object fields: ${[...known].sort().join(', ')}.` : ''), + }); + } + return findings; +} + /** * Validate every `searchableFields` declaration in the stack — the object's own * (the canonical set, ADR-0061) and the list views that narrow it. Returns * findings (empty = clean). + * + * The react page surface (``) is deliberately + * NOT walked here: its declaration lives inside JSX source, and + * `validate-react-page-props` — the gate that already parses that source — + * runs the same `checkSearchableFieldList` core on it (#4329). */ export function validateSearchableFields(stack: AnyRec): SearchableFieldFinding[] { const findings: SearchableFieldFinding[] = []; if (!isRec(stack)) return findings; - // object name → declared field names. `null` marks an object with no readable - // field map, so "declared nothing" stays distinguishable from "not in stack". const objects = asArray(stack.objects); - const fieldsByObject = new Map | null>(); - for (const obj of objects) { - const name = strName(obj.name); - if (name) fieldsByObject.set(name, declaredFieldNames(obj)); - } + const fieldsByObject = indexObjectSearchTargets(stack); - /** - * Check one `searchableFields` array against `objectName`'s field map. - * `subject` names the declaration for the message, since an object's own - * set and a view's narrowing of it are fixed differently. - */ const check = ( declared: unknown, objectName: string | undefined, @@ -199,44 +274,9 @@ export function validateSearchableFields(stack: AnyRec): SearchableFieldFinding[ path: string, subject: string, ) => { - if (!Array.isArray(declared) || declared.length === 0) return; - if (!objectName) return; // nothing to resolve against - if (!fieldsByObject.has(objectName)) return; // ① object from another package - const known = fieldsByObject.get(objectName); - if (!known) return; // ② external / introspected — no authored field map - - for (let i = 0; i < declared.length; i++) { - const entry = declared[i]; - // Pre-parse input may carry junk here; a non-string is a SHAPE error the - // schema owns, not a dangling reference. - const name = strName(entry); - if (!name) continue; - if (known.has(name) || SYSTEM_FIELDS.has(name)) continue; // ③ system column - - const dotted = name.includes('.'); - findings.push({ - severity: 'error', - rule: SEARCHABLE_FIELD_UNKNOWN, - where, - path: `${path}[${i}]`, - message: - `${subject} entry "${name}" is not a field on object "${objectName}". ` + - `The declaration is stale: searching it can never match, and the engine ` + - `silently drops it — leaving a narrower search than declared, or the ` + - `auto-default set once every entry is dropped.` + - (dotted ? '' : suggest(name, known)), - hint: - (dotted - ? `'search' scans this object's own columns, so a related record's ` + - `column cannot be a search target — expand the relation and search ` + - `the related object, or copy the value onto a formula field here. ` - : `Fix the name, or add "${name}" to ${objectName}.fields. `) + - `Clients echo this declaration verbatim as the '$searchFields' ` + - `override, so a stale entry becomes a 400 INVALID_FIELD on list ` + - `search (#4254), not just a quietly narrowed one.` + - (known.size > 0 ? ` Object fields: ${[...known].sort().join(', ')}.` : ''), - }); - } + findings.push( + ...checkSearchableFieldList(declared, objectName, fieldsByObject, where, path, subject), + ); }; // ── The object's own canonical set, and its built-in named list views ──