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
35 changes: 35 additions & 0 deletions .changeset/react-listview-searchable-fields-lint.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
---
"@objectstack/lint": minor
---

feat(lint): `<ListView searchableFields>` 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
`<ListView searchableFields={['renamed_field']}>` 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 `<ListView>` 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).
107 changes: 107 additions & 0 deletions packages/lint/src/validate-react-page-props.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 }] });

Expand Down Expand Up @@ -240,3 +241,109 @@ describe('validateReactPageProps — <ObjectChart> bindings (#3701)', () => {
expect(f).toEqual([]);
});
});

// ─────────────────────────────────────────────────────────────────────────
// <ListView> 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 <ListView ${attrs} />; }`;

describe('validateReactPageProps — <ListView> 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" › <ListView>');
// 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 <ListView objectName="crm_account" searchableFields={sf} />; }',
),
);
expect(f).toEqual([]);
});

it('skips everything behind a spread (props may come from it)', () => {
const f = validateReactPageProps(
listPage(
'function Page(){ const p = {}; return <ListView objectName="crm_account" {...p} searchableFields={["nope"]} />; }',
),
);
expect(f).toEqual([]);
});
});
30 changes: 30 additions & 0 deletions packages/lint/src/validate-react-page-props.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@
// - <ObjectChart>'s data BINDINGS, by reading the attribute VALUES (#3701,
// retargeted to the spec shape in #3729). See the block comment above
// `checkObjectChart`.
// - <ListView>'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
Expand All @@ -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
Expand Down Expand Up @@ -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];
Expand Down Expand Up @@ -454,6 +466,24 @@ export function validateReactPageProps(stack: AnyRec): ReactPropFinding[] {
if (tag === 'ObjectChart' && !hasSpread) {
checkObjectChart({ values, where, path }, objectFields, findings);
}
// <ListView searchableFields> 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);
Expand Down
140 changes: 90 additions & 50 deletions packages/lint/src/validate-searchable-fields.ts
Original file line number Diff line number Diff line change
Expand Up @@ -169,74 +169,114 @@ 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<string, unknown>,
): Map<string, Set<string> | null> {
const fieldsByObject = new Map<string, Set<string> | 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
* (`<ListView searchableFields={…}>`, `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<string, Set<string> | 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 (`<ListView searchableFields={…}>`) 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<string, Set<string> | 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,
where: string,
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 ──
Expand Down
Loading