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
38 changes: 38 additions & 0 deletions .changeset/3909-query-params-filter-union.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
---
'@object-ui/types': patch
'@object-ui/fields': patch
---

`QueryParams.$filter` now declares both shapes the data sources actually accept — the
MongoDB-style field-keyed record, or a `FilterArray`, the ObjectQL AST sugar bound from
`@objectstack/spec/data` (objectui#3909).

**Nothing is narrowed and no accepted value changes.** `Record<string, any>` already
accepted arrays structurally — they satisfy its string index — so the union documents
shapes that were always legal rather than admitting new ones. Measured both ways under
`tsc --strict`: all five inputs `translateFilterToAST` enumerates assign to the old and
new declarations alike, and both reject a bare number and a bare string identically. A
downstream `turbo run build` over all 43 dependent packages is green, which is the
evidence a published type change breaks no consumer.

The harm was entirely on the type face, and it was two-sided. The declaration blocked
nothing while describing one legal shape as though it were the only one — objectui#3831
is what that cost, a rule array accepted by a `Record<string, any>` slot, object-spread
flattened to `{"0": {...}}`, types green, and the query filtering on a column literally
named `0`. And someone writing a new consumer would read the type and its record-only
`@example`, conclude the array path was illegal, and add a tolerant conversion for it —
the "widen the consumer to tolerate the producer" shape AGENTS.md #0.1 forbids. Two
producers have fed arrays through this slot all along: `plugin-list`'s
`buildEffectiveFilter` (grid and export) and `plugin-view`'s `ObjectView` (calendar /
kanban / gallery / timeline). The runtime was right; the declaration was narrow.

The array half is **bound** to the spec's `FilterArray` rather than restated locally, so
it cannot fork from the vocabulary the servers parse — the same failure two hand-written
operator lists had in objectui#3948. The doc comment names `translateFilterToAST` as the
authoritative accepted set instead of carrying a second list to drift from.

