Skip to content

Commit d753744

Browse files
fix(lint): waive only a MISSING object beside a dataSource binding (#20454)
Fixes #20400 Clause-②: no ## What was wrong `validateComponentProps` does not report the props schema's required `object` when the component carries a `dataSource.object` binding. Two docblocks scope that waiver to absence: `DATASOURCE_SUPPLIED_PROP` ("the one prop whose absence this rule does NOT report") and `suppliedByDataSource` ("is this issue 'the required `object` prop is missing'"). The code matched on the issue's path alone. Any issue at exactly `['object']` was waived whenever `dataSource.object` was a non-empty string, without looking at `properties.object` or at the issue. So a present but wrong `object` beside a binding was silenced, and the same value without a binding was reported. ## Before and after (measured) Each row is one component in a one-page stack, run through `validateComponentProps` from source with `tsx`. Before is `origin/main` `6e3e5462c6`. After is this branch at `e994035362`. | component | before | after | |---|---|---| | `element:number`, `dataSource: { object: 'contact' }`, `properties: { object: 7, aggregate: 'count' }` | **0 findings** | 1 `component-props-invalid` at `properties.object` | | the same, with no `dataSource` (control) | 1 `component-props-invalid` at `properties.object` | the same, unchanged | | `object: null` beside the binding | **0 findings** | 1 `component-props-invalid` at `properties.object` | | no `object` key beside the binding | 0 | 0 (still waived) | | `object: undefined` beside the binding | 0 | 0 (still waived) | | the existing picker pin (`element:record_picker`, binding, `labelField` only) | 0 | 0 | ## The change The landing site is the expected one: the body of `suppliedByDataSource` in `packages/lint/src/validate-component-props.ts`, plus its test file and a changeset. After the existing path and binding checks, the function reads absence off the component. It waives only when `properties.object` is not present or is `undefined`, which is triage's definition of missing. Any other issue at that path passes through as the props row raised it. Both docblocks are unchanged: their contract sentences already say this, and the renderer sentence is out of scope (see Acceptance notes). The call site is unchanged. ## Pins (`packages/lint/src/validate-component-props.test.ts`) - The existing `does not report the required object prop when dataSource supplies it` is unchanged and green. - New, run for `object: 7` and for `object: null`: the same `element:number` bag is judged beside `dataSource: { object: 'contact' }` and without it. Each is reported as `[COMPONENT_PROPS_INVALID, '...properties.object']`, and the two finding lists are deep-equal. The assertions are on rule id, path and equality, never on message text. - New: `object: undefined` beside the binding reports nothing. The same bag without the binding reports `properties.object`, so the silence comes from the waiver and not from the row accepting `undefined`. ## Reverse verification The fix was committed first (`e994035362`). The one predicate line was then mutated back to the old behaviour through `scripts/ablation-replace.mjs` in WRAP mode, with an absolute-path `trap` restore around it. The line `return props?.[DATASOURCE_SUPPLIED_PROP] === undefined;` became `return true;`, which equals the old `strName(dataSource?.object) !== undefined` once the new early return has run. - On-disk proof: anchor count 1 → 0, mutant count 0 → 1, blob `f4793c5782` → `52df0d1033`. - The run: `Tests 2 failed | 46 passed (48)`. The two failures are the `object: 7` and `object: null` pins (`expected [] to deeply equal [ [ 'component-props-invalid', …(1) ] ]`). The `undefined` pin and the existing pin stayed green, as expected, because the old code waived both. - The restore: blob after restore `f4793c5782` equals the HEAD blob, `git diff HEAD` is 0 bytes, and `git status --porcelain` is empty. - The test imports the rule by relative path (`./validate-component-props.js`, resolved to `src/`), so no `dist/` leg applies. ## Verification (all at `e994035362`) - Build: `pnpm --filter '@objectstack/lint^...' build` through `os-verify-lock.sh`, `VERDICT command-exit 0`. - `pnpm --filter @objectstack/lint exec vitest run --maxWorkers=2 src/validate-component-props.test.ts`: `Tests 48 passed (48)`. - `pnpm --filter @objectstack/lint test`: `Test Files 115 passed (115)`, `Tests 5329 passed | 5 skipped (5334)`. - `pnpm --filter @objectstack/lint typecheck`: exit 0. `tsc --noEmit` is clean. `check:test-typecheck` is OK with the ledger unchanged at 2 files, 6 errors and 2 signatures. `--listFiles` shows the edited test file is in the `tsconfig.test.json` program (1 hit) and is not a ledgered file. - `node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands` derived 60 commands. Each was run with its exit code captured before any pipe, and the results were reconciled with `--ran`: `60 derived famil(ies) accounted for — 58 run, 2 NOT-MEASURED (2 DERIVED from a recorded exit 3)`. - 58 exited 0. That count includes `check:doc-authoring`, `check:nul-bytes`, `check:adr-0087-registration --base origin/main`, `check:changeset-no-major --base origin/main`, `check:empty-changeset` and `check:changeset-gate-self-tests`. - `check:docs-transcript-drift` and `check:lean-entry-closure` first answered `PREREQUISITE NOT MET`. They exited 0 after `pnpm --filter @objectstack/lint build` and `turbo run build --filter=@objectstack/objectql`. - NOT MEASURED: `check:dual-build-cjs-loads` and `check:type-check-debt`. Reason: both refuse without the whole workspace built (84 and 28 packages have no `dist/`). That build is CI's (`Build Core`, `Lint & Repo Gates`), not a local targeted run. - The four roster gates the derivation flags for directories this diff touches all exited 0: `check-changeset-fixed`, `check:authz-resolver`, `check:error-code-casing` and `check:filter-alias-parity`. - Published surface: `@objectstack/lint` ships `files: ["dist", "README.md", "CHANGELOG.md"]`. After a build, the new line (built spelling `return props?.[DATASOURCE_SUPPLIED_PROP] === void 0;`) appears once in each of `dist/index.js`, `dist/index.cjs`, `dist/runtime.js` and `dist/runtime.cjs`. The positive control `function suppliedByDataSource(issue, component)` also appears once in each. So this publishes, and it takes a `patch` changeset. - Diff size: +84 / -1 across 3 files. ## Acceptance notes - **`null` is reported.** Triage defined missing as "no key, or `undefined`". `object: null` is a present value that `z.string()` rejects, so it now reaches the author, like `object: 7`. It is pinned beside `7` so the boundary stays explicit. - **The renderer sentence is untouched.** `DATASOURCE_SUPPLIED_PROP`'s docblock says objectui's element renderers "read it FIRST". That is still false for `element:number` until objectui#10909 lands. objectui#10909 is not addressed here: its fix is that renderer, per triage's out-of-scope list. - **No per-type table.** Restricting the waiver to types whose renderer honours the binding would be a new per-type table on this rule. Triage ruled it out of this card. - **The two readers now agree on this input.** objectui's mirror of this waiver (PR objectui#10908) already refuses `object: 7` beside a binding, following the docblock. After this change, this gate refuses it as well. - **`main` moved after the base.** It gained `7fa3e3e07c` and `8cdbe0c6e5`, which touch `packages/rest` and `packages/metadata-protocol` only. Neither touches `packages/lint` or `packages/spec`, so the branch was not merged forward and the merge queue arbitrates. - **PR #20420 is text-disjoint.** It adds `ComponentPropsMap` rows that this rule reads, but it does not touch this file. --- _Generated by [Claude Code](https://claude.ai/code/session_014EJ1ED8X4MMrT18BhVx4tx)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent bea6d2e commit d753744

3 files changed

Lines changed: 86 additions & 1 deletion

File tree

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
---
2+
'@objectstack/lint': patch
3+
---
4+
5+
`component-props-invalid` no longer hides a wrong `object` prop on a component that also carries a `dataSource` binding
6+
7+
Clause-②: no
8+
9+
A page component whose `dataSource.object` names the object may leave the flat
10+
`properties.object` shorthand out: the binding supplies it, so the rule does not
11+
report the props schema's required `object` as missing. That waiver was matched
12+
on the issue's path alone, so it also swallowed every other issue the props
13+
schema raised at `object`. A present but wrong value, such as `object: 7` or
14+
`object: null`, was reported without a binding and silently passed with one.
15+
16+
The waiver now covers what its contract says: a missing `object`, meaning no
17+
key or an explicit `undefined`. A value the author did write is judged as
18+
written, and it is reported at `properties.object` exactly as it is on the same
19+
component without a binding.
20+
21+
Effect on `os validate`, `os lint` and `os build`: a document that sets both a
22+
`dataSource` binding and a wrong-typed `properties.object` now gets one
23+
`component-props-invalid` warning it did not get before. The rule stays
24+
advisory: without `--strict` nothing that validated before is refused; under
25+
`os validate --strict` or `os lint --strict` the new warning fails the run, as
26+
every warning does. A component that binds
27+
through `dataSource` and omits `properties.object` is still clean.

‎packages/lint/src/validate-component-props.test.ts‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -231,6 +231,58 @@ describe('validateComponentProps — value verdicts', () => {
231231
]);
232232
});
233233

