Repository navigation
fix(filters)!: type a filter value as string or boolean - #134
Merged
Merged
Conversation
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 free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Why
ANT-2463. The published
FilterValue = [value: string, mode: string]does not describe what the filters carry. dms's archive filter (packages/dms/src/implementations/dms-base/search-route.ts,archiveFilter) writes the booleantrue. Every data-api reader passes the value to the database unchanged: the field's filter, the default filter (proxy.eqandproxy.ne), and the indexedgetAll. The type made dms keep aSAFETY:comment and atrue as unknown as string.What
FilterValueis[value: string | boolean, mode: string], the narrowest type that covers dms. No reader needs more: the default filter's comparisons andgetAllalready accept a boolean.FilterFunction'svaluereadsFilterValue[0], so it widens with it. The default filter's own operator type follows. Behaviour is unchanged.docs/8.filters.mdsays a route may write a boolean value and that every reader passes it unchanged. ItsnumberFilterandbooleanFilterexamples now handlestring | boolean; the oldraw.trim()no longer type-checks. Thedata-api-interfaceskill describesFilterValue.src/tests/components/boolean_filter.test.tsdrivesQuery.Listwith route-written boolean filters on a table with an@Index()boolean field:archived: [true, "eq"]goes through the indexedgetAll(the source's stages holdgetAll) and answers the two archived rows;archived: [true, "ne"]goes through the default filter, with nogetAll, and answers the other row;pinned: [true, "eq"]goes through the field's own filter.valueas text now callparseFloat(String(value)).flowchart LR route["route writes [true, mode]"] --> idx{"eq on an indexed field?"} idx -- yes --> getAll["table.getAll(true, index)"] idx -- no --> f{"field filter"} f -- default --> def["proxy.eq / ne(true)"] f -- custom --> own["filter(ctx, proxy, key, true, mode)"]Compatibility and version
This is source-breaking for one kind of implementer, so I name the version 0.8.0, a minor bump in 0.x.
FilterFunctionis a function type, so understrictFunctionTypesitsvalueparameter is checked contravariantly. A probe against this head:valueis annotatedstring(line 6), and a filter that usesvalueas text, such asparseFloat(value), even with the type inferred (line 7).valuethrough, an inferred@Filter((…, value) => proxy.eq(value)), and a filter that ignoresvalue.>=0.7.2 <0.8.0, so dms (ANT-1088) moves its pin to>=0.8.0 <0.9.0with this tarball. dms can then drop theSAFETY:comment and the cast.Upgrade note for the
docs(changelog)PR after the release:SAFETY comments added
None. The suppression budget is unchanged.
Proof
On
origin/main, the new test does not type-check: a boolean filter value is refused, which is the bug.On this head:
pnpm run checks: 11 passed, 0 failed.pnpm exec tsc -p docs/tsconfig.jsonpasses. Thedocs/8.filters.mddocs-checkblock, compiled against this head: the old block fails withTS2339: Property 'trim' does not exist on type 'string | boolean', and the new one compiles.The api-diff differs on one line,
value: string→value: boolean | stringin theFilterValuetuple, repeated in each of the 20 bundled entries. Runtime is unchanged.antelopejs-api-diff origin/main