`@object-ui/fields` drops the local cast this defect forced. PR objectui#3908 wrote
`filter as Record<string, any>` at one assignment in `useRecordQuery`, deliberately, as
debt rather than widening the shared type. `hasFilter` is now a type predicate narrowing
to the `$filter` slot's own type, so the assignment needs no cast and the guard cannot
drift from the declaration it guards. Type-only throughout; no runtime behaviour changes.
20 changes: 13 additions & 7 deletions packages/fields/src/widgets/useRecordQuery.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -111,8 +111,12 @@ export interface UseRecordQueryResult {
* `Object.keys` on an array returns its INDICES — so the record-only test read
* `['0','1','2']` for an AST node and was right only by accident. An empty array
* is "no filter" for the same reason an empty object is.
*
* Narrows to the `$filter` slot's own type rather than a restatement of it
* (#3909), so the caller assigns with no cast and this predicate cannot drift
* from the declaration it is guarding.
*/
function hasFilter(filter: unknown): boolean {
function hasFilter(filter: unknown): filter is NonNullable<QueryParams['$filter']> {
if (filter === null || filter === undefined) return false;
if (Array.isArray(filter)) return filter.length > 0;
if (typeof filter !== 'object') return false;
Expand DownExpand Up@@ -172,12 +176,14 @@ export function useRecordQuery(options: UseRecordQueryOptions): UseRecordQueryRe
if (searchTerm && searchTerm.trim()) params.$search = searchTerm.trim();
if (searchFields && searchFields.length > 0) params.$searchFields = searchFields;
if (sortArg) params.$orderby = { [sortArg.field]: sortArg.direction };
// `QueryParams.$filter` is declared `Record< string, any >`, which the
// AST-array form does not describe — the cast is at this ONE assignment
// rather than widening a shared type that several other producers
// (plugin-list's `buildEffectiveFilter`, plugin-view's ObjectView)
// already feed arrays through.
if (hasFilter(filter)) params.$filter = filter as Record<string, any>;
// No cast: `QueryParams.$filter` now declares both shapes it accepts
// (#3909), so the AST-array form the picker's merge yields is describable
// here. The local cast this replaces was deliberate debt — taken at this
// ONE assignment rather than widening the shared type that several other
// producers (plugin-list's `buildEffectiveFilter`, plugin-view's
// ObjectView) already feed arrays through. The shared type is honest now,
// so the debt is paid rather than moved.
if (hasFilter(filter)) params.$filter = filter;
if (expand && expand.length > 0) params.$expand = expand;

const result = await dataSource.find(objectName, params);
Expand Down
131 changes: 131 additions & 0 deletions packages/types/src/__tests__/query-params-filter-union.test.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
/**
* ObjectUI
* Copyright (c) 2024-present ObjectStack Inc.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

/**
* objectui#3909 — `QueryParams.$filter` declares BOTH shapes the data sources
* accept, and binds the array half to `@objectstack/spec`'s `FilterArray`
* rather than restating it.
*
* ## Why this file exists at all
*
* The defect it pins was invisible to every runtime suite, and had to be: the
* declaration was `Record< string, any >`, which **structurally accepts arrays**
* (they satisfy its string index). So the two producers that have fed ObjectQL
* AST arrays through this slot all along — `plugin-list`'s
* `buildEffectiveFilter` (grid and export) and `plugin-view`'s `ObjectView`
* (calendar / kanban / gallery / timeline) — type-checked, ran, and shipped
* correct results. Nothing was broken at runtime and nothing could go red.
*
* The cost was paid on the type face instead, in both directions:
*
* 1. The type **blocked nothing while describing one legal shape as if it were
* the only one**. objectui#3831 is what that buys: a `Record< string, any >`
* slot accepted a rule array, an object spread flattened it to
* `{"0": {...}}`, types stayed green, and the query filtered on a column
* literally named `0`.
* 2. Someone writing a new consumer reads the type and its `@example`,
* concludes only the record form is legal, and adds a tolerant conversion
* for the array path — the "widen the consumer to tolerate the producer"
* shape AGENTS.md #0.1 forbids.
*
* Both failure modes are compile-time by nature, so the pins are too. Reverting
* `$filter` to `Record< string, any >` leaves every runtime suite green and
* turns THIS FILE red under `tsc -p tsconfig.test.json` (the `type-check`
* script) — the drift's own signature, reproduced deliberately.
*
* ## What is NOT pinned here
*
* That the union is the *authoritative* accepted set. It is not: the authority
* is `translateFilterToAST` (`@object-ui/data-objectstack`), which enumerates
* five input shapes. A second list here would be a third place to drift from —
* which is exactly how two operator vocabularies came apart in #3948. The
* binding below is to the spec's `FilterArray`, so the array half cannot fork
* locally.
*/

import { describe, it, expect } from 'vitest';
import type { FilterArray } from '@objectstack/spec/data';
import type { QueryParams } from '../data';

type Assert< T extends true > = T;
/** True when `V` is accepted by the `$filter` slot. */
type AcceptsFilter< V > = V extends QueryParams['$filter'] ? true : false;
/** Exact type identity — NOT mutual assignability. See the note below. */
type Equal< A, B > =
(< T >() => T extends A ? 1 : 2) extends (< T >() => T extends B ? 1 : 2) ? true : false;

describe('QueryParams.$filter — declares the union it actually accepts (#3909)', () => {
it('accepts the MongoDB-style field-keyed record', () => {
type _Record = Assert< AcceptsFilter< { age: { $gt: number } } > >;
const params: QueryParams = { $filter: { age: { $gt: 18 }, status: 'active' } };
expect(params.$filter).toEqual({ age: { $gt: 18 }, status: 'active' });
});

it('accepts a bare AST comparison tuple with no cast', () => {
// The shape `buildEffectiveFilter` returns for a single condition. Before
// #3909 this compiled only because arrays satisfy `Record`'s string index —
// accepted by accident rather than by declaration.
const params: QueryParams = { $filter: ['status', '=', 'active'] };
expect(params.$filter).toEqual(['status', '=', 'active']);
});

it('accepts a logical AST group with no cast', () => {
// What `mergeFilterNodes` returns once more than one source is active.
const params: QueryParams = {
$filter: ['and', ['age', '>=', 18], ['status', '=', 'active']],
};
expect(Array.isArray(params.$filter)).toBe(true);
});

it('accepts the legacy bare list, combined with implicit AND', () => {
const params: QueryParams = {
$filter: [['stage', '=', 'won'], ['amount', '>', 1000]],
};
expect(Array.isArray(params.$filter)).toBe(true);
});

it('binds the array half to the spec, by IDENTITY not assignability', () => {
// ## Why this pin is an identity check, and why nothing weaker works
//
// Every assignment-shaped pin in this file is, on its own, VACUOUS as a
// regression guard — measured, not assumed. Reverting the declaration to
// `Record< string, any >` and re-running `type-check` leaves it GREEN
// (exit 0), because assignability cannot separate the two: arrays satisfy
// `Record< string, any >`'s string index, so `FilterArray extends
// QueryParams['$filter']` holds under BOTH declarations, and the old
// declaration is itself assignable to the new union. A guard that passes
// equally before and after the fix is a phantom check — it reads like
// enforcement and enforces nothing.
//
// Identity is the property that actually differs. This assertion goes red
// on a revert to the bare record, AND on the subtler regression: someone
// re-declaring a local `FilterNode` fork instead of binding the spec's
// type. That fork would satisfy every assignment above while being free to
// drift from the vocabulary the servers parse — the exact failure two
// hand-written operator lists had in #3948.
type _Bound = Assert<
Equal< NonNullable< QueryParams['$filter'] >, Record< string, any > | FilterArray >
>;
const fromSpec: FilterArray = ['status', '=', 'active'];
const params: QueryParams = { $filter: fromSpec };
expect(params.$filter).toBe(fromSpec);
});

it('still refuses a value that is neither shape', () => {
// The union documents; it must not have become `any` on the way. Note the
// slot sits on an interface that also carries `[key: string]: any` — these
// pins prove the declared property still wins over that index signature,
// which is the whole reason the declaration is worth anything.
// @ts-expect-error a number is not a filter
const bad: QueryParams = { $filter: 42 };
// @ts-expect-error a string is not a filter
const alsoBad: QueryParams = { $filter: 'status eq active' };
expect(bad.$filter).toBe(42);
expect(alsoBad.$filter).toBe('status eq active');
});
});
34 changes: 32 additions & 2 deletions packages/types/src/data.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -31,6 +31,7 @@ import type {
CreateExportJobInput as SpecCreateExportJobInput,
CreateExportJobResult,
} from '@objectstack/spec/contracts';
import type { FilterArray } from '@objectstack/spec/data';
import type { ValidationError } from '@objectstack/spec/kernel';

export type { ExportJobStatus, ImportJobStatus, ImportWriteMode, ValidationError };
Expand All@@ -47,10 +48,39 @@ export interface QueryParams {
$select?: string[];

/**
* Filter expression
* Filter expression, in either of the two forms the data sources accept:
* the MongoDB-style field-keyed record, or a `FilterArray` — the spec-owned
* ObjectQL AST sugar (`@objectstack/spec/data`).
*
* Both forms are normal here, and the array form is not an edge case: the
* repo's own canonical sink `mergeFilterNodes` / `toFilterNode`
* (`@object-ui/core`'s `filter-converter.ts`) returns AST nodes, and its two
* standing producers — `plugin-list`'s `buildEffectiveFilter` (grid and
* export) and `plugin-view`'s `ObjectView` (calendar / kanban / gallery /
* timeline) — have fed arrays through this slot all along.
*
* ⛔ Do not add a "tolerant conversion" in a consumer to cope with the array
* path. The array IS legal input; a consumer that needs one shape lowers
* through the shared sink rather than widening itself to tolerate the
* producer.
*
* The authoritative acceptable set is the one `translateFilterToAST`
* (`@object-ui/data-objectstack`'s `index.ts`) enumerates — five input
* shapes, of which the array forms below are three. Read it there rather than
* trusting a second list here; a partial restatement is exactly how two
* operator vocabularies drifted apart before.
*
* Note the declaration does not *narrow* anything: `Record<string, any>`
* already structurally accepts arrays (they satisfy its string index), so the
* union documents the shapes that were always accepted rather than admitting
* new ones. It is the description that was wrong, not the runtime.
*
* @example { age: { $gt: 18 }, status: 'active' }
* @example ['status', '=', 'active']
* @example ['and', ['age', '>=', 18], ['status', '=', 'active']]
* @example [['stage', '=', 'won'], ['amount', '>', 1000]]
*/
$filter?: Record<string, any>;
$filter?: Record<string, any> | FilterArray;

/**
* Sort order
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
fix(types): declare QueryParams.$filter as the union it already accepts by yinlianghui · Pull Request #5999 · objectstack-ai/objectui · GitHub
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
38 changes: 38 additions & 0 deletions .changeset/3909-query-params-filter-union.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
---
'@object-ui/types': patch
'@object-ui/fields': patch
---

`QueryParams.$filter` now declares both shapes the data sources actually accept — the
MongoDB-style field-keyed record, or a `FilterArray`, the ObjectQL AST sugar bound from
`@objectstack/spec/data` (objectui#3909).

**Nothing is narrowed and no accepted value changes.** `Record<string, any>` already
accepted arrays structurally — they satisfy its string index — so the union documents
shapes that were always legal rather than admitting new ones. Measured both ways under
`tsc --strict`: all five inputs `translateFilterToAST` enumerates assign to the old and
new declarations alike, and both reject a bare number and a bare string identically. A
downstream `turbo run build` over all 43 dependent packages is green, which is the
evidence a published type change breaks no consumer.

The harm was entirely on the type face, and it was two-sided. The declaration blocked
nothing while describing one legal shape as though it were the only one — objectui#3831
is what that cost, a rule array accepted by a `Record<string, any>` slot, object-spread
flattened to `{"0": {...}}`, types green, and the query filtering on a column literally
named `0`. And someone writing a new consumer would read the type and its record-only
`@example`, conclude the array path was illegal, and add a tolerant conversion for it —
the "widen the consumer to tolerate the producer" shape AGENTS.md #0.1 forbids. Two
producers have fed arrays through this slot all along: `plugin-list`'s
`buildEffectiveFilter` (grid and export) and `plugin-view`'s `ObjectView` (calendar /
kanban / gallery / timeline). The runtime was right; the declaration was narrow.

The array half is **bound** to the spec's `FilterArray` rather than restated locally, so
it cannot fork from the vocabulary the servers parse — the same failure two hand-written
operator lists had in objectui#3948. The doc comment names `translateFilterToAST` as the
authoritative accepted set instead of carrying a second list to drift from.

`@object-ui/fields` drops the local cast this defect forced. PR objectui#3908 wrote
`filter as Record<string, any>` at one assignment in `useRecordQuery`, deliberately, as
debt rather than widening the shared type. `hasFilter` is now a type predicate narrowing
to the `$filter` slot's own type, so the assignment needs no cast and the guard cannot
drift from the declaration it guards. Type-only throughout; no runtime behaviour changes.
20 changes: 13 additions & 7 deletions packages/fields/src/widgets/useRecordQuery.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -111,8 +111,12 @@ export interface UseRecordQueryResult {
* `Object.keys` on an array returns its INDICES — so the record-only test read
* `['0','1','2']` for an AST node and was right only by accident. An empty array
* is "no filter" for the same reason an empty object is.
*
* Narrows to the `$filter` slot's own type rather than a restatement of it
* (#3909), so the caller assigns with no cast and this predicate cannot drift
* from the declaration it is guarding.
*/
function hasFilter(filter: unknown): boolean {
function hasFilter(filter: unknown): filter is NonNullable<QueryParams['$filter']> {
if (filter === null || filter === undefined) return false;
if (Array.isArray(filter)) return filter.length > 0;
if (typeof filter !== 'object') return false;
Expand DownExpand Up@@ -172,12 +176,14 @@ export function useRecordQuery(options: UseRecordQueryOptions): UseRecordQueryRe
if (searchTerm && searchTerm.trim()) params.$search = searchTerm.trim();
if (searchFields && searchFields.length > 0) params.$searchFields = searchFields;
if (sortArg) params.$orderby = { [sortArg.field]: sortArg.direction };
// `QueryParams.$filter` is declared `Record< string, any >`, which the
// AST-array form does not describe — the cast is at this ONE assignment
// rather than widening a shared type that several other producers
// (plugin-list's `buildEffectiveFilter`, plugin-view's ObjectView)
// already feed arrays through.
if (hasFilter(filter)) params.$filter = filter as Record<string, any>;
// No cast: `QueryParams.$filter` now declares both shapes it accepts
// (#3909), so the AST-array form the picker's merge yields is describable
// here. The local cast this replaces was deliberate debt — taken at this
// ONE assignment rather than widening the shared type that several other
// producers (plugin-list's `buildEffectiveFilter`, plugin-view's
// ObjectView) already feed arrays through. The shared type is honest now,
// so the debt is paid rather than moved.
if (hasFilter(filter)) params.$filter = filter;
if (expand && expand.length > 0) params.$expand = expand;

const result = await dataSource.find(objectName, params);
Expand Down
131 changes: 131 additions & 0 deletions packages/types/src/__tests__/query-params-filter-union.test.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
/**
* ObjectUI
* Copyright (c) 2024-present ObjectStack Inc.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

/**
* objectui#3909 — `QueryParams.$filter` declares BOTH shapes the data sources
* accept, and binds the array half to `@objectstack/spec`'s `FilterArray`
* rather than restating it.
*
* ## Why this file exists at all
*
* The defect it pins was invisible to every runtime suite, and had to be: the
* declaration was `Record< string, any >`, which **structurally accepts arrays**
* (they satisfy its string index). So the two producers that have fed ObjectQL
* AST arrays through this slot all along — `plugin-list`'s
* `buildEffectiveFilter` (grid and export) and `plugin-view`'s `ObjectView`
* (calendar / kanban / gallery / timeline) — type-checked, ran, and shipped
* correct results. Nothing was broken at runtime and nothing could go red.
*
* The cost was paid on the type face instead, in both directions:
*
* 1. The type **blocked nothing while describing one legal shape as if it were
* the only one**. objectui#3831 is what that buys: a `Record< string, any >`
* slot accepted a rule array, an object spread flattened it to
* `{"0": {...}}`, types stayed green, and the query filtered on a column
* literally named `0`.
* 2. Someone writing a new consumer reads the type and its `@example`,
* concludes only the record form is legal, and adds a tolerant conversion
* for the array path — the "widen the consumer to tolerate the producer"
* shape AGENTS.md #0.1 forbids.
*
* Both failure modes are compile-time by nature, so the pins are too. Reverting
* `$filter` to `Record< string, any >` leaves every runtime suite green and
* turns THIS FILE red under `tsc -p tsconfig.test.json` (the `type-check`
* script) — the drift's own signature, reproduced deliberately.
*
* ## What is NOT pinned here
*
* That the union is the *authoritative* accepted set. It is not: the authority
* is `translateFilterToAST` (`@object-ui/data-objectstack`), which enumerates
* five input shapes. A second list here would be a third place to drift from —
* which is exactly how two operator vocabularies came apart in #3948. The
* binding below is to the spec's `FilterArray`, so the array half cannot fork
* locally.
*/

import { describe, it, expect } from 'vitest';
import type { FilterArray } from '@objectstack/spec/data';
import type { QueryParams } from '../data';

type Assert< T extends true > = T;
/** True when `V` is accepted by the `$filter` slot. */
type AcceptsFilter< V > = V extends QueryParams['$filter'] ? true : false;
/** Exact type identity — NOT mutual assignability. See the note below. */
type Equal< A, B > =
(< T >() => T extends A ? 1 : 2) extends (< T >() => T extends B ? 1 : 2) ? true : false;

describe('QueryParams.$filter — declares the union it actually accepts (#3909)', () => {
it('accepts the MongoDB-style field-keyed record', () => {
type _Record = Assert< AcceptsFilter< { age: { $gt: number } } > >;
const params: QueryParams = { $filter: { age: { $gt: 18 }, status: 'active' } };
expect(params.$filter).toEqual({ age: { $gt: 18 }, status: 'active' });
});

it('accepts a bare AST comparison tuple with no cast', () => {
// The shape `buildEffectiveFilter` returns for a single condition. Before
// #3909 this compiled only because arrays satisfy `Record`'s string index —
// accepted by accident rather than by declaration.
const params: QueryParams = { $filter: ['status', '=', 'active'] };
expect(params.$filter).toEqual(['status', '=', 'active']);
});

it('accepts a logical AST group with no cast', () => {
// What `mergeFilterNodes` returns once more than one source is active.
const params: QueryParams = {
$filter: ['and', ['age', '>=', 18], ['status', '=', 'active']],
};
expect(Array.isArray(params.$filter)).toBe(true);
});

it('accepts the legacy bare list, combined with implicit AND', () => {
const params: QueryParams = {
$filter: [['stage', '=', 'won'], ['amount', '>', 1000]],
};
expect(Array.isArray(params.$filter)).toBe(true);
});

it('binds the array half to the spec, by IDENTITY not assignability', () => {
// ## Why this pin is an identity check, and why nothing weaker works
//
// Every assignment-shaped pin in this file is, on its own, VACUOUS as a
// regression guard — measured, not assumed. Reverting the declaration to
// `Record< string, any >` and re-running `type-check` leaves it GREEN
// (exit 0), because assignability cannot separate the two: arrays satisfy
// `Record< string, any >`'s string index, so `FilterArray extends
// QueryParams['$filter']` holds under BOTH declarations, and the old
// declaration is itself assignable to the new union. A guard that passes
// equally before and after the fix is a phantom check — it reads like
// enforcement and enforces nothing.
//
// Identity is the property that actually differs. This assertion goes red
// on a revert to the bare record, AND on the subtler regression: someone
// re-declaring a local `FilterNode` fork instead of binding the spec's
// type. That fork would satisfy every assignment above while being free to
// drift from the vocabulary the servers parse — the exact failure two
// hand-written operator lists had in #3948.
type _Bound = Assert<
Equal< NonNullable< QueryParams['$filter'] >, Record< string, any > | FilterArray >
>;
const fromSpec: FilterArray = ['status', '=', 'active'];
const params: QueryParams = { $filter: fromSpec };
expect(params.$filter).toBe(fromSpec);
});

it('still refuses a value that is neither shape', () => {
// The union documents; it must not have become `any` on the way. Note the
// slot sits on an interface that also carries `[key: string]: any` — these
// pins prove the declared property still wins over that index signature,
// which is the whole reason the declaration is worth anything.
// @ts-expect-error a number is not a filter
const bad: QueryParams = { $filter: 42 };
// @ts-expect-error a string is not a filter
const alsoBad: QueryParams = { $filter: 'status eq active' };
expect(bad.$filter).toBe(42);
expect(alsoBad.$filter).toBe('status eq active');
});
});
34 changes: 32 additions & 2 deletions packages/types/src/data.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -31,6 +31,7 @@ import type {
CreateExportJobInput as SpecCreateExportJobInput,
CreateExportJobResult,
} from '@objectstack/spec/contracts';
import type { FilterArray } from '@objectstack/spec/data';
import type { ValidationError } from '@objectstack/spec/kernel';

export type { ExportJobStatus, ImportJobStatus, ImportWriteMode, ValidationError };
Expand All@@ -47,10 +48,39 @@ export interface QueryParams {
$select?: string[];

/**
* Filter expression
* Filter expression, in either of the two forms the data sources accept:
* the MongoDB-style field-keyed record, or a `FilterArray` — the spec-owned
* ObjectQL AST sugar (`@objectstack/spec/data`).
*
* Both forms are normal here, and the array form is not an edge case: the
* repo's own canonical sink `mergeFilterNodes` / `toFilterNode`
* (`@object-ui/core`'s `filter-converter.ts`) returns AST nodes, and its two
* standing producers — `plugin-list`'s `buildEffectiveFilter` (grid and
* export) and `plugin-view`'s `ObjectView` (calendar / kanban / gallery /
* timeline) — have fed arrays through this slot all along.
*
* ⛔ Do not add a "tolerant conversion" in a consumer to cope with the array
* path. The array IS legal input; a consumer that needs one shape lowers
* through the shared sink rather than widening itself to tolerate the
* producer.
*
* The authoritative acceptable set is the one `translateFilterToAST`
* (`@object-ui/data-objectstack`'s `index.ts`) enumerates — five input
* shapes, of which the array forms below are three. Read it there rather than
* trusting a second list here; a partial restatement is exactly how two
* operator vocabularies drifted apart before.
*
* Note the declaration does not *narrow* anything: `Record<string, any>`
* already structurally accepts arrays (they satisfy its string index), so the
* union documents the shapes that were always accepted rather than admitting
* new ones. It is the description that was wrong, not the runtime.
*
* @example { age: { $gt: 18 }, status: 'active' }
* @example ['status', '=', 'active']
* @example ['and', ['age', '>=', 18], ['status', '=', 'active']]
* @example [['stage', '=', 'won'], ['amount', '>', 1000]]
*/
$filter?: Record<string, any>;
$filter?: Record<string, any> | FilterArray;

/**
* Sort order
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Force GitHub README to respect dark mode (function() { var style = document.createElement('style'); style.textContent = ' .markdown-body { color-scheme: dark light; } .markdown-body pre { background: #161b22 !important; } .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; } .markdown-body table th, .markdown-body table td { border-color: #30363d !important; } .markdown-body img { background: #0d1117; } .markdown-body blockquote { border-left-color: #8b949e; } .markdown-body hr { border-color: #30363d; } '; document.head.appendChild(style); })(); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' fix(types): declare QueryParams.$filter as the union it already accepts by yinlianghui · Pull Request #5999 · objectstack-ai/objectui · GitHub
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
38 changes: 38 additions & 0 deletions .changeset/3909-query-params-filter-union.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
---
'@object-ui/types': patch
'@object-ui/fields': patch
---

`QueryParams.$filter` now declares both shapes the data sources actually accept — the
MongoDB-style field-keyed record, or a `FilterArray`, the ObjectQL AST sugar bound from
`@objectstack/spec/data` (objectui#3909).

**Nothing is narrowed and no accepted value changes.** `Record<string, any>` already
accepted arrays structurally — they satisfy its string index — so the union documents
shapes that were always legal rather than admitting new ones. Measured both ways under
`tsc --strict`: all five inputs `translateFilterToAST` enumerates assign to the old and
new declarations alike, and both reject a bare number and a bare string identically. A
downstream `turbo run build` over all 43 dependent packages is green, which is the
evidence a published type change breaks no consumer.

The harm was entirely on the type face, and it was two-sided. The declaration blocked
nothing while describing one legal shape as though it were the only one — objectui#3831
is what that cost, a rule array accepted by a `Record<string, any>` slot, object-spread
flattened to `{"0": {...}}`, types green, and the query filtering on a column literally
named `0`. And someone writing a new consumer would read the type and its record-only
`@example`, conclude the array path was illegal, and add a tolerant conversion for it —
the "widen the consumer to tolerate the producer" shape AGENTS.md #0.1 forbids. Two
producers have fed arrays through this slot all along: `plugin-list`'s
`buildEffectiveFilter` (grid and export) and `plugin-view`'s `ObjectView` (calendar /
kanban / gallery / timeline). The runtime was right; the declaration was narrow.

The array half is **bound** to the spec's `FilterArray` rather than restated locally, so
it cannot fork from the vocabulary the servers parse — the same failure two hand-written
operator lists had in objectui#3948. The doc comment names `translateFilterToAST` as the
authoritative accepted set instead of carrying a second list to drift from.

`@object-ui/fields` drops the local cast this defect forced. PR objectui#3908 wrote
`filter as Record<string, any>` at one assignment in `useRecordQuery`, deliberately, as
debt rather than widening the shared type. `hasFilter` is now a type predicate narrowing
to the `$filter` slot's own type, so the assignment needs no cast and the guard cannot
drift from the declaration it guards. Type-only throughout; no runtime behaviour changes.
20 changes: 13 additions & 7 deletions packages/fields/src/widgets/useRecordQuery.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -111,8 +111,12 @@ export interface UseRecordQueryResult {
* `Object.keys` on an array returns its INDICES — so the record-only test read
* `['0','1','2']` for an AST node and was right only by accident. An empty array
* is "no filter" for the same reason an empty object is.
*
* Narrows to the `$filter` slot's own type rather than a restatement of it
* (#3909), so the caller assigns with no cast and this predicate cannot drift
* from the declaration it is guarding.
*/
function hasFilter(filter: unknown): boolean {
function hasFilter(filter: unknown): filter is NonNullable<QueryParams['$filter']> {
if (filter === null || filter === undefined) return false;
if (Array.isArray(filter)) return filter.length > 0;
if (typeof filter !== 'object') return false;
Expand DownExpand Up@@ -172,12 +176,14 @@ export function useRecordQuery(options: UseRecordQueryOptions): UseRecordQueryRe
if (searchTerm && searchTerm.trim()) params.$search = searchTerm.trim();
if (searchFields && searchFields.length > 0) params.$searchFields = searchFields;
if (sortArg) params.$orderby = { [sortArg.field]: sortArg.direction };
// `QueryParams.$filter` is declared `Record< string, any >`, which the
// AST-array form does not describe — the cast is at this ONE assignment
// rather than widening a shared type that several other producers
// (plugin-list's `buildEffectiveFilter`, plugin-view's ObjectView)
// already feed arrays through.
if (hasFilter(filter)) params.$filter = filter as Record<string, any>;
// No cast: `QueryParams.$filter` now declares both shapes it accepts
// (#3909), so the AST-array form the picker's merge yields is describable
// here. The local cast this replaces was deliberate debt — taken at this
// ONE assignment rather than widening the shared type that several other
// producers (plugin-list's `buildEffectiveFilter`, plugin-view's
// ObjectView) already feed arrays through. The shared type is honest now,
// so the debt is paid rather than moved.
if (hasFilter(filter)) params.$filter = filter;
if (expand && expand.length > 0) params.$expand = expand;

const result = await dataSource.find(objectName, params);
Expand Down
131 changes: 131 additions & 0 deletions packages/types/src/__tests__/query-params-filter-union.test.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
/**
* ObjectUI
* Copyright (c) 2024-present ObjectStack Inc.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

/**
* objectui#3909 — `QueryParams.$filter` declares BOTH shapes the data sources
* accept, and binds the array half to `@objectstack/spec`'s `FilterArray`
* rather than restating it.
*
* ## Why this file exists at all
*
* The defect it pins was invisible to every runtime suite, and had to be: the
* declaration was `Record< string, any >`, which **structurally accepts arrays**
* (they satisfy its string index). So the two producers that have fed ObjectQL
* AST arrays through this slot all along — `plugin-list`'s
* `buildEffectiveFilter` (grid and export) and `plugin-view`'s `ObjectView`
* (calendar / kanban / gallery / timeline) — type-checked, ran, and shipped
* correct results. Nothing was broken at runtime and nothing could go red.
*
* The cost was paid on the type face instead, in both directions:
*
* 1. The type **blocked nothing while describing one legal shape as if it were
* the only one**. objectui#3831 is what that buys: a `Record< string, any >`
* slot accepted a rule array, an object spread flattened it to
* `{"0": {...}}`, types stayed green, and the query filtered on a column
* literally named `0`.
* 2. Someone writing a new consumer reads the type and its `@example`,
* concludes only the record form is legal, and adds a tolerant conversion
* for the array path — the "widen the consumer to tolerate the producer"
* shape AGENTS.md #0.1 forbids.
*
* Both failure modes are compile-time by nature, so the pins are too. Reverting
* `$filter` to `Record< string, any >` leaves every runtime suite green and
* turns THIS FILE red under `tsc -p tsconfig.test.json` (the `type-check`
* script) — the drift's own signature, reproduced deliberately.
*
* ## What is NOT pinned here
*
* That the union is the *authoritative* accepted set. It is not: the authority
* is `translateFilterToAST` (`@object-ui/data-objectstack`), which enumerates
* five input shapes. A second list here would be a third place to drift from —
* which is exactly how two operator vocabularies came apart in #3948. The
* binding below is to the spec's `FilterArray`, so the array half cannot fork
* locally.
*/

import { describe, it, expect } from 'vitest';
import type { FilterArray } from '@objectstack/spec/data';
import type { QueryParams } from '../data';

type Assert< T extends true > = T;
/** True when `V` is accepted by the `$filter` slot. */
type AcceptsFilter< V > = V extends QueryParams['$filter'] ? true : false;
/** Exact type identity — NOT mutual assignability. See the note below. */
type Equal< A, B > =
(< T >() => T extends A ? 1 : 2) extends (< T >() => T extends B ? 1 : 2) ? true : false;

describe('QueryParams.$filter — declares the union it actually accepts (#3909)', () => {
it('accepts the MongoDB-style field-keyed record', () => {
type _Record = Assert< AcceptsFilter< { age: { $gt: number } } > >;
const params: QueryParams = { $filter: { age: { $gt: 18 }, status: 'active' } };
expect(params.$filter).toEqual({ age: { $gt: 18 }, status: 'active' });
});

it('accepts a bare AST comparison tuple with no cast', () => {
// The shape `buildEffectiveFilter` returns for a single condition. Before
// #3909 this compiled only because arrays satisfy `Record`'s string index —
// accepted by accident rather than by declaration.
const params: QueryParams = { $filter: ['status', '=', 'active'] };
expect(params.$filter).toEqual(['status', '=', 'active']);
});

it('accepts a logical AST group with no cast', () => {
// What `mergeFilterNodes` returns once more than one source is active.
const params: QueryParams = {
$filter: ['and', ['age', '>=', 18], ['status', '=', 'active']],
};
expect(Array.isArray(params.$filter)).toBe(true);
});

it('accepts the legacy bare list, combined with implicit AND', () => {
const params: QueryParams = {
$filter: [['stage', '=', 'won'], ['amount', '>', 1000]],
};
expect(Array.isArray(params.$filter)).toBe(true);
});

it('binds the array half to the spec, by IDENTITY not assignability', () => {
// ## Why this pin is an identity check, and why nothing weaker works
//
// Every assignment-shaped pin in this file is, on its own, VACUOUS as a
// regression guard — measured, not assumed. Reverting the declaration to
// `Record< string, any >` and re-running `type-check` leaves it GREEN
// (exit 0), because assignability cannot separate the two: arrays satisfy
// `Record< string, any >`'s string index, so `FilterArray extends
// QueryParams['$filter']` holds under BOTH declarations, and the old
// declaration is itself assignable to the new union. A guard that passes
// equally before and after the fix is a phantom check — it reads like
// enforcement and enforces nothing.
//
// Identity is the property that actually differs. This assertion goes red
// on a revert to the bare record, AND on the subtler regression: someone
// re-declaring a local `FilterNode` fork instead of binding the spec's
// type. That fork would satisfy every assignment above while being free to
// drift from the vocabulary the servers parse — the exact failure two
// hand-written operator lists had in #3948.
type _Bound = Assert<
Equal< NonNullable< QueryParams['$filter'] >, Record< string, any > | FilterArray >
>;
const fromSpec: FilterArray = ['status', '=', 'active'];
const params: QueryParams = { $filter: fromSpec };
expect(params.$filter).toBe(fromSpec);
});

it('still refuses a value that is neither shape', () => {
// The union documents; it must not have become `any` on the way. Note the
// slot sits on an interface that also carries `[key: string]: any` — these
// pins prove the declared property still wins over that index signature,
// which is the whole reason the declaration is worth anything.
// @ts-expect-error a number is not a filter
const bad: QueryParams = { $filter: 42 };
// @ts-expect-error a string is not a filter
const alsoBad: QueryParams = { $filter: 'status eq active' };
expect(bad.$filter).toBe(42);
expect(alsoBad.$filter).toBe('status eq active');
});
});
34 changes: 32 additions & 2 deletions packages/types/src/data.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -31,6 +31,7 @@ import type {
CreateExportJobInput as SpecCreateExportJobInput,
CreateExportJobResult,
} from '@objectstack/spec/contracts';
import type { FilterArray } from '@objectstack/spec/data';
import type { ValidationError } from '@objectstack/spec/kernel';

export type { ExportJobStatus, ImportJobStatus, ImportWriteMode, ValidationError };
Expand All@@ -47,10 +48,39 @@ export interface QueryParams {
$select?: string[];

/**
* Filter expression
* Filter expression, in either of the two forms the data sources accept:
* the MongoDB-style field-keyed record, or a `FilterArray` — the spec-owned
* ObjectQL AST sugar (`@objectstack/spec/data`).
*
* Both forms are normal here, and the array form is not an edge case: the
* repo's own canonical sink `mergeFilterNodes` / `toFilterNode`
* (`@object-ui/core`'s `filter-converter.ts`) returns AST nodes, and its two
* standing producers — `plugin-list`'s `buildEffectiveFilter` (grid and
* export) and `plugin-view`'s `ObjectView` (calendar / kanban / gallery /
* timeline) — have fed arrays through this slot all along.
*
* ⛔ Do not add a "tolerant conversion" in a consumer to cope with the array
* path. The array IS legal input; a consumer that needs one shape lowers
* through the shared sink rather than widening itself to tolerate the
* producer.
*
* The authoritative acceptable set is the one `translateFilterToAST`
* (`@object-ui/data-objectstack`'s `index.ts`) enumerates — five input
* shapes, of which the array forms below are three. Read it there rather than
* trusting a second list here; a partial restatement is exactly how two
* operator vocabularies drifted apart before.
*
* Note the declaration does not *narrow* anything: `Record<string, any>`
* already structurally accepts arrays (they satisfy its string index), so the
* union documents the shapes that were always accepted rather than admitting
* new ones. It is the description that was wrong, not the runtime.
*
* @example { age: { $gt: 18 }, status: 'active' }
* @example ['status', '=', 'active']
* @example ['and', ['age', '>=', 18], ['status', '=', 'active']]
* @example [['stage', '=', 'won'], ['amount', '>', 1000]]
*/
$filter?: Record<string, any>;
$filter?: Record<string, any> | FilterArray;

/**
* Sort order
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Highlight search terms from Google/DuckDuckGo/Bing referrer (function() { var ref = document.referrer; var terms = []; if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) { var url = new URL(ref); var q = url.searchParams.get('q') || url.searchParams.get('p'); if (q) { terms = q.split(/\s+/).filter(function(t) { return t.length > 2; }); } } if (terms.length === 0) return; var style = document.createElement('style'); style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }'; document.head.appendChild(style); function highlight(node) { if (node.nodeType === 3) { // text node var text = node.textContent; var found = false; terms.forEach(function(term) { var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\]\\]/g, '\\') + ')', 'gi'); if (regex.test(text)) { found = true; var frag = document.createDocumentFragment(); var parts = text.split(regex); parts.forEach(function(part, i) { if (i % 2 === 0) { frag.appendChild(document.createTextNode(part)); } else { var span = document.createElement('span'); span.className = 'userscript-highlight'; span.textContent = part; frag.appendChild(span); } }); node.parentNode.replaceChild(frag, node); } }); } else if (node.nodeType === 1 && node.childNodes) { // element var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT']; if (!skipTags.includes(node.tagName)) { Array.from(node.childNodes).forEach(highlight); } } } highlight(document.body); // Re-highlight on dynamic content var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1 || node.nodeType === 3) highlight(node); }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' fix(types): declare QueryParams.$filter as the union it already accepts by yinlianghui · Pull Request #5999 · objectstack-ai/objectui · GitHub
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
38 changes: 38 additions & 0 deletions .changeset/3909-query-params-filter-union.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
---
'@object-ui/types': patch
'@object-ui/fields': patch
---

`QueryParams.$filter` now declares both shapes the data sources actually accept — the
MongoDB-style field-keyed record, or a `FilterArray`, the ObjectQL AST sugar bound from
`@objectstack/spec/data` (objectui#3909).

**Nothing is narrowed and no accepted value changes.** `Record<string, any>` already
accepted arrays structurally — they satisfy its string index — so the union documents
shapes that were always legal rather than admitting new ones. Measured both ways under
`tsc --strict`: all five inputs `translateFilterToAST` enumerates assign to the old and
new declarations alike, and both reject a bare number and a bare string identically. A
downstream `turbo run build` over all 43 dependent packages is green, which is the
evidence a published type change breaks no consumer.

The harm was entirely on the type face, and it was two-sided. The declaration blocked
nothing while describing one legal shape as though it were the only one — objectui#3831
is what that cost, a rule array accepted by a `Record<string, any>` slot, object-spread
flattened to `{"0": {...}}`, types green, and the query filtering on a column literally
named `0`. And someone writing a new consumer would read the type and its record-only
`@example`, conclude the array path was illegal, and add a tolerant conversion for it —
the "widen the consumer to tolerate the producer" shape AGENTS.md #0.1 forbids. Two
producers have fed arrays through this slot all along: `plugin-list`'s
`buildEffectiveFilter` (grid and export) and `plugin-view`'s `ObjectView` (calendar /
kanban / gallery / timeline). The runtime was right; the declaration was narrow.

The array half is **bound** to the spec's `FilterArray` rather than restated locally, so
it cannot fork from the vocabulary the servers parse — the same failure two hand-written
operator lists had in objectui#3948. The doc comment names `translateFilterToAST` as the
authoritative accepted set instead of carrying a second list to drift from.

`@object-ui/fields` drops the local cast this defect forced. PR objectui#3908 wrote
`filter as Record<string, any>` at one assignment in `useRecordQuery`, deliberately, as
debt rather than widening the shared type. `hasFilter` is now a type predicate narrowing
to the `$filter` slot's own type, so the assignment needs no cast and the guard cannot
drift from the declaration it guards. Type-only throughout; no runtime behaviour changes.
20 changes: 13 additions & 7 deletions packages/fields/src/widgets/useRecordQuery.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -111,8 +111,12 @@ export interface UseRecordQueryResult {
* `Object.keys` on an array returns its INDICES — so the record-only test read
* `['0','1','2']` for an AST node and was right only by accident. An empty array
* is "no filter" for the same reason an empty object is.
*
* Narrows to the `$filter` slot's own type rather than a restatement of it
* (#3909), so the caller assigns with no cast and this predicate cannot drift
* from the declaration it is guarding.
*/
function hasFilter(filter: unknown): boolean {
function hasFilter(filter: unknown): filter is NonNullable<QueryParams['$filter']> {
if (filter === null || filter === undefined) return false;
if (Array.isArray(filter)) return filter.length > 0;
if (typeof filter !== 'object') return false;
Expand DownExpand Up@@ -172,12 +176,14 @@ export function useRecordQuery(options: UseRecordQueryOptions): UseRecordQueryRe
if (searchTerm && searchTerm.trim()) params.$search = searchTerm.trim();
if (searchFields && searchFields.length > 0) params.$searchFields = searchFields;
if (sortArg) params.$orderby = { [sortArg.field]: sortArg.direction };
// `QueryParams.$filter` is declared `Record< string, any >`, which the
// AST-array form does not describe — the cast is at this ONE assignment
// rather than widening a shared type that several other producers
// (plugin-list's `buildEffectiveFilter`, plugin-view's ObjectView)
// already feed arrays through.
if (hasFilter(filter)) params.$filter = filter as Record<string, any>;
// No cast: `QueryParams.$filter` now declares both shapes it accepts
// (#3909), so the AST-array form the picker's merge yields is describable
// here. The local cast this replaces was deliberate debt — taken at this
// ONE assignment rather than widening the shared type that several other
// producers (plugin-list's `buildEffectiveFilter`, plugin-view's
// ObjectView) already feed arrays through. The shared type is honest now,
// so the debt is paid rather than moved.
if (hasFilter(filter)) params.$filter = filter;
if (expand && expand.length > 0) params.$expand = expand;

const result = await dataSource.find(objectName, params);
Expand Down
131 changes: 131 additions & 0 deletions packages/types/src/__tests__/query-params-filter-union.test.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
/**
* ObjectUI
* Copyright (c) 2024-present ObjectStack Inc.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

/**
* objectui#3909 — `QueryParams.$filter` declares BOTH shapes the data sources
* accept, and binds the array half to `@objectstack/spec`'s `FilterArray`
* rather than restating it.
*
* ## Why this file exists at all
*
* The defect it pins was invisible to every runtime suite, and had to be: the
* declaration was `Record< string, any >`, which **structurally accepts arrays**
* (they satisfy its string index). So the two producers that have fed ObjectQL
* AST arrays through this slot all along — `plugin-list`'s
* `buildEffectiveFilter` (grid and export) and `plugin-view`'s `ObjectView`
* (calendar / kanban / gallery / timeline) — type-checked, ran, and shipped
* correct results. Nothing was broken at runtime and nothing could go red.
*
* The cost was paid on the type face instead, in both directions:
*
* 1. The type **blocked nothing while describing one legal shape as if it were
* the only one**. objectui#3831 is what that buys: a `Record< string, any >`
* slot accepted a rule array, an object spread flattened it to
* `{"0": {...}}`, types stayed green, and the query filtered on a column
* literally named `0`.
* 2. Someone writing a new consumer reads the type and its `@example`,
* concludes only the record form is legal, and adds a tolerant conversion
* for the array path — the "widen the consumer to tolerate the producer"
* shape AGENTS.md #0.1 forbids.
*
* Both failure modes are compile-time by nature, so the pins are too. Reverting
* `$filter` to `Record< string, any >` leaves every runtime suite green and
* turns THIS FILE red under `tsc -p tsconfig.test.json` (the `type-check`
* script) — the drift's own signature, reproduced deliberately.
*
* ## What is NOT pinned here
*
* That the union is the *authoritative* accepted set. It is not: the authority
* is `translateFilterToAST` (`@object-ui/data-objectstack`), which enumerates
* five input shapes. A second list here would be a third place to drift from —
* which is exactly how two operator vocabularies came apart in #3948. The
* binding below is to the spec's `FilterArray`, so the array half cannot fork
* locally.
*/

import { describe, it, expect } from 'vitest';
import type { FilterArray } from '@objectstack/spec/data';
import type { QueryParams } from '../data';

type Assert< T extends true > = T;
/** True when `V` is accepted by the `$filter` slot. */
type AcceptsFilter< V > = V extends QueryParams['$filter'] ? true : false;
/** Exact type identity — NOT mutual assignability. See the note below. */
type Equal< A, B > =
(< T >() => T extends A ? 1 : 2) extends (< T >() => T extends B ? 1 : 2) ? true : false;

describe('QueryParams.$filter — declares the union it actually accepts (#3909)', () => {
it('accepts the MongoDB-style field-keyed record', () => {
type _Record = Assert< AcceptsFilter< { age: { $gt: number } } > >;
const params: QueryParams = { $filter: { age: { $gt: 18 }, status: 'active' } };
expect(params.$filter).toEqual({ age: { $gt: 18 }, status: 'active' });
});

it('accepts a bare AST comparison tuple with no cast', () => {
// The shape `buildEffectiveFilter` returns for a single condition. Before
// #3909 this compiled only because arrays satisfy `Record`'s string index —
// accepted by accident rather than by declaration.
const params: QueryParams = { $filter: ['status', '=', 'active'] };
expect(params.$filter).toEqual(['status', '=', 'active']);
});

it('accepts a logical AST group with no cast', () => {
// What `mergeFilterNodes` returns once more than one source is active.
const params: QueryParams = {
$filter: ['and', ['age', '>=', 18], ['status', '=', 'active']],
};
expect(Array.isArray(params.$filter)).toBe(true);
});

it('accepts the legacy bare list, combined with implicit AND', () => {
const params: QueryParams = {
$filter: [['stage', '=', 'won'], ['amount', '>', 1000]],
};
expect(Array.isArray(params.$filter)).toBe(true);
});

it('binds the array half to the spec, by IDENTITY not assignability', () => {
// ## Why this pin is an identity check, and why nothing weaker works
//
// Every assignment-shaped pin in this file is, on its own, VACUOUS as a
// regression guard — measured, not assumed. Reverting the declaration to
// `Record< string, any >` and re-running `type-check` leaves it GREEN
// (exit 0), because assignability cannot separate the two: arrays satisfy
// `Record< string, any >`'s string index, so `FilterArray extends
// QueryParams['$filter']` holds under BOTH declarations, and the old
// declaration is itself assignable to the new union. A guard that passes
// equally before and after the fix is a phantom check — it reads like
// enforcement and enforces nothing.
//
// Identity is the property that actually differs. This assertion goes red
// on a revert to the bare record, AND on the subtler regression: someone
// re-declaring a local `FilterNode` fork instead of binding the spec's
// type. That fork would satisfy every assignment above while being free to
// drift from the vocabulary the servers parse — the exact failure two
// hand-written operator lists had in #3948.
type _Bound = Assert<
Equal< NonNullable< QueryParams['$filter'] >, Record< string, any > | FilterArray >
>;
const fromSpec: FilterArray = ['status', '=', 'active'];
const params: QueryParams = { $filter: fromSpec };
expect(params.$filter).toBe(fromSpec);
});

it('still refuses a value that is neither shape', () => {
// The union documents; it must not have become `any` on the way. Note the
// slot sits on an interface that also carries `[key: string]: any` — these
// pins prove the declared property still wins over that index signature,
// which is the whole reason the declaration is worth anything.
// @ts-expect-error a number is not a filter
const bad: QueryParams = { $filter: 42 };
// @ts-expect-error a string is not a filter
const alsoBad: QueryParams = { $filter: 'status eq active' };
expect(bad.$filter).toBe(42);
expect(alsoBad.$filter).toBe('status eq active');
});
});
34 changes: 32 additions & 2 deletions packages/types/src/data.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -31,6 +31,7 @@ import type {
CreateExportJobInput as SpecCreateExportJobInput,
CreateExportJobResult,
} from '@objectstack/spec/contracts';
import type { FilterArray } from '@objectstack/spec/data';
import type { ValidationError } from '@objectstack/spec/kernel';

export type { ExportJobStatus, ImportJobStatus, ImportWriteMode, ValidationError };
Expand All@@ -47,10 +48,39 @@ export interface QueryParams {
$select?: string[];

/**
* Filter expression
* Filter expression, in either of the two forms the data sources accept:
* the MongoDB-style field-keyed record, or a `FilterArray` — the spec-owned
* ObjectQL AST sugar (`@objectstack/spec/data`).
*
* Both forms are normal here, and the array form is not an edge case: the
* repo's own canonical sink `mergeFilterNodes` / `toFilterNode`
* (`@object-ui/core`'s `filter-converter.ts`) returns AST nodes, and its two
* standing producers — `plugin-list`'s `buildEffectiveFilter` (grid and
* export) and `plugin-view`'s `ObjectView` (calendar / kanban / gallery /
* timeline) — have fed arrays through this slot all along.
*
* ⛔ Do not add a "tolerant conversion" in a consumer to cope with the array
* path. The array IS legal input; a consumer that needs one shape lowers
* through the shared sink rather than widening itself to tolerate the
* producer.
*
* The authoritative acceptable set is the one `translateFilterToAST`
* (`@object-ui/data-objectstack`'s `index.ts`) enumerates — five input
* shapes, of which the array forms below are three. Read it there rather than
* trusting a second list here; a partial restatement is exactly how two
* operator vocabularies drifted apart before.
*
* Note the declaration does not *narrow* anything: `Record<string, any>`
* already structurally accepts arrays (they satisfy its string index), so the
* union documents the shapes that were always accepted rather than admitting
* new ones. It is the description that was wrong, not the runtime.
*
* @example { age: { $gt: 18 }, status: 'active' }
* @example ['status', '=', 'active']
* @example ['and', ['age', '>=', 18], ['status', '=', 'active']]
* @example [['stage', '=', 'won'], ['amount', '>', 1000]]
*/
$filter?: Record<string, any>;
$filter?: Record<string, any> | FilterArray;

/**
* Sort order
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + ' fix(types): declare QueryParams.$filter as the union it already accepts by yinlianghui · Pull Request #5999 · objectstack-ai/objectui · GitHub
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
38 changes: 38 additions & 0 deletions .changeset/3909-query-params-filter-union.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
---
'@object-ui/types': patch
'@object-ui/fields': patch
---

`QueryParams.$filter` now declares both shapes the data sources actually accept — the
MongoDB-style field-keyed record, or a `FilterArray`, the ObjectQL AST sugar bound from
`@objectstack/spec/data` (objectui#3909).

**Nothing is narrowed and no accepted value changes.** `Record<string, any>` already
accepted arrays structurally — they satisfy its string index — so the union documents
shapes that were always legal rather than admitting new ones. Measured both ways under
`tsc --strict`: all five inputs `translateFilterToAST` enumerates assign to the old and
new declarations alike, and both reject a bare number and a bare string identically. A
downstream `turbo run build` over all 43 dependent packages is green, which is the
evidence a published type change breaks no consumer.

The harm was entirely on the type face, and it was two-sided. The declaration blocked
nothing while describing one legal shape as though it were the only one — objectui#3831
is what that cost, a rule array accepted by a `Record<string, any>` slot, object-spread
flattened to `{"0": {...}}`, types green, and the query filtering on a column literally
named `0`. And someone writing a new consumer would read the type and its record-only
`@example`, conclude the array path was illegal, and add a tolerant conversion for it —
the "widen the consumer to tolerate the producer" shape AGENTS.md #0.1 forbids. Two
producers have fed arrays through this slot all along: `plugin-list`'s
`buildEffectiveFilter` (grid and export) and `plugin-view`'s `ObjectView` (calendar /
kanban / gallery / timeline). The runtime was right; the declaration was narrow.

The array half is **bound** to the spec's `FilterArray` rather than restated locally, so
it cannot fork from the vocabulary the servers parse — the same failure two hand-written
operator lists had in objectui#3948. The doc comment names `translateFilterToAST` as the
authoritative accepted set instead of carrying a second list to drift from.

`@object-ui/fields` drops the local cast this defect forced. PR objectui#3908 wrote
`filter as Record<string, any>` at one assignment in `useRecordQuery`, deliberately, as
debt rather than widening the shared type. `hasFilter` is now a type predicate narrowing
to the `$filter` slot's own type, so the assignment needs no cast and the guard cannot
drift from the declaration it guards. Type-only throughout; no runtime behaviour changes.
20 changes: 13 additions & 7 deletions packages/fields/src/widgets/useRecordQuery.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -111,8 +111,12 @@ export interface UseRecordQueryResult {
* `Object.keys` on an array returns its INDICES — so the record-only test read
* `['0','1','2']` for an AST node and was right only by accident. An empty array
* is "no filter" for the same reason an empty object is.
*
* Narrows to the `$filter` slot's own type rather than a restatement of it
* (#3909), so the caller assigns with no cast and this predicate cannot drift
* from the declaration it is guarding.
*/
function hasFilter(filter: unknown): boolean {
function hasFilter(filter: unknown): filter is NonNullable<QueryParams['$filter']> {
if (filter === null || filter === undefined) return false;
if (Array.isArray(filter)) return filter.length > 0;
if (typeof filter !== 'object') return false;
Expand DownExpand Up@@ -172,12 +176,14 @@ export function useRecordQuery(options: UseRecordQueryOptions): UseRecordQueryRe
if (searchTerm && searchTerm.trim()) params.$search = searchTerm.trim();
if (searchFields && searchFields.length > 0) params.$searchFields = searchFields;
if (sortArg) params.$orderby = { [sortArg.field]: sortArg.direction };
// `QueryParams.$filter` is declared `Record< string, any >`, which the
// AST-array form does not describe — the cast is at this ONE assignment
// rather than widening a shared type that several other producers
// (plugin-list's `buildEffectiveFilter`, plugin-view's ObjectView)
// already feed arrays through.
if (hasFilter(filter)) params.$filter = filter as Record<string, any>;
// No cast: `QueryParams.$filter` now declares both shapes it accepts
// (#3909), so the AST-array form the picker's merge yields is describable
// here. The local cast this replaces was deliberate debt — taken at this
// ONE assignment rather than widening the shared type that several other
// producers (plugin-list's `buildEffectiveFilter`, plugin-view's
// ObjectView) already feed arrays through. The shared type is honest now,
// so the debt is paid rather than moved.
if (hasFilter(filter)) params.$filter = filter;
if (expand && expand.length > 0) params.$expand = expand;

const result = await dataSource.find(objectName, params);
Expand Down
131 changes: 131 additions & 0 deletions packages/types/src/__tests__/query-params-filter-union.test.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
/**
* ObjectUI
* Copyright (c) 2024-present ObjectStack Inc.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

/**
* objectui#3909 — `QueryParams.$filter` declares BOTH shapes the data sources
* accept, and binds the array half to `@objectstack/spec`'s `FilterArray`
* rather than restating it.
*
* ## Why this file exists at all
*
* The defect it pins was invisible to every runtime suite, and had to be: the
* declaration was `Record< string, any >`, which **structurally accepts arrays**
* (they satisfy its string index). So the two producers that have fed ObjectQL
* AST arrays through this slot all along — `plugin-list`'s
* `buildEffectiveFilter` (grid and export) and `plugin-view`'s `ObjectView`
* (calendar / kanban / gallery / timeline) — type-checked, ran, and shipped
* correct results. Nothing was broken at runtime and nothing could go red.
*
* The cost was paid on the type face instead, in both directions:
*
* 1. The type **blocked nothing while describing one legal shape as if it were
* the only one**. objectui#3831 is what that buys: a `Record< string, any >`
* slot accepted a rule array, an object spread flattened it to
* `{"0": {...}}`, types stayed green, and the query filtered on a column
* literally named `0`.
* 2. Someone writing a new consumer reads the type and its `@example`,
* concludes only the record form is legal, and adds a tolerant conversion
* for the array path — the "widen the consumer to tolerate the producer"
* shape AGENTS.md #0.1 forbids.
*
* Both failure modes are compile-time by nature, so the pins are too. Reverting
* `$filter` to `Record< string, any >` leaves every runtime suite green and
* turns THIS FILE red under `tsc -p tsconfig.test.json` (the `type-check`
* script) — the drift's own signature, reproduced deliberately.
*
* ## What is NOT pinned here
*
* That the union is the *authoritative* accepted set. It is not: the authority
* is `translateFilterToAST` (`@object-ui/data-objectstack`), which enumerates
* five input shapes. A second list here would be a third place to drift from —
* which is exactly how two operator vocabularies came apart in #3948. The
* binding below is to the spec's `FilterArray`, so the array half cannot fork
* locally.
*/

import { describe, it, expect } from 'vitest';
import type { FilterArray } from '@objectstack/spec/data';
import type { QueryParams } from '../data';

type Assert< T extends true > = T;
/** True when `V` is accepted by the `$filter` slot. */
type AcceptsFilter< V > = V extends QueryParams['$filter'] ? true : false;
/** Exact type identity — NOT mutual assignability. See the note below. */
type Equal< A, B > =
(< T >() => T extends A ? 1 : 2) extends (< T >() => T extends B ? 1 : 2) ? true : false;

describe('QueryParams.$filter — declares the union it actually accepts (#3909)', () => {
it('accepts the MongoDB-style field-keyed record', () => {
type _Record = Assert< AcceptsFilter< { age: { $gt: number } } > >;
const params: QueryParams = { $filter: { age: { $gt: 18 }, status: 'active' } };
expect(params.$filter).toEqual({ age: { $gt: 18 }, status: 'active' });
});

it('accepts a bare AST comparison tuple with no cast', () => {
// The shape `buildEffectiveFilter` returns for a single condition. Before
// #3909 this compiled only because arrays satisfy `Record`'s string index —
// accepted by accident rather than by declaration.
const params: QueryParams = { $filter: ['status', '=', 'active'] };
expect(params.$filter).toEqual(['status', '=', 'active']);
});

it('accepts a logical AST group with no cast', () => {
// What `mergeFilterNodes` returns once more than one source is active.
const params: QueryParams = {
$filter: ['and', ['age', '>=', 18], ['status', '=', 'active']],
};
expect(Array.isArray(params.$filter)).toBe(true);
});

it('accepts the legacy bare list, combined with implicit AND', () => {
const params: QueryParams = {
$filter: [['stage', '=', 'won'], ['amount', '>', 1000]],
};
expect(Array.isArray(params.$filter)).toBe(true);
});

it('binds the array half to the spec, by IDENTITY not assignability', () => {
// ## Why this pin is an identity check, and why nothing weaker works
//
// Every assignment-shaped pin in this file is, on its own, VACUOUS as a
// regression guard — measured, not assumed. Reverting the declaration to
// `Record< string, any >` and re-running `type-check` leaves it GREEN
// (exit 0), because assignability cannot separate the two: arrays satisfy
// `Record< string, any >`'s string index, so `FilterArray extends
// QueryParams['$filter']` holds under BOTH declarations, and the old
// declaration is itself assignable to the new union. A guard that passes
// equally before and after the fix is a phantom check — it reads like
// enforcement and enforces nothing.
//
// Identity is the property that actually differs. This assertion goes red
// on a revert to the bare record, AND on the subtler regression: someone
// re-declaring a local `FilterNode` fork instead of binding the spec's
// type. That fork would satisfy every assignment above while being free to
// drift from the vocabulary the servers parse — the exact failure two
// hand-written operator lists had in #3948.
type _Bound = Assert<
Equal< NonNullable< QueryParams['$filter'] >, Record< string, any > | FilterArray >
>;
const fromSpec: FilterArray = ['status', '=', 'active'];
const params: QueryParams = { $filter: fromSpec };
expect(params.$filter).toBe(fromSpec);
});

it('still refuses a value that is neither shape', () => {
// The union documents; it must not have become `any` on the way. Note the
// slot sits on an interface that also carries `[key: string]: any` — these
// pins prove the declared property still wins over that index signature,
// which is the whole reason the declaration is worth anything.
// @ts-expect-error a number is not a filter
const bad: QueryParams = { $filter: 42 };
// @ts-expect-error a string is not a filter
const alsoBad: QueryParams = { $filter: 'status eq active' };
expect(bad.$filter).toBe(42);
expect(alsoBad.$filter).toBe('status eq active');
});
});
34 changes: 32 additions & 2 deletions packages/types/src/data.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -31,6 +31,7 @@ import type {
CreateExportJobInput as SpecCreateExportJobInput,
CreateExportJobResult,
} from '@objectstack/spec/contracts';
import type { FilterArray } from '@objectstack/spec/data';
import type { ValidationError } from '@objectstack/spec/kernel';

export type { ExportJobStatus, ImportJobStatus, ImportWriteMode, ValidationError };
Expand All@@ -47,10 +48,39 @@ export interface QueryParams {
$select?: string[];

/**
* Filter expression
* Filter expression, in either of the two forms the data sources accept:
* the MongoDB-style field-keyed record, or a `FilterArray` — the spec-owned
* ObjectQL AST sugar (`@objectstack/spec/data`).
*
* Both forms are normal here, and the array form is not an edge case: the
* repo's own canonical sink `mergeFilterNodes` / `toFilterNode`
* (`@object-ui/core`'s `filter-converter.ts`) returns AST nodes, and its two
* standing producers — `plugin-list`'s `buildEffectiveFilter` (grid and
* export) and `plugin-view`'s `ObjectView` (calendar / kanban / gallery /
* timeline) — have fed arrays through this slot all along.
*
* ⛔ Do not add a "tolerant conversion" in a consumer to cope with the array
* path. The array IS legal input; a consumer that needs one shape lowers
* through the shared sink rather than widening itself to tolerate the
* producer.
*
* The authoritative acceptable set is the one `translateFilterToAST`
* (`@object-ui/data-objectstack`'s `index.ts`) enumerates — five input
* shapes, of which the array forms below are three. Read it there rather than
* trusting a second list here; a partial restatement is exactly how two
* operator vocabularies drifted apart before.
*
* Note the declaration does not *narrow* anything: `Record<string, any>`
* already structurally accepts arrays (they satisfy its string index), so the
* union documents the shapes that were always accepted rather than admitting
* new ones. It is the description that was wrong, not the runtime.
*
* @example { age: { $gt: 18 }, status: 'active' }
* @example ['status', '=', 'active']
* @example ['and', ['age', '>=', 18], ['status', '=', 'active']]
* @example [['stage', '=', 'won'], ['amount', '>', 1000]]
*/
$filter?: Record<string, any>;
$filter?: Record<string, any> | FilterArray;

/**
* Sort order
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' fix(types): declare QueryParams.$filter as the union it already accepts by yinlianghui · Pull Request #5999 · objectstack-ai/objectui · GitHub
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
38 changes: 38 additions & 0 deletions .changeset/3909-query-params-filter-union.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
---
'@object-ui/types': patch
'@object-ui/fields': patch
---

`QueryParams.$filter` now declares both shapes the data sources actually accept — the
MongoDB-style field-keyed record, or a `FilterArray`, the ObjectQL AST sugar bound from
`@objectstack/spec/data` (objectui#3909).

**Nothing is narrowed and no accepted value changes.** `Record<string, any>` already
accepted arrays structurally — they satisfy its string index — so the union documents
shapes that were always legal rather than admitting new ones. Measured both ways under
`tsc --strict`: all five inputs `translateFilterToAST` enumerates assign to the old and
new declarations alike, and both reject a bare number and a bare string identically. A
downstream `turbo run build` over all 43 dependent packages is green, which is the
evidence a published type change breaks no consumer.

The harm was entirely on the type face, and it was two-sided. The declaration blocked
nothing while describing one legal shape as though it were the only one — objectui#3831
is what that cost, a rule array accepted by a `Record<string, any>` slot, object-spread
flattened to `{"0": {...}}`, types green, and the query filtering on a column literally
named `0`. And someone writing a new consumer would read the type and its record-only
`@example`, conclude the array path was illegal, and add a tolerant conversion for it —
the "widen the consumer to tolerate the producer" shape AGENTS.md #0.1 forbids. Two
producers have fed arrays through this slot all along: `plugin-list`'s
`buildEffectiveFilter` (grid and export) and `plugin-view`'s `ObjectView` (calendar /
kanban / gallery / timeline). The runtime was right; the declaration was narrow.

The array half is **bound** to the spec's `FilterArray` rather than restated locally, so
it cannot fork from the vocabulary the servers parse — the same failure two hand-written
operator lists had in objectui#3948. The doc comment names `translateFilterToAST` as the
authoritative accepted set instead of carrying a second list to drift from.

`@object-ui/fields` drops the local cast this defect forced. PR objectui#3908 wrote
`filter as Record<string, any>` at one assignment in `useRecordQuery`, deliberately, as
debt rather than widening the shared type. `hasFilter` is now a type predicate narrowing
to the `$filter` slot's own type, so the assignment needs no cast and the guard cannot
drift from the declaration it guards. Type-only throughout; no runtime behaviour changes.
20 changes: 13 additions & 7 deletions packages/fields/src/widgets/useRecordQuery.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -111,8 +111,12 @@ export interface UseRecordQueryResult {
* `Object.keys` on an array returns its INDICES — so the record-only test read
* `['0','1','2']` for an AST node and was right only by accident. An empty array
* is "no filter" for the same reason an empty object is.
*
* Narrows to the `$filter` slot's own type rather than a restatement of it
* (#3909), so the caller assigns with no cast and this predicate cannot drift
* from the declaration it is guarding.
*/
function hasFilter(filter: unknown): boolean {
function hasFilter(filter: unknown): filter is NonNullable<QueryParams['$filter']> {
if (filter === null || filter === undefined) return false;
if (Array.isArray(filter)) return filter.length > 0;
if (typeof filter !== 'object') return false;
Expand DownExpand Up@@ -172,12 +176,14 @@ export function useRecordQuery(options: UseRecordQueryOptions): UseRecordQueryRe
if (searchTerm && searchTerm.trim()) params.$search = searchTerm.trim();
if (searchFields && searchFields.length > 0) params.$searchFields = searchFields;
if (sortArg) params.$orderby = { [sortArg.field]: sortArg.direction };
// `QueryParams.$filter` is declared `Record< string, any >`, which the
// AST-array form does not describe — the cast is at this ONE assignment
// rather than widening a shared type that several other producers
// (plugin-list's `buildEffectiveFilter`, plugin-view's ObjectView)
// already feed arrays through.
if (hasFilter(filter)) params.$filter = filter as Record<string, any>;
// No cast: `QueryParams.$filter` now declares both shapes it accepts
// (#3909), so the AST-array form the picker's merge yields is describable
// here. The local cast this replaces was deliberate debt — taken at this
// ONE assignment rather than widening the shared type that several other
// producers (plugin-list's `buildEffectiveFilter`, plugin-view's
// ObjectView) already feed arrays through. The shared type is honest now,
// so the debt is paid rather than moved.
if (hasFilter(filter)) params.$filter = filter;
if (expand && expand.length > 0) params.$expand = expand;

const result = await dataSource.find(objectName, params);
Expand Down
131 changes: 131 additions & 0 deletions packages/types/src/__tests__/query-params-filter-union.test.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
/**
* ObjectUI
* Copyright (c) 2024-present ObjectStack Inc.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

/**
* objectui#3909 — `QueryParams.$filter` declares BOTH shapes the data sources
* accept, and binds the array half to `@objectstack/spec`'s `FilterArray`
* rather than restating it.
*
* ## Why this file exists at all
*
* The defect it pins was invisible to every runtime suite, and had to be: the
* declaration was `Record< string, any >`, which **structurally accepts arrays**
* (they satisfy its string index). So the two producers that have fed ObjectQL
* AST arrays through this slot all along — `plugin-list`'s
* `buildEffectiveFilter` (grid and export) and `plugin-view`'s `ObjectView`
* (calendar / kanban / gallery / timeline) — type-checked, ran, and shipped
* correct results. Nothing was broken at runtime and nothing could go red.
*
* The cost was paid on the type face instead, in both directions:
*
* 1. The type **blocked nothing while describing one legal shape as if it were
* the only one**. objectui#3831 is what that buys: a `Record< string, any >`
* slot accepted a rule array, an object spread flattened it to
* `{"0": {...}}`, types stayed green, and the query filtered on a column
* literally named `0`.
* 2. Someone writing a new consumer reads the type and its `@example`,
* concludes only the record form is legal, and adds a tolerant conversion
* for the array path — the "widen the consumer to tolerate the producer"
* shape AGENTS.md #0.1 forbids.
*
* Both failure modes are compile-time by nature, so the pins are too. Reverting
* `$filter` to `Record< string, any >` leaves every runtime suite green and
* turns THIS FILE red under `tsc -p tsconfig.test.json` (the `type-check`
* script) — the drift's own signature, reproduced deliberately.
*
* ## What is NOT pinned here
*
* That the union is the *authoritative* accepted set. It is not: the authority
* is `translateFilterToAST` (`@object-ui/data-objectstack`), which enumerates
* five input shapes. A second list here would be a third place to drift from —
* which is exactly how two operator vocabularies came apart in #3948. The
* binding below is to the spec's `FilterArray`, so the array half cannot fork
* locally.
*/

import { describe, it, expect } from 'vitest';
import type { FilterArray } from '@objectstack/spec/data';
import type { QueryParams } from '../data';

type Assert< T extends true > = T;
/** True when `V` is accepted by the `$filter` slot. */
type AcceptsFilter< V > = V extends QueryParams['$filter'] ? true : false;
/** Exact type identity — NOT mutual assignability. See the note below. */
type Equal< A, B > =
(< T >() => T extends A ? 1 : 2) extends (< T >() => T extends B ? 1 : 2) ? true : false;

describe('QueryParams.$filter — declares the union it actually accepts (#3909)', () => {
it('accepts the MongoDB-style field-keyed record', () => {
type _Record = Assert< AcceptsFilter< { age: { $gt: number } } > >;
const params: QueryParams = { $filter: { age: { $gt: 18 }, status: 'active' } };
expect(params.$filter).toEqual({ age: { $gt: 18 }, status: 'active' });
});

it('accepts a bare AST comparison tuple with no cast', () => {
// The shape `buildEffectiveFilter` returns for a single condition. Before
// #3909 this compiled only because arrays satisfy `Record`'s string index —
// accepted by accident rather than by declaration.
const params: QueryParams = { $filter: ['status', '=', 'active'] };
expect(params.$filter).toEqual(['status', '=', 'active']);
});

it('accepts a logical AST group with no cast', () => {
// What `mergeFilterNodes` returns once more than one source is active.
const params: QueryParams = {
$filter: ['and', ['age', '>=', 18], ['status', '=', 'active']],
};
expect(Array.isArray(params.$filter)).toBe(true);
});

it('accepts the legacy bare list, combined with implicit AND', () => {
const params: QueryParams = {
$filter: [['stage', '=', 'won'], ['amount', '>', 1000]],
};
expect(Array.isArray(params.$filter)).toBe(true);
});

it('binds the array half to the spec, by IDENTITY not assignability', () => {
// ## Why this pin is an identity check, and why nothing weaker works
//
// Every assignment-shaped pin in this file is, on its own, VACUOUS as a
// regression guard — measured, not assumed. Reverting the declaration to
// `Record< string, any >` and re-running `type-check` leaves it GREEN
// (exit 0), because assignability cannot separate the two: arrays satisfy
// `Record< string, any >`'s string index, so `FilterArray extends
// QueryParams['$filter']` holds under BOTH declarations, and the old
// declaration is itself assignable to the new union. A guard that passes
// equally before and after the fix is a phantom check — it reads like
// enforcement and enforces nothing.
//
// Identity is the property that actually differs. This assertion goes red
// on a revert to the bare record, AND on the subtler regression: someone
// re-declaring a local `FilterNode` fork instead of binding the spec's
// type. That fork would satisfy every assignment above while being free to
// drift from the vocabulary the servers parse — the exact failure two
// hand-written operator lists had in #3948.
type _Bound = Assert<
Equal< NonNullable< QueryParams['$filter'] >, Record< string, any > | FilterArray >
>;
const fromSpec: FilterArray = ['status', '=', 'active'];
const params: QueryParams = { $filter: fromSpec };
expect(params.$filter).toBe(fromSpec);
});

it('still refuses a value that is neither shape', () => {
// The union documents; it must not have become `any` on the way. Note the
// slot sits on an interface that also carries `[key: string]: any` — these
// pins prove the declared property still wins over that index signature,
// which is the whole reason the declaration is worth anything.
// @ts-expect-error a number is not a filter
const bad: QueryParams = { $filter: 42 };
// @ts-expect-error a string is not a filter
const alsoBad: QueryParams = { $filter: 'status eq active' };
expect(bad.$filter).toBe(42);
expect(alsoBad.$filter).toBe('status eq active');
});
});
34 changes: 32 additions & 2 deletions packages/types/src/data.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -31,6 +31,7 @@ import type {
CreateExportJobInput as SpecCreateExportJobInput,
CreateExportJobResult,
} from '@objectstack/spec/contracts';
import type { FilterArray } from '@objectstack/spec/data';
import type { ValidationError } from '@objectstack/spec/kernel';

export type { ExportJobStatus, ImportJobStatus, ImportWriteMode, ValidationError };
Expand All@@ -47,10 +48,39 @@ export interface QueryParams {
$select?: string[];

/**
* Filter expression
* Filter expression, in either of the two forms the data sources accept:
* the MongoDB-style field-keyed record, or a `FilterArray` — the spec-owned
* ObjectQL AST sugar (`@objectstack/spec/data`).
*
* Both forms are normal here, and the array form is not an edge case: the
* repo's own canonical sink `mergeFilterNodes` / `toFilterNode`
* (`@object-ui/core`'s `filter-converter.ts`) returns AST nodes, and its two
* standing producers — `plugin-list`'s `buildEffectiveFilter` (grid and
* export) and `plugin-view`'s `ObjectView` (calendar / kanban / gallery /
* timeline) — have fed arrays through this slot all along.
*
* ⛔ Do not add a "tolerant conversion" in a consumer to cope with the array
* path. The array IS legal input; a consumer that needs one shape lowers
* through the shared sink rather than widening itself to tolerate the
* producer.
*
* The authoritative acceptable set is the one `translateFilterToAST`
* (`@object-ui/data-objectstack`'s `index.ts`) enumerates — five input
* shapes, of which the array forms below are three. Read it there rather than
* trusting a second list here; a partial restatement is exactly how two
* operator vocabularies drifted apart before.
*
* Note the declaration does not *narrow* anything: `Record<string, any>`
* already structurally accepts arrays (they satisfy its string index), so the
* union documents the shapes that were always accepted rather than admitting
* new ones. It is the description that was wrong, not the runtime.
*
* @example { age: { $gt: 18 }, status: 'active' }
* @example ['status', '=', 'active']
* @example ['and', ['age', '>=', 18], ['status', '=', 'active']]
* @example [['stage', '=', 'won'], ['amount', '>', 1000]]
*/
$filter?: Record<string, any>;
$filter?: Record<string, any> | FilterArray;

/**
* Sort order
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' fix(types): declare QueryParams.$filter as the union it already accepts by yinlianghui · Pull Request #5999 · objectstack-ai/objectui · GitHub
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
38 changes: 38 additions & 0 deletions .changeset/3909-query-params-filter-union.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
---
'@object-ui/types': patch
'@object-ui/fields': patch
---

`QueryParams.$filter` now declares both shapes the data sources actually accept — the
MongoDB-style field-keyed record, or a `FilterArray`, the ObjectQL AST sugar bound from
`@objectstack/spec/data` (objectui#3909).

**Nothing is narrowed and no accepted value changes.** `Record<string, any>` already
accepted arrays structurally — they satisfy its string index — so the union documents
shapes that were always legal rather than admitting new ones. Measured both ways under
`tsc --strict`: all five inputs `translateFilterToAST` enumerates assign to the old and
new declarations alike, and both reject a bare number and a bare string identically. A
downstream `turbo run build` over all 43 dependent packages is green, which is the
evidence a published type change breaks no consumer.

The harm was entirely on the type face, and it was two-sided. The declaration blocked
nothing while describing one legal shape as though it were the only one — objectui#3831
is what that cost, a rule array accepted by a `Record<string, any>` slot, object-spread
flattened to `{"0": {...}}`, types green, and the query filtering on a column literally
named `0`. And someone writing a new consumer would read the type and its record-only
`@example`, conclude the array path was illegal, and add a tolerant conversion for it —
the "widen the consumer to tolerate the producer" shape AGENTS.md #0.1 forbids. Two
producers have fed arrays through this slot all along: `plugin-list`'s
`buildEffectiveFilter` (grid and export) and `plugin-view`'s `ObjectView` (calendar /
kanban / gallery / timeline). The runtime was right; the declaration was narrow.

The array half is **bound** to the spec's `FilterArray` rather than restated locally, so
it cannot fork from the vocabulary the servers parse — the same failure two hand-written
operator lists had in objectui#3948. The doc comment names `translateFilterToAST` as the
authoritative accepted set instead of carrying a second list to drift from.

`@object-ui/fields` drops the local cast this defect forced. PR objectui#3908 wrote
`filter as Record<string, any>` at one assignment in `useRecordQuery`, deliberately, as
debt rather than widening the shared type. `hasFilter` is now a type predicate narrowing
to the `$filter` slot's own type, so the assignment needs no cast and the guard cannot
drift from the declaration it guards. Type-only throughout; no runtime behaviour changes.
20 changes: 13 additions & 7 deletions packages/fields/src/widgets/useRecordQuery.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -111,8 +111,12 @@ export interface UseRecordQueryResult {
* `Object.keys` on an array returns its INDICES — so the record-only test read
* `['0','1','2']` for an AST node and was right only by accident. An empty array
* is "no filter" for the same reason an empty object is.
*
* Narrows to the `$filter` slot's own type rather than a restatement of it
* (#3909), so the caller assigns with no cast and this predicate cannot drift
* from the declaration it is guarding.
*/
function hasFilter(filter: unknown): boolean {
function hasFilter(filter: unknown): filter is NonNullable<QueryParams['$filter']> {
if (filter === null || filter === undefined) return false;
if (Array.isArray(filter)) return filter.length > 0;
if (typeof filter !== 'object') return false;
Expand DownExpand Up@@ -172,12 +176,14 @@ export function useRecordQuery(options: UseRecordQueryOptions): UseRecordQueryRe
if (searchTerm && searchTerm.trim()) params.$search = searchTerm.trim();
if (searchFields && searchFields.length > 0) params.$searchFields = searchFields;
if (sortArg) params.$orderby = { [sortArg.field]: sortArg.direction };
// `QueryParams.$filter` is declared `Record< string, any >`, which the
// AST-array form does not describe — the cast is at this ONE assignment
// rather than widening a shared type that several other producers
// (plugin-list's `buildEffectiveFilter`, plugin-view's ObjectView)
// already feed arrays through.
if (hasFilter(filter)) params.$filter = filter as Record<string, any>;
// No cast: `QueryParams.$filter` now declares both shapes it accepts
// (#3909), so the AST-array form the picker's merge yields is describable
// here. The local cast this replaces was deliberate debt — taken at this
// ONE assignment rather than widening the shared type that several other
// producers (plugin-list's `buildEffectiveFilter`, plugin-view's
// ObjectView) already feed arrays through. The shared type is honest now,
// so the debt is paid rather than moved.
if (hasFilter(filter)) params.$filter = filter;
if (expand && expand.length > 0) params.$expand = expand;

const result = await dataSource.find(objectName, params);
Expand Down
131 changes: 131 additions & 0 deletions packages/types/src/__tests__/query-params-filter-union.test.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
/**
* ObjectUI
* Copyright (c) 2024-present ObjectStack Inc.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

/**
* objectui#3909 — `QueryParams.$filter` declares BOTH shapes the data sources
* accept, and binds the array half to `@objectstack/spec`'s `FilterArray`
* rather than restating it.
*
* ## Why this file exists at all
*
* The defect it pins was invisible to every runtime suite, and had to be: the
* declaration was `Record< string, any >`, which **structurally accepts arrays**
* (they satisfy its string index). So the two producers that have fed ObjectQL
* AST arrays through this slot all along — `plugin-list`'s
* `buildEffectiveFilter` (grid and export) and `plugin-view`'s `ObjectView`
* (calendar / kanban / gallery / timeline) — type-checked, ran, and shipped
* correct results. Nothing was broken at runtime and nothing could go red.
*
* The cost was paid on the type face instead, in both directions:
*
* 1. The type **blocked nothing while describing one legal shape as if it were
* the only one**. objectui#3831 is what that buys: a `Record< string, any >`
* slot accepted a rule array, an object spread flattened it to
* `{"0": {...}}`, types stayed green, and the query filtered on a column
* literally named `0`.
* 2. Someone writing a new consumer reads the type and its `@example`,
* concludes only the record form is legal, and adds a tolerant conversion
* for the array path — the "widen the consumer to tolerate the producer"
* shape AGENTS.md #0.1 forbids.
*
* Both failure modes are compile-time by nature, so the pins are too. Reverting
* `$filter` to `Record< string, any >` leaves every runtime suite green and
* turns THIS FILE red under `tsc -p tsconfig.test.json` (the `type-check`
* script) — the drift's own signature, reproduced deliberately.
*
* ## What is NOT pinned here
*
* That the union is the *authoritative* accepted set. It is not: the authority
* is `translateFilterToAST` (`@object-ui/data-objectstack`), which enumerates
* five input shapes. A second list here would be a third place to drift from —
* which is exactly how two operator vocabularies came apart in #3948. The
* binding below is to the spec's `FilterArray`, so the array half cannot fork
* locally.
*/

import { describe, it, expect } from 'vitest';
import type { FilterArray } from '@objectstack/spec/data';
import type { QueryParams } from '../data';

type Assert< T extends true > = T;
/** True when `V` is accepted by the `$filter` slot. */
type AcceptsFilter< V > = V extends QueryParams['$filter'] ? true : false;
/** Exact type identity — NOT mutual assignability. See the note below. */
type Equal< A, B > =
(< T >() => T extends A ? 1 : 2) extends (< T >() => T extends B ? 1 : 2) ? true : false;

describe('QueryParams.$filter — declares the union it actually accepts (#3909)', () => {
it('accepts the MongoDB-style field-keyed record', () => {
type _Record = Assert< AcceptsFilter< { age: { $gt: number } } > >;
const params: QueryParams = { $filter: { age: { $gt: 18 }, status: 'active' } };
expect(params.$filter).toEqual({ age: { $gt: 18 }, status: 'active' });
});

it('accepts a bare AST comparison tuple with no cast', () => {
// The shape `buildEffectiveFilter` returns for a single condition. Before
// #3909 this compiled only because arrays satisfy `Record`'s string index —
// accepted by accident rather than by declaration.
const params: QueryParams = { $filter: ['status', '=', 'active'] };
expect(params.$filter).toEqual(['status', '=', 'active']);
});

it('accepts a logical AST group with no cast', () => {
// What `mergeFilterNodes` returns once more than one source is active.
const params: QueryParams = {
$filter: ['and', ['age', '>=', 18], ['status', '=', 'active']],
};
expect(Array.isArray(params.$filter)).toBe(true);
});

it('accepts the legacy bare list, combined with implicit AND', () => {
const params: QueryParams = {
$filter: [['stage', '=', 'won'], ['amount', '>', 1000]],
};
expect(Array.isArray(params.$filter)).toBe(true);
});

it('binds the array half to the spec, by IDENTITY not assignability', () => {
// ## Why this pin is an identity check, and why nothing weaker works
//
// Every assignment-shaped pin in this file is, on its own, VACUOUS as a
// regression guard — measured, not assumed. Reverting the declaration to
// `Record< string, any >` and re-running `type-check` leaves it GREEN
// (exit 0), because assignability cannot separate the two: arrays satisfy
// `Record< string, any >`'s string index, so `FilterArray extends
// QueryParams['$filter']` holds under BOTH declarations, and the old
// declaration is itself assignable to the new union. A guard that passes
// equally before and after the fix is a phantom check — it reads like
// enforcement and enforces nothing.
//
// Identity is the property that actually differs. This assertion goes red
// on a revert to the bare record, AND on the subtler regression: someone
// re-declaring a local `FilterNode` fork instead of binding the spec's
// type. That fork would satisfy every assignment above while being free to
// drift from the vocabulary the servers parse — the exact failure two
// hand-written operator lists had in #3948.
type _Bound = Assert<
Equal< NonNullable< QueryParams['$filter'] >, Record< string, any > | FilterArray >
>;
const fromSpec: FilterArray = ['status', '=', 'active'];
const params: QueryParams = { $filter: fromSpec };
expect(params.$filter).toBe(fromSpec);
});

it('still refuses a value that is neither shape', () => {
// The union documents; it must not have become `any` on the way. Note the
// slot sits on an interface that also carries `[key: string]: any` — these
// pins prove the declared property still wins over that index signature,
// which is the whole reason the declaration is worth anything.
// @ts-expect-error a number is not a filter
const bad: QueryParams = { $filter: 42 };
// @ts-expect-error a string is not a filter
const alsoBad: QueryParams = { $filter: 'status eq active' };
expect(bad.$filter).toBe(42);
expect(alsoBad.$filter).toBe('status eq active');
});
});
34 changes: 32 additions & 2 deletions packages/types/src/data.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -31,6 +31,7 @@ import type {
CreateExportJobInput as SpecCreateExportJobInput,
CreateExportJobResult,
} from '@objectstack/spec/contracts';
import type { FilterArray } from '@objectstack/spec/data';
import type { ValidationError } from '@objectstack/spec/kernel';

export type { ExportJobStatus, ImportJobStatus, ImportWriteMode, ValidationError };
Expand All@@ -47,10 +48,39 @@ export interface QueryParams {
$select?: string[];

/**
* Filter expression
* Filter expression, in either of the two forms the data sources accept:
* the MongoDB-style field-keyed record, or a `FilterArray` — the spec-owned
* ObjectQL AST sugar (`@objectstack/spec/data`).
*
* Both forms are normal here, and the array form is not an edge case: the
* repo's own canonical sink `mergeFilterNodes` / `toFilterNode`
* (`@object-ui/core`'s `filter-converter.ts`) returns AST nodes, and its two
* standing producers — `plugin-list`'s `buildEffectiveFilter` (grid and
* export) and `plugin-view`'s `ObjectView` (calendar / kanban / gallery /
* timeline) — have fed arrays through this slot all along.
*
* ⛔ Do not add a "tolerant conversion" in a consumer to cope with the array
* path. The array IS legal input; a consumer that needs one shape lowers
* through the shared sink rather than widening itself to tolerate the
* producer.
*
* The authoritative acceptable set is the one `translateFilterToAST`
* (`@object-ui/data-objectstack`'s `index.ts`) enumerates — five input
* shapes, of which the array forms below are three. Read it there rather than
* trusting a second list here; a partial restatement is exactly how two
* operator vocabularies drifted apart before.
*
* Note the declaration does not *narrow* anything: `Record<string, any>`
* already structurally accepts arrays (they satisfy its string index), so the
* union documents the shapes that were always accepted rather than admitting
* new ones. It is the description that was wrong, not the runtime.
*
* @example { age: { $gt: 18 }, status: 'active' }
* @example ['status', '=', 'active']
* @example ['and', ['age', '>=', 18], ['status', '=', 'active']]
* @example [['stage', '=', 'won'], ['amount', '>', 1000]]
*/
$filter?: Record<string, any>;
$filter?: Record<string, any> | FilterArray;

/**
* Sort order
Expand Down
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Universal Dark Mode - works on any site (function() { var enabled = true; function applyDarkMode() { if (!enabled) return; // Create style element if it doesn't exist var style = document.getElementById('universal-dark-mode-style'); if (!style) { style = document.createElement('style'); style.id = 'universal-dark-mode-style'; document.head.appendChild(style); } // Dark mode CSS - inverts colors but preserves images/video style.textContent = ' /* Invert everything except media */ html { filter: invert(1) hue-rotate(180deg) !important; background: #1a1a2e !important; } /* Restore images, videos, iframes, canvas */ img, video, iframe, canvas, svg, picture, [style*="background-image"] { filter: invert(1) hue-rotate(180deg) !important; } /* Preserve specific elements that should not be inverted */ .no-dark-mode, .no-dark-mode *, [data-theme="light"], [data-theme="light"], .ace_editor, .ace_editor *, .CodeMirror, .CodeMirror *, .monaco-editor, .monaco-editor *, .markdown-body pre, .markdown-body pre *, .highlight, .highlight *, pre code, pre code * { filter: none !important; } /* Fix common UI elements */ .modal, .popup, .dropdown-menu, .tooltip, .popover { filter: invert(1) hue-rotate(180deg) !important; background: #2d2d44 !important; border-color: #444 !important; } /* Scrollbars */ ::-webkit-scrollbar { background: #1a1a2e !important; } ::-webkit-scrollbar-thumb { background: #444 !important; } ::-webkit-scrollbar-thumb:hover { background: #555 !important; } /* Selection */ ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; } ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; } '; } function removeDarkMode() { var style = document.getElementById('universal-dark-mode-style'); if (style) style.remove(); } // Toggle with Alt+Shift+D document.addEventListener('keydown', function(e) { if (e.altKey && e.shiftKey && e.key === 'D') { e.preventDefault(); enabled = !enabled; if (enabled) { applyDarkMode(); console.log('[Universal Dark Mode] Enabled'); } else { removeDarkMode(); console.log('[Universal Dark Mode] Disabled'); } } }); // Apply on load applyDarkMode(); // Re-apply on dynamic content var observer = new MutationObserver(function(mutations) { if (enabled && !document.getElementById('universal-dark-mode-style')) { applyDarkMode(); } }); observer.observe(document.head, { childList: true }); console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle'); })(); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })(); fix(types): declare QueryParams.$filter as the union it already accepts by yinlianghui · Pull Request #5999 · objectstack-ai/objectui · GitHub
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
38 changes: 38 additions & 0 deletions .changeset/3909-query-params-filter-union.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
---
'@object-ui/types': patch
'@object-ui/fields': patch
---

`QueryParams.$filter` now declares both shapes the data sources actually accept — the
MongoDB-style field-keyed record, or a `FilterArray`, the ObjectQL AST sugar bound from
`@objectstack/spec/data` (objectui#3909).

**Nothing is narrowed and no accepted value changes.** `Record<string, any>` already
accepted arrays structurally — they satisfy its string index — so the union documents
shapes that were always legal rather than admitting new ones. Measured both ways under
`tsc --strict`: all five inputs `translateFilterToAST` enumerates assign to the old and
new declarations alike, and both reject a bare number and a bare string identically. A
downstream `turbo run build` over all 43 dependent packages is green, which is the
evidence a published type change breaks no consumer.

The harm was entirely on the type face, and it was two-sided. The declaration blocked
nothing while describing one legal shape as though it were the only one — objectui#3831
is what that cost, a rule array accepted by a `Record<string, any>` slot, object-spread
flattened to `{"0": {...}}`, types green, and the query filtering on a column literally
named `0`. And someone writing a new consumer would read the type and its record-only
`@example`, conclude the array path was illegal, and add a tolerant conversion for it —
the "widen the consumer to tolerate the producer" shape AGENTS.md #0.1 forbids. Two
producers have fed arrays through this slot all along: `plugin-list`'s
`buildEffectiveFilter` (grid and export) and `plugin-view`'s `ObjectView` (calendar /
kanban / gallery / timeline). The runtime was right; the declaration was narrow.

The array half is **bound** to the spec's `FilterArray` rather than restated locally, so
it cannot fork from the vocabulary the servers parse — the same failure two hand-written
operator lists had in objectui#3948. The doc comment names `translateFilterToAST` as the
authoritative accepted set instead of carrying a second list to drift from.

`@object-ui/fields` drops the local cast this defect forced. PR objectui#3908 wrote
`filter as Record<string, any>` at one assignment in `useRecordQuery`, deliberately, as
debt rather than widening the shared type. `hasFilter` is now a type predicate narrowing
to the `$filter` slot's own type, so the assignment needs no cast and the guard cannot
drift from the declaration it guards. Type-only throughout; no runtime behaviour changes.
20 changes: 13 additions & 7 deletions packages/fields/src/widgets/useRecordQuery.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -111,8 +111,12 @@ export interface UseRecordQueryResult {
* `Object.keys` on an array returns its INDICES — so the record-only test read
* `['0','1','2']` for an AST node and was right only by accident. An empty array
* is "no filter" for the same reason an empty object is.
*
* Narrows to the `$filter` slot's own type rather than a restatement of it
* (#3909), so the caller assigns with no cast and this predicate cannot drift
* from the declaration it is guarding.
*/
function hasFilter(filter: unknown): boolean {
function hasFilter(filter: unknown): filter is NonNullable<QueryParams['$filter']> {
if (filter === null || filter === undefined) return false;
if (Array.isArray(filter)) return filter.length > 0;
if (typeof filter !== 'object') return false;
Expand DownExpand Up@@ -172,12 +176,14 @@ export function useRecordQuery(options: UseRecordQueryOptions): UseRecordQueryRe
if (searchTerm && searchTerm.trim()) params.$search = searchTerm.trim();
if (searchFields && searchFields.length > 0) params.$searchFields = searchFields;
if (sortArg) params.$orderby = { [sortArg.field]: sortArg.direction };
// `QueryParams.$filter` is declared `Record< string, any >`, which the
// AST-array form does not describe — the cast is at this ONE assignment
// rather than widening a shared type that several other producers
// (plugin-list's `buildEffectiveFilter`, plugin-view's ObjectView)
// already feed arrays through.
if (hasFilter(filter)) params.$filter = filter as Record<string, any>;
// No cast: `QueryParams.$filter` now declares both shapes it accepts
// (#3909), so the AST-array form the picker's merge yields is describable
// here. The local cast this replaces was deliberate debt — taken at this
// ONE assignment rather than widening the shared type that several other
// producers (plugin-list's `buildEffectiveFilter`, plugin-view's
// ObjectView) already feed arrays through. The shared type is honest now,
// so the debt is paid rather than moved.
if (hasFilter(filter)) params.$filter = filter;
if (expand && expand.length > 0) params.$expand = expand;

const result = await dataSource.find(objectName, params);
Expand Down
131 changes: 131 additions & 0 deletions packages/types/src/__tests__/query-params-filter-union.test.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
/**
* ObjectUI
* Copyright (c) 2024-present ObjectStack Inc.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

/**
* objectui#3909 — `QueryParams.$filter` declares BOTH shapes the data sources
* accept, and binds the array half to `@objectstack/spec`'s `FilterArray`
* rather than restating it.
*
* ## Why this file exists at all
*
* The defect it pins was invisible to every runtime suite, and had to be: the
* declaration was `Record< string, any >`, which **structurally accepts arrays**
* (they satisfy its string index). So the two producers that have fed ObjectQL
* AST arrays through this slot all along — `plugin-list`'s
* `buildEffectiveFilter` (grid and export) and `plugin-view`'s `ObjectView`
* (calendar / kanban / gallery / timeline) — type-checked, ran, and shipped
* correct results. Nothing was broken at runtime and nothing could go red.
*
* The cost was paid on the type face instead, in both directions:
*
* 1. The type **blocked nothing while describing one legal shape as if it were
* the only one**. objectui#3831 is what that buys: a `Record< string, any >`
* slot accepted a rule array, an object spread flattened it to
* `{"0": {...}}`, types stayed green, and the query filtered on a column
* literally named `0`.
* 2. Someone writing a new consumer reads the type and its `@example`,
* concludes only the record form is legal, and adds a tolerant conversion
* for the array path — the "widen the consumer to tolerate the producer"
* shape AGENTS.md #0.1 forbids.
*
* Both failure modes are compile-time by nature, so the pins are too. Reverting
* `$filter` to `Record< string, any >` leaves every runtime suite green and
* turns THIS FILE red under `tsc -p tsconfig.test.json` (the `type-check`
* script) — the drift's own signature, reproduced deliberately.
*
* ## What is NOT pinned here
*
* That the union is the *authoritative* accepted set. It is not: the authority
* is `translateFilterToAST` (`@object-ui/data-objectstack`), which enumerates
* five input shapes. A second list here would be a third place to drift from —
* which is exactly how two operator vocabularies came apart in #3948. The
* binding below is to the spec's `FilterArray`, so the array half cannot fork
* locally.
*/

import { describe, it, expect } from 'vitest';
import type { FilterArray } from '@objectstack/spec/data';
import type { QueryParams } from '../data';

type Assert< T extends true > = T;
/** True when `V` is accepted by the `$filter` slot. */
type AcceptsFilter< V > = V extends QueryParams['$filter'] ? true : false;
/** Exact type identity — NOT mutual assignability. See the note below. */
type Equal< A, B > =
(< T >() => T extends A ? 1 : 2) extends (< T >() => T extends B ? 1 : 2) ? true : false;

describe('QueryParams.$filter — declares the union it actually accepts (#3909)', () => {
it('accepts the MongoDB-style field-keyed record', () => {
type _Record = Assert< AcceptsFilter< { age: { $gt: number } } > >;
const params: QueryParams = { $filter: { age: { $gt: 18 }, status: 'active' } };
expect(params.$filter).toEqual({ age: { $gt: 18 }, status: 'active' });
});

it('accepts a bare AST comparison tuple with no cast', () => {
// The shape `buildEffectiveFilter` returns for a single condition. Before
// #3909 this compiled only because arrays satisfy `Record`'s string index —
// accepted by accident rather than by declaration.
const params: QueryParams = { $filter: ['status', '=', 'active'] };
expect(params.$filter).toEqual(['status', '=', 'active']);
});

it('accepts a logical AST group with no cast', () => {
// What `mergeFilterNodes` returns once more than one source is active.
const params: QueryParams = {
$filter: ['and', ['age', '>=', 18], ['status', '=', 'active']],
};
expect(Array.isArray(params.$filter)).toBe(true);
});

it('accepts the legacy bare list, combined with implicit AND', () => {
const params: QueryParams = {
$filter: [['stage', '=', 'won'], ['amount', '>', 1000]],
};
expect(Array.isArray(params.$filter)).toBe(true);
});

it('binds the array half to the spec, by IDENTITY not assignability', () => {
// ## Why this pin is an identity check, and why nothing weaker works
//
// Every assignment-shaped pin in this file is, on its own, VACUOUS as a
// regression guard — measured, not assumed. Reverting the declaration to
// `Record< string, any >` and re-running `type-check` leaves it GREEN
// (exit 0), because assignability cannot separate the two: arrays satisfy
// `Record< string, any >`'s string index, so `FilterArray extends
// QueryParams['$filter']` holds under BOTH declarations, and the old
// declaration is itself assignable to the new union. A guard that passes
// equally before and after the fix is a phantom check — it reads like
// enforcement and enforces nothing.
//
// Identity is the property that actually differs. This assertion goes red
// on a revert to the bare record, AND on the subtler regression: someone
// re-declaring a local `FilterNode` fork instead of binding the spec's
// type. That fork would satisfy every assignment above while being free to
// drift from the vocabulary the servers parse — the exact failure two
// hand-written operator lists had in #3948.
type _Bound = Assert<
Equal< NonNullable< QueryParams['$filter'] >, Record< string, any > | FilterArray >
>;
const fromSpec: FilterArray = ['status', '=', 'active'];
const params: QueryParams = { $filter: fromSpec };
expect(params.$filter).toBe(fromSpec);
});

it('still refuses a value that is neither shape', () => {
// The union documents; it must not have become `any` on the way. Note the
// slot sits on an interface that also carries `[key: string]: any` — these
// pins prove the declared property still wins over that index signature,
// which is the whole reason the declaration is worth anything.
// @ts-expect-error a number is not a filter
const bad: QueryParams = { $filter: 42 };
// @ts-expect-error a string is not a filter
const alsoBad: QueryParams = { $filter: 'status eq active' };
expect(bad.$filter).toBe(42);
expect(alsoBad.$filter).toBe('status eq active');
});
});
34 changes: 32 additions & 2 deletions packages/types/src/data.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -31,6 +31,7 @@ import type {
CreateExportJobInput as SpecCreateExportJobInput,
CreateExportJobResult,
} from '@objectstack/spec/contracts';
import type { FilterArray } from '@objectstack/spec/data';
import type { ValidationError } from '@objectstack/spec/kernel';

export type { ExportJobStatus, ImportJobStatus, ImportWriteMode, ValidationError };
Expand All@@ -47,10 +48,39 @@ export interface QueryParams {
$select?: string[];

/**
* Filter expression
* Filter expression, in either of the two forms the data sources accept:
* the MongoDB-style field-keyed record, or a `FilterArray` — the spec-owned
* ObjectQL AST sugar (`@objectstack/spec/data`).
*
* Both forms are normal here, and the array form is not an edge case: the
* repo's own canonical sink `mergeFilterNodes` / `toFilterNode`
* (`@object-ui/core`'s `filter-converter.ts`) returns AST nodes, and its two
* standing producers — `plugin-list`'s `buildEffectiveFilter` (grid and
* export) and `plugin-view`'s `ObjectView` (calendar / kanban / gallery /
* timeline) — have fed arrays through this slot all along.
*
* ⛔ Do not add a "tolerant conversion" in a consumer to cope with the array
* path. The array IS legal input; a consumer that needs one shape lowers
* through the shared sink rather than widening itself to tolerate the
* producer.
*
* The authoritative acceptable set is the one `translateFilterToAST`
* (`@object-ui/data-objectstack`'s `index.ts`) enumerates — five input
* shapes, of which the array forms below are three. Read it there rather than
* trusting a second list here; a partial restatement is exactly how two
* operator vocabularies drifted apart before.
*
* Note the declaration does not *narrow* anything: `Record<string, any>`
* already structurally accepts arrays (they satisfy its string index), so the
* union documents the shapes that were always accepted rather than admitting
* new ones. It is the description that was wrong, not the runtime.
*
* @example { age: { $gt: 18 }, status: 'active' }
* @example ['status', '=', 'active']
* @example ['and', ['age', '>=', 18], ['status', '=', 'active']]
* @example [['stage', '=', 'won'], ['amount', '>', 1000]]
*/
$filter?: Record<string, any>;
$filter?: Record<string, any> | FilterArray;

/**
* Sort order
Expand Down
Loading