You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Hi!
I encountered this issue while describing my endpoints, and it can be demonstrated with the flowers api example.
Setting up the example from the wiki:
// ...exportconstflowerAPI=defineAPI({listFlowers: GET`/flowers`.query({'sortBy': FlowerIndexedAttribute,'filterBy': FlowerIndexedAttribute}).response(rt.Array(Flower))//...});// if you create a clientimportaxiosfrom'axios';constdriver=axios.create(config);constflowers=createConsumer(flowerAPI,driver);// and when you try to call it with the supposedly optional query params:flowers.listFlowers({query: {sortBy: 'color'// filterBy: 'color' // without this parameter it will not pass typecheck}});
The error comes from the type calculations made on the inferred incoming type in defineAPI will see every Runtype defined type requirements as present, non-undefined, so none of them can be left out ultimately. (I hope this makes sense).
For now I don't know what would be a good solution, but I feel that if we actually want to allow ALL query params as optional (as they usually should be in RESTful design) then maybe the type calculations shouldn't be removing undefineable fields, but rather making the whole query type Partial.
Actually this is pretty bad because in the server all parameters are typed as string and not string | undefined, but the route will match regardless of whether the parameter was provided or not... So potential for crash right here.
The prototype of query should be changed to something like this:
Just a comment - I agree that in RESTful design it's usually good practice to have all query params as optional but there are some legit cases where you want query params to be mandatory, so I think the framework should allow for that too.
So there should be either a flag to mark properties as mandatory, or a flag to mark them as optional or use some Partial runtimes wrapper.
Maybe there should be a runtime type check for request query parameters, similar to the one available for request body with runtypes.
The thing is: query params are user input (as seen by the server). We can make rest.ts enforce mandatory query params on the client, but the server might still receive bad data.
Hi!
I encountered this issue while describing my endpoints, and it can be demonstrated with the flowers api example.
Setting up the example from the wiki:
As you can see, in the example, I'm calling the created API endpoint with an optional query param (I would like to think all query params are optional by default as stated here: https://github.com/hmil/rest.ts/blob/master/packages/rest-ts-core/src/builder-kit.ts#L136) it fails.
The error comes from the type calculations made on the inferred incoming type in defineAPI will see every Runtype defined type requirements as present, non-undefined, so none of them can be left out ultimately. (I hope this makes sense).
For now I don't know what would be a good solution, but I feel that if we actually want to allow ALL query params as optional (as they usually should be in RESTful design) then maybe the type calculations shouldn't be removing undefineable fields, but rather making the whole query type Partial.
My workaround, btw with the current code is this:
What do you think? Is this a legit concern?
The text was updated successfully, but these errors were encountered: