Conversation
Contributor
📊 JSDoc Documentation Analysis📈 Current Analysis ResultsThis comment is automatically updated on each push. View the analysis script for details. |
Contributor
Bundle Size Analysis
|
e8802d6 to
950fedd
Compare
18c4ed1 to
75e4198
Compare
Closed
6e1aa44 to
6372efb
Compare
cf31c52 to
e1b887d
Compare
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.
Request Context Renamings
To align with other frameworks, the proposal is to rename:
pathtoparams(Express, Hono, Elysia)urlParamstoquery(Express, Hono, Elysia)I also propose requiring a record of schemas (instead of allowing a single schema like before) for:
GET)This makes the API easier to read and avoids issues when generating an OpenAPI spec. OpenAPI needs a record of schemas and cannot represent other schema shapes (for example, a union).
Example (Declaring params, query, and headers)
Status Codes
I added an explicit
HttpApiSchema.statusAPI to set the response status code.We can keep the
httpApiStatusannotation, or remove it entirely.Encodings
I propose removing explicit encoding annotations and using these APIs instead of
HttpApiSchema.withEncoding:HttpApiSchema.asJsonHttpApiSchema.asUrlParamsHttpApiSchema.asTextHttpApiSchema.asUint8ArrayHttpApiSchema.asMultipartHttpApiSchema.asMultipartStreamI use
HttpApiSchema.asUrlParamshere because it's like this now but I think it's a bit confusing, maybeasQuerywould be better to align withqueryabove, orasFormUrlEncodedMultiple Encodings
Right now there is also a hidden assumption: encoding annotations only apply at the top level (nested unions are not handled). To make this behavior explicit, I propose using an array of schemas for
payload,success, anderror.Example (Multiple payloads and responses with status and encoding)
No Content Handling
Schema.Voidis still treated as "no content".The general rule is unchanged: if a schema encodes to
Schema.Void, the response is treated as having no content.Previously, decoding a "no content" response required using
HttpApiSchema.asEmptyorHttpApiSchema.EmptyErrorClass(which was a bit hacky).The proposal is to replace both with a single API:
HttpApiSchema.asNoContent.Example (Decoding typed errors from no-content responses)