Repository navigation
fix(engine): emit query parameters that admit QueryValue - #196
Merged
Merged
Conversation
…from interface-api 0.3.1+
…p typecheck from deleting them
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
The Builder has to write period-bound and comparison queries that typecheck on the current
@antelopejs/interface-api. Since 0.3.1,@Parameter(name, "query")refuses a handler argument whose type does not admitQueryValue(string | true | undefined).@antelopejs/dms-builder@0.10.2emits@Parameter("from", "query") from: string. The engine typechecks every page it writes, so in a project on 0.3.2 the Builder refuses to save the query with TS1239. That is the known issue in AntelopeJS/template-dms-connect-demo#21, where the assistant could not write the period-bound queries (step 7).This repository did not notice because its lockfile still resolved interface-api 0.3.0.
Changes
engine/query-emit.ts). A query-string parameter is declaredstring | true | undefinedand narrowed in the call:new Date(String(from))for a date,String(status)for text.Number(x)andx === "true"already accept the union, so they are unchanged. A path parameter keepsstring. The type is written out rather than imported asQueryValue, because interface-api 0.3.0 does not export that name and a project may still resolve it.engine/query-structure.ts). The Builder readsString(x)andnew Date(String(x))back as the parameterx. Routes written by 0.10.2 still read back.engine/project-scaffold.ts) and both playgrounds now require interface-api>=0.3.2 <1.0.0. The lockfiles resolve 0.3.2, and the README table is regenerated.routes/builder.ts,routes/database.ts). Their 16 query parameters had the same bug, which the 0.3.2 floor exposes. They now takeQueryValueand keep only real text throughroutes/query-text.ts. A?refwith no value becomes"", which every engine operation answers as not found. Before, it reached them astrue.10.a-chart-without-code.mdshows the new route shape.package.json).antelopejs-checksrunstypecheckandlintat the same time.typecheckranrimraf dist && tscon the interface while type-aware lint read its declarations. When lint loaded first, every interface type became an error type: 2,514 lint errors on this PR's first four CI runs, and the failuresmainhas had on several pushes. A newchecks:preparescript builds bothdistfolders once before the steps start.typecheckstill runs a fulltscover both packages and still emits bothdistfolders, but no longer deletes them first. With no interfacedist,oxlintreproduces exactly those 2,514 errors locally.flowchart LR A[AddQuery with $param from/to] --> B[emit route] B -->|0.10.2| C["from: string"] --> D{engine typecheck on interface-api 0.3.2} D -->|TS1239| E[typecheck_failed, nothing written] B -->|this PR| F["from: string | true | undefined<br/>new Date(String(from))"] --> G{engine typecheck} G -->|ok| H[page written, route answers]Proof
The failing test lands before the fix:
58424f5adds it on top of the 0.3.2 floor (099fc69), ande0c84c9fixes the emitter.At
58424f5,mocha dist/test/query-response.spec.js dist/test/query-route-read.spec.js:At head, the same two files plus
project-without-tsconfig.spec.js:The full engine suite (
pnpm --filter @antelopejs/dms-builder test:unit) shows527 passing (4m), andpnpm run checksshowsChecks: 12 passed, 0 failed.The generated route shape, compiled with
tsc --strict --experimentalDecorators --emitDecoratorMetadataagainst each interface-api version:Builder end to end
The
verify-dms-builderjourneyqueries/a-period-bound-series-and-a-comparison-card-typecheck-and-answer-over-httpfollows the step 7 scenario of template-dms-connect-demo#21 on a DMS with interface-api 0.3.2.With the emitter from
58424f5, the journey fails:At head, it passes 16 of 16 checks:
AddQuerycalls succeed, and the page imports noQueryValue.paid=2,refunded=1for 50 to 20 days ago andpaid=2,pending=1for the last 20 days.value705 againstpreviousValue530, adeltaof 33.02 percent.The whole skill run (smoke and all 17 features) passes 59 of 59 journeys. Its verdict is PASS+NOTES.
Attention
String(undefined)is"undefined"and a bare?xis"true". For date bounds this is still an Invalid Date, as before. For a text filter it was already wrong under thestringtype, sinceundefinedortruereached the model. The preview refuses a missing value, while the route does not. Matching it would mean answering 400, which changes the route's return type and how the Builder reads the route back. That belongs in its own change.from: stringuntil their query is reconfigured, so they still fail typecheck on interface-api 0.3.1 and later. This PR does not migrate them.@antelopejs/dms-builderneeds a release for projects to get the fix. The interface package changes only in its docs.