Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat: Empty response support #2191

Merged
merged 100 commits into from
Nov 20, 2024
Merged

Feat: Empty response support #2191

merged 100 commits into from
Nov 20, 2024

Conversation

RobinTail
Copy link
Owner

@RobinTail RobinTail commented Nov 18, 2024

The following HTTP status codes imply no content in response:

  • 204
  • redirects, such as 302

First one usually used for REST deletions.

There should be an easy way to describe and such things. Basically, there is no MIME type for those cases, because there is no body.

Current obstacles:

  • ResultHandler::positive::mimeTypes is currently non-empty array, either mimeType is a string
  • z.never() is probably the best candidate for the schema but it's unsupported
    • however, not: {} is used for describing fixed tuples, which the best depiction

Consideration:

  • if never type should be used in the client, because of no body
  • check the presence of content type in example impl.
  • or maybe it should be undefined, because never means that .provide() does not return at all, such as throw case
  • or maybe response type should not be processed by ZTS at all, similar to the desired OpenAPI no-MIME-type case
  • probably it should be the feature of v21 having less variation on MIME type declaration

Changing peer dependencies (breaking)
This feature allows to avoid spinning up HTTP server when only HTTPS one
is needed.
It should enable best practices in the establishing secure communication
channel.

Changes to `ServerConfig` type:
- `server` prop renamed to `http`
- nested props `jsonParser`, `upload`, `compression`, `rawParser` and
`beforeRouting` moved to the top level
- all that included into the automated migration (ESLint rule) for v21
This should ease declaration and enable future improvements like #2099 

Making `createServer()` to return an object having `servers` property
instead of `httpServer` and `httpsServer`
Due to #2144 

- [x] needs a migration script
@RobinTail RobinTail marked this pull request as ready for review November 19, 2024 10:47
@RobinTail RobinTail changed the title Feat: No Content response support feat(v21): No Content response support Nov 19, 2024
src/api-response.ts Outdated Show resolved Hide resolved
src/result-helpers.ts Outdated Show resolved Hide resolved
Copy link
Owner Author

@RobinTail RobinTail left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good, it's very straightforward now

needs only documentation

Base automatically changed from make-v21 to master November 20, 2024 09:53
@RobinTail RobinTail removed this from the v21 milestone Nov 20, 2024
@RobinTail RobinTail changed the title feat(v21): No Content response support Feat: No Content response support Nov 20, 2024
@RobinTail RobinTail changed the title Feat: No Content response support Feat: Empty response support Nov 20, 2024
Copy link
Owner Author

@RobinTail RobinTail left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

@RobinTail
Copy link
Owner Author

🚀 v21.1.0
✅ QA passed

Copy link
Owner Author

@RobinTail RobinTail left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ready

@RobinTail RobinTail merged commit 5e6354e into master Nov 20, 2024
12 checks passed
@RobinTail RobinTail deleted the no-content-response branch November 20, 2024 21:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request miracle Mysterious events are happening here
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant