Uh oh!
There was an error while loading. Please reload this page.
fix(types): declare QueryParams.$filter as the union it already accepts - #5999
Merged
Conversation
`$filter` was declared `Record<string, any>` with a record-only `@example`, while two standing producers — plugin-list's `buildEffectiveFilter` and plugin-view's `ObjectView` — have fed ObjectQL AST arrays through the slot all along, and the data sources accept them. Nothing is narrowed: `Record<string, any>` already accepts arrays structurally, so the union documents what was always legal. The array half is bound to `@objectstack/spec/data`'s `FilterArray` rather than restated, and the comment names `translateFilterToAST` as the authoritative accepted set rather than carrying a second list to drift from. Drops the cast objectui#3908 took at one assignment in `useRecordQuery` as declared debt; `hasFilter` narrows to the slot's own type instead. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CSoz9uGhaaSgiq3hshtN7L
The assignment-shaped pins were vacuous as regression guards, measured rather than assumed: reverting the declaration to `Record<string, any>` left `type-check` green (exit 0). Assignability cannot separate the two — arrays satisfy the record's string index, so `FilterArray extends QueryParams['$filter']` holds under both declarations. Identity is the property that differs. The pin now fails on a revert AND on a locally forked AST type replacing the spec binding. Reverse-verified: mutated tree -> type-check exit 2 (TS2344), vitest 6/6 green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CSoz9uGhaaSgiq3hshtN7L
Contributor
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
yinlianghui
marked this pull request as ready for review
August 24, 2026 11:44
Uh oh!
There was an error while loading. Please reload this page.
This was referenced Aug 24, 2026
This was referenced Aug 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes#3909
QueryParams.$filterwas declaredRecord<string, any>with a record-only@example, while two standing producers have fed ObjectQL AST arrays through the slot all along and the data sources accept them. The runtime was right; the declaration was narrow.What changed
$filternow declares both shapes it accepts. The array half is bound to@objectstack/spec/data'sFilterArrayrather than restated locally:Binding rather than re-declaring is the file's own stated convention (
Spec-owned names are bound here, not re-declared), and it matters here specifically: a localFilterNodefork would satisfy every call site while being free to drift from the vocabulary the servers parse — the exact failure two hand-written operator lists had in #3948. The spec'sFilterArraydoc even names "the wire$filterface" as one of its measured producers, so this slot was already in its declared scope.The doc comment names
translateFilterToAST(@object-ui/data-objectstack) as the authoritative accepted set rather than carrying a second list to drift from, and gains array@examples.Nothing is narrowed — measured, not assumed
Record<string, any>already accepts arrays structurally (they satisfy its string index), so the union documents shapes that were always legal rather than admitting new ones. Measured under the repo'stscwith--strict, no casts:{ age: { $gt: 18 } }['status', '=', 'active']['and', [...], [...]][[...], [...]](legacy list)[{ field, operator, value }]42/'status eq active'The rejection row is a negative control: it proves the probe can go red, and that
QueryParams'[key: string]: anyindex signature is not swallowing the slot.Why it was worth fixing anyway
The harm was entirely on the type face and two-sided:
record:related_list.add.picker.filter全仓零读点:作者限定了 Add 选择器的候选范围,对话框照样提供该对象的全部记录 #3831 is what that cost: aRecord<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 named0.@example, concludes the array path is illegal, and adds a tolerant conversion for it — the "widen the consumer to tolerate the producer" shape AGENTS.md #0.1 forbids.The #3908 workaround is removed
PR #3908 wrote
filter as Record<string, any>at one assignment inpackages/fields/src/widgets/useRecordQuery.ts, deliberately, as debt rather than widening the shared type — its comment said so. The shared type is honest now, so the debt is paid rather than moved:hasFilteris a type predicate narrowing to the$filterslot's own type (filter is NonNullable<QueryParams['$filter']>), so the assignment needs no cast and the guard cannot drift from the declaration it guards.The pin test was a phantom, and that is why it is an identity check
Worth flagging, because the first version of the guard was wrong in a way that reads like enforcement. Every assignment-shaped pin is vacuous as a regression guard here — measured, not assumed. Reverting the declaration to the bare record and re-running
type-checkleft it green (exit 0): assignability cannot separate the two, since arrays satisfyRecord<string, any>'s string index, soFilterArray extends QueryParams['$filter']holds under both declarations and the old declaration is itself assignable to the new union.Identity is the property that actually differs, so the pin asserts it:
Reverse-verified from a committed state, mutation confirmed on disk, mutation and measurement in one shell invocation with a restoring
trap:type-checktype-checkTS2344: Type 'false' does not satisfy the constraint 'true'That runtime-green/compile-red split is the drift's own signature: no runtime suite could ever have caught this.
Verification
All at
ab70c650:pnpm --filter @object-ui/types build— exit 0pnpm --filter @object-ui/types type-check— exit 0 (tsc --noEmit && tsc -p tsconfig.examples.json && tsc -p tsconfig.test.json)pnpm --filter @object-ui/fields type-check— exit 0pnpm exec vitest run packages/types/src/__tests__/query-params-filter-union.test.ts—Tests 6 passed (6)turbo run build --filter '...@object-ui/types'(prefix = dependents): 43 successful, 43 total. Each is atscof its ownsrc/against the newdist/*.d.ts, which is the evidence a published type change breaks no consumer.eslint . --no-inline-configrepo-wide: population 3626 files; my three files contribute 0 errors (37 warnings, all pre-existingno-explicit-any/react-hookson lines untouched by this diff). The 74 error-bearing files are all outside the diff, and type-aware linting is not configured, so this change provably cannot move any untouched file's verdict.Type-only throughout; no runtime behaviour changes.
Generated by Claude Code
Generated by Claude Code