234+
/**
235+
* The waiver above covers a MISSING `object` only — no key, or `undefined`.
236+
* A present value the row rejects is the author's own, and the binding
237+
* supplies nothing in its place, so the row's verdict on it must reach the
238+
* author exactly as it does with no binding at all. Each case is judged
239+
* twice, beside the binding and without it, and the two answers must be the
240+
* same finding: equality rather than wording, so the pin measures that the
241+
* waiver lets the row's issue through unchanged.
242+
*/
243+
it.each([
244+
['a number', 7],
245+
['null', null],
246+
])('reports a present-but-wrong `object` (%s) beside a `dataSource` binding, as it does without one', (_label, value) => {
247+
const component = { type: 'element:number', properties: { object: value, aggregate: 'count' } };
248+
const withBinding = validateComponentProps(
249+
stackWith([{ ...component, dataSource: { object: 'contact' } }]),
250+
);
251+
const without = validateComponentProps(stackWith([component]));
252+
253+
// Control first: with nothing supplying `object`, the row reports it.
254+
expect(without.map((f) => [f.rule, f.path])).toEqual([
255+
[COMPONENT_PROPS_INVALID, 'pages[0].regions[0].components[0].properties.object'],
256+
]);
257+
// The binding does not silence it.
258+
expect(withBinding.map((f) => [f.rule, f.path])).toEqual([
259+
[COMPONENT_PROPS_INVALID, 'pages[0].regions[0].components[0].properties.object'],
260+
]);
261+
expect(withBinding).toEqual(without);
262+
});
263+
264+
it('still waives `object: undefined` beside a binding — an explicit undefined is a missing value', () => {
265+
const findings = validateComponentProps(
266+
stackWith([
267+
{
268+
type: 'element:number',
269+
dataSource: { object: 'contact' },
270+
properties: { object: undefined, aggregate: 'count' },
271+
},
272+
]),
273+
);
274+
expect(findings).toEqual([]);
275+
276+
// …and the same bag with no binding is reported, so the silence above is
277+
// the waiver and not the row accepting `undefined`.
278+
const without = validateComponentProps(
279+
stackWith([{ type: 'element:number', properties: { object: undefined, aggregate: 'count' } }]),
280+
);
281+
expect(invalid(without).map((f) => f.path)).toEqual([
282+
'pages[0].regions[0].components[0].properties.object',
283+
]);
284+
});
285+
234286
/**
235287
* #5775 — the retirement's author-facing channel. `displayField` was the
236288
* picker's REQUIRED prop and no renderer ever read it, so the tombstone's

‎packages/lint/src/validate-component-props.ts‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,13 @@ const DATASOURCE_SUPPLIED_PROP = 'object';
175175
function suppliedByDataSource(issue: LintZodIssue, component: AnyRec): boolean {
176176
if (issue.path.length !== 1 || issue.path[0] !== DATASOURCE_SUPPLIED_PROP) return false;
177177
const dataSource = isRec(component.dataSource) ? component.dataSource : undefined;
178-
return strName(dataSource?.object) !== undefined;
178+
if (strName(dataSource?.object) === undefined) return false;
179+
// "Missing" is read off the component, never off the issue: the path alone
180+
// also matches a PRESENT value the row rejects (`object: 7`, `object: null`),
181+
// and the binding supplies nothing there — the author wrote that value and
182+
// the row's own verdict on it stands. Only no key, or `undefined`, is waived.
183+
const props = isRec(component.properties) ? component.properties : undefined;
184+
return props?.[DATASOURCE_SUPPLIED_PROP] === undefined;
179185
}
180186

181187
/**

0 commit comments

Comments
 (0)