-
-
Notifications
You must be signed in to change notification settings - Fork 35
feat: HEAD method handling
#2816
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
Conversation
WalkthroughThe changes implement and test proper support for the HTTP HEAD method throughout the codebase. Adjustments include treating HEAD requests like GET for input extraction, ensuring HEAD is included in allowed methods and CORS headers, and handling HEAD requests correctly in file streaming endpoints by setting headers without sending the file body. Several new and updated tests verify this behavior. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Server
participant FileHandler
Client->>Server: HEAD /file
Server->>FileHandler: get file metadata
FileHandler-->>Server: filename, size
Server-->>Client: 200 OK + headers (Content-Type, Content-Length, etc.)
Note right of Server: No file body is sent
Possibly related PRs
Suggested labels
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🧠 Learnings (2)📓 Common learningsexpress-zod-api/tests/method.spec.ts (9)🧬 Code Graph Analysis (1)express-zod-api/tests/method.spec.ts (1)
🪛 GitHub Check: build (24.0.0)express-zod-api/tests/method.spec.ts[warning] 10-10: 🪛 GitHub Check: build (24.x)express-zod-api/tests/method.spec.ts[warning] 10-10: ⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (4)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
turned out that |
c2b65cb to
bfccb45
Compare
|
Should somehow reflect |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
example/example.client.ts (1)
189-206: Replace '{}' type with a more explicit empty object type.The HEAD endpoint implementation is correct, but the input type uses '{}' which is discouraged. Consider using a more explicit type for empty objects.
Apply this diff to improve type safety:
-/** head /v1/user/list */ -type HeadV1UserListInput = {}; +/** head /v1/user/list */ +type HeadV1UserListInput = Record<string, never>;Alternatively, you could define an explicit empty interface:
-/** head /v1/user/list */ -type HeadV1UserListInput = {}; +/** head /v1/user/list */ +interface HeadV1UserListInput {}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
express-zod-api/tests/__snapshots__/documentation.spec.ts.snapis excluded by!**/*.snapexpress-zod-api/tests/__snapshots__/integration.spec.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (11)
example/example.client.ts(12 hunks)example/example.documentation.yaml(5 hunks)express-zod-api/src/common-helpers.ts(1 hunks)express-zod-api/src/documentation-helpers.ts(3 hunks)express-zod-api/src/documentation.ts(7 hunks)express-zod-api/src/endpoint.ts(4 hunks)express-zod-api/src/endpoints-factory.ts(3 hunks)express-zod-api/src/integration-base.ts(5 hunks)express-zod-api/src/integration.ts(4 hunks)express-zod-api/src/method.ts(1 hunks)express-zod-api/src/routing-walker.ts(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- express-zod-api/src/routing-walker.ts
- express-zod-api/src/documentation-helpers.ts
- example/example.documentation.yaml
🚧 Files skipped from review as they are similar to previous changes (2)
- express-zod-api/src/method.ts
- express-zod-api/src/common-helpers.ts
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: RobinTail
PR: RobinTail/express-zod-api#2697
File: CHANGELOG.md:5-5
Timestamp: 2025-06-02T21:11:20.768Z
Learning: In the express-zod-api repository, RobinTail follows a release workflow where package.json version is only updated on the master branch after merging all planned release changes. Changelog entries may show future version numbers while package.json remains at the previous version during feature development, and this is intentional workflow, not a version inconsistency that needs to be flagged.
express-zod-api/src/endpoints-factory.ts (2)
Learnt from: RobinTail
PR: RobinTail/express-zod-api#2428
File: express-zod-api/src/index.ts:44-44
Timestamp: 2025-05-28T18:58:10.064Z
Learning: The type-only import `import type {} from "qs";` in express-zod-api/src/index.ts is necessary to avoid TS2742 errors for exported functions like attachRouting, makeRequestMock, testEndpoint, and testMiddleware that have types depending on @types/qs. This import provides the reference TypeScript needs to infer portable type names.
Learnt from: RobinTail
PR: RobinTail/express-zod-api#2736
File: express-zod-api/tsup.config.ts:12-26
Timestamp: 2025-06-14T16:42:52.972Z
Learning: In express-zod-api tsup configurations, the direct mutation of `options.supported` in the `esbuildOptions` callback is intentional behavior and should not be flagged as a side effect issue.
express-zod-api/src/integration.ts (10)
Learnt from: RobinTail
PR: RobinTail/express-zod-api#2428
File: express-zod-api/src/index.ts:44-44
Timestamp: 2025-05-28T18:58:10.064Z
Learning: The type-only import `import type {} from "qs";` in express-zod-api/src/index.ts is necessary to avoid TS2742 errors for exported functions like attachRouting, makeRequestMock, testEndpoint, and testMiddleware that have types depending on @types/qs. This import provides the reference TypeScript needs to infer portable type names.
Learnt from: RobinTail
PR: RobinTail/express-zod-api#2736
File: express-zod-api/tsup.config.ts:12-26
Timestamp: 2025-06-14T16:42:52.972Z
Learning: In express-zod-api tsup configurations, the direct mutation of `options.supported` in the `esbuildOptions` callback is intentional behavior and should not be flagged as a side effect issue.
Learnt from: RobinTail
PR: RobinTail/express-zod-api#2546
File: example/factories.ts:35-42
Timestamp: 2025-05-27T20:03:34.213Z
Learning: The `./example` directory in the express-zod-api repository contains demonstration code for educational purposes only, not intended for production use. Example code can make simplified assumptions for brevity and clarity, and should not be flagged for missing production-level error handling, security measures, or edge case handling.
Learnt from: RobinTail
PR: RobinTail/express-zod-api#2546
File: express-zod-api/src/documentation-helpers.ts:508-512
Timestamp: 2025-05-28T07:58:09.853Z
Learning: In express-zod-api, when working with Zod's JSON schema override callbacks, using `delete` to mutate `ctx.jsonSchema` is the recommended approach per Zod's official documentation, even if it triggers performance linting warnings. This is preferable to creating copies with `undefined` values, especially for snapshot testing.
Learnt from: RobinTail
PR: RobinTail/express-zod-api#2546
File: express-zod-api/src/json-schema-helpers.ts:1-3
Timestamp: 2025-05-27T20:27:17.015Z
Learning: Ramda is correctly listed as a dependency in express-zod-api/package.json, so imports of ramda utilities are properly supported.
Learnt from: RobinTail
PR: RobinTail/express-zod-api#2546
File: express-zod-api/src/json-schema-helpers.ts:1-3
Timestamp: 2025-05-27T20:27:17.015Z
Learning: Ramda is correctly listed as a dependency in express-zod-api/package.json, so imports of ramda utilities are properly supported.
Learnt from: RobinTail
PR: RobinTail/express-zod-api#2697
File: CHANGELOG.md:5-5
Timestamp: 2025-06-02T21:08:56.475Z
Learning: The `cjs-test` directory in the express-zod-api repository is a test workspace and should be excluded when checking for main project version consistency with changelog entries.
Learnt from: RobinTail
PR: RobinTail/express-zod-api#2546
File: express-zod-api/src/json-schema-helpers.ts:75-87
Timestamp: 2025-05-27T20:40:19.548Z
Learning: In express-zod-api's `flattenIO` function in json-schema-helpers.ts, the `additionalProperties` field is used as a template to generate property schemas for literal property names extracted from `propertyNames.const` and `propertyNames.enum`. Converting boolean `additionalProperties` values to empty objects `{}` via `Object(entry.additionalProperties)` is intentional behavior, as the function only needs property schema templates, not the boolean semantics of `additionalProperties`.
Learnt from: RobinTail
PR: RobinTail/express-zod-api#2546
File: express-zod-api/tests/form-schema.spec.ts:31-31
Timestamp: 2025-05-27T19:27:13.492Z
Learning: Zod version 3.25.0 and later expose the Zod v4 API through the special import paths "zod/v4" and "zod/v4/core", allowing v4 features like .loose() to be used even when the package.json dependency shows a 3.x version.
Learnt from: RobinTail
PR: RobinTail/express-zod-api#2546
File: express-zod-api/tests/buffer-schema.spec.ts:32-37
Timestamp: 2025-05-27T19:35:57.357Z
Learning: In the express-zod-api project, tests are run from the `express-zod-api` workspace directory, and the project uses an ESM-first environment without `__dirname`. Relative paths like `../logo.svg` in test files correctly resolve to the repository root due to this test execution context.
express-zod-api/src/integration-base.ts (4)
Learnt from: RobinTail
PR: RobinTail/express-zod-api#2428
File: express-zod-api/src/index.ts:44-44
Timestamp: 2025-05-28T18:58:10.064Z
Learning: The type-only import `import type {} from "qs";` in express-zod-api/src/index.ts is necessary to avoid TS2742 errors for exported functions like attachRouting, makeRequestMock, testEndpoint, and testMiddleware that have types depending on @types/qs. This import provides the reference TypeScript needs to infer portable type names.
Learnt from: RobinTail
PR: RobinTail/express-zod-api#2736
File: express-zod-api/tsup.config.ts:12-26
Timestamp: 2025-06-14T16:42:52.972Z
Learning: In express-zod-api tsup configurations, the direct mutation of `options.supported` in the `esbuildOptions` callback is intentional behavior and should not be flagged as a side effect issue.
Learnt from: RobinTail
PR: RobinTail/express-zod-api#2546
File: express-zod-api/tests/form-schema.spec.ts:31-31
Timestamp: 2025-05-27T19:27:13.492Z
Learning: Zod version 3.25.0 and later expose the Zod v4 API through the special import paths "zod/v4" and "zod/v4/core", allowing v4 features like .loose() to be used even when the package.json dependency shows a 3.x version.
Learnt from: RobinTail
PR: RobinTail/express-zod-api#2546
File: example/factories.ts:35-42
Timestamp: 2025-05-27T20:03:34.213Z
Learning: The `./example` directory in the express-zod-api repository contains demonstration code for educational purposes only, not intended for production use. Example code can make simplified assumptions for brevity and clarity, and should not be flagged for missing production-level error handling, security measures, or edge case handling.
express-zod-api/src/endpoint.ts (3)
Learnt from: RobinTail
PR: RobinTail/express-zod-api#2428
File: express-zod-api/src/index.ts:44-44
Timestamp: 2025-05-28T18:58:10.064Z
Learning: The type-only import `import type {} from "qs";` in express-zod-api/src/index.ts is necessary to avoid TS2742 errors for exported functions like attachRouting, makeRequestMock, testEndpoint, and testMiddleware that have types depending on @types/qs. This import provides the reference TypeScript needs to infer portable type names.
Learnt from: RobinTail
PR: RobinTail/express-zod-api#2736
File: express-zod-api/tsup.config.ts:12-26
Timestamp: 2025-06-14T16:42:52.972Z
Learning: In express-zod-api tsup configurations, the direct mutation of `options.supported` in the `esbuildOptions` callback is intentional behavior and should not be flagged as a side effect issue.
Learnt from: RobinTail
PR: RobinTail/express-zod-api#2546
File: express-zod-api/tests/buffer-schema.spec.ts:32-37
Timestamp: 2025-05-27T19:35:57.357Z
Learning: In the express-zod-api project, tests are run from the `express-zod-api` workspace directory, and the project uses an ESM-first environment without `__dirname`. Relative paths like `../logo.svg` in test files correctly resolve to the repository root due to this test execution context.
express-zod-api/src/documentation.ts (4)
Learnt from: RobinTail
PR: RobinTail/express-zod-api#2428
File: express-zod-api/src/index.ts:44-44
Timestamp: 2025-05-28T18:58:10.064Z
Learning: The type-only import `import type {} from "qs";` in express-zod-api/src/index.ts is necessary to avoid TS2742 errors for exported functions like attachRouting, makeRequestMock, testEndpoint, and testMiddleware that have types depending on @types/qs. This import provides the reference TypeScript needs to infer portable type names.
Learnt from: RobinTail
PR: RobinTail/express-zod-api#2736
File: express-zod-api/tsup.config.ts:12-26
Timestamp: 2025-06-14T16:42:52.972Z
Learning: In express-zod-api tsup configurations, the direct mutation of `options.supported` in the `esbuildOptions` callback is intentional behavior and should not be flagged as a side effect issue.
Learnt from: RobinTail
PR: RobinTail/express-zod-api#2546
File: express-zod-api/tests/form-schema.spec.ts:31-31
Timestamp: 2025-05-27T19:27:13.492Z
Learning: Zod version 3.25.0 and later expose the Zod v4 API through the special import paths "zod/v4" and "zod/v4/core", allowing v4 features like .loose() to be used even when the package.json dependency shows a 3.x version.
Learnt from: RobinTail
PR: RobinTail/express-zod-api#2546
File: example/factories.ts:35-42
Timestamp: 2025-05-27T20:03:34.213Z
Learning: The `./example` directory in the express-zod-api repository contains demonstration code for educational purposes only, not intended for production use. Example code can make simplified assumptions for brevity and clarity, and should not be flagged for missing production-level error handling, security measures, or edge case handling.
🧬 Code Graph Analysis (2)
express-zod-api/src/endpoints-factory.ts (1)
express-zod-api/src/method.ts (1)
ClientMethod(19-19)
express-zod-api/src/endpoint.ts (1)
express-zod-api/src/method.ts (1)
ClientMethod(19-19)
🪛 Biome (1.9.4)
example/example.client.ts
[error] 190-190: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: report
- GitHub Check: Analyze (javascript)
🔇 Additional comments (28)
express-zod-api/src/endpoints-factory.ts (3)
10-10: Import update looks good.The addition of
ClientMethodimport is necessary to support the new HEAD method handling functionality.
48-48: Correct type update for operation ID.Updating the
operationIdproperty to acceptClientMethodparameter aligns with the new HEAD method support and ensures consistent typing across the framework.
131-134: Well-implemented HEAD operation ID handling.The logic correctly ensures unique operation IDs for HEAD methods by appending
"__HEAD"suffix while preserving backward compatibility for other methods. This follows the non-breaking change principle mentioned in the comment.express-zod-api/src/integration-base.ts (5)
5-5: Correct import update for HEAD method support.Replacing
MethodandmethodswithClientMethodandclientMethodsproperly supports the new HEAD method functionality.
99-99: Documentation updated to reflect HEAD method support.The example comment correctly includes
"head"in the list of supported methods.
102-102: Method type property correctly updated.Using
clientMethodsinstead ofmethodsensures the type includes the HEAD method.
419-421: Correct HEAD method handling in request body logic.Adding
"head"to the list of methods that don't have request bodies is correct according to HTTP specifications. HEAD requests, like GET and DELETE, should not include a request body.
629-629: Usage examples properly typed.The usage statements correctly use
ClientMethodtype assertions for the example endpoints, maintaining type safety.Also applies to: 638-638
express-zod-api/src/endpoint.ts (4)
26-26: Import update is correct.Adding
ClientMethodimport supports the type updates for HEAD method handling.
57-57: Abstract method signature correctly updated.Changing the parameter type from
MethodtoClientMethodaligns with the HEAD method support and maintains consistency across the framework.
108-108: Constructor parameter type correctly updated.The
getOperationIdfunction parameter type change toClientMethodmaintains consistency with the abstract method signature.
197-197: Implementation method signature correctly updated.The override method parameter type change to
ClientMethodmaintains consistency with the abstract method and constructor.express-zod-api/src/integration.ts (5)
25-26: Import updates support HEAD method functionality.Adding
AbstractEndpointandClientMethodimports provides the necessary types for the HEAD method implementation.
99-103: Explicit typing improves code clarity.The explicit parameter types for the
onEndpointcallback enhance type safety and make the HEAD method support more apparent.
115-117: Correct HEAD method response handling.The
hasContentlogic properly implements HTTP HEAD method specification by excluding response content for HEAD requests with positive responses, while preserving content for error responses.
120-120: Proper use of hasContent flag.Using
hasContentinstead of justmimeTypesensures HEAD responses are correctly handled as no-content responses.
156-162: Correct HEAD endpoint duplication strategy.The approach of duplicating GET endpoints as HEAD endpoints aligns with HTTP specifications and Express.js behavior, where HEAD automatically handles GET routes.
express-zod-api/src/documentation.ts (6)
14-14: Import updates support HEAD method functionality.The updated imports for
getInputSources,ClientMethod, andAbstractEndpointprovide the necessary functionality for HEAD method documentation generation.Also applies to: 17-17, 33-34
105-109: Method parameter type correctly updated.Updating the
#ensureUniqOperationIdmethod to useClientMethodmaintains consistency with other parts of the framework.
159-163: Explicit callback typing improves clarity.The explicit parameter types for the
onEndpointcallback enhance type safety and make the HEAD method support clear.
178-178: Correct use of input sources helper.Using
getInputSources(method, config.inputSources)instead of direct config access ensures proper input source determination for different HTTP methods, including HEAD.
209-210: Correct HEAD response documentation.Setting
mimeTypestonullfor HEAD method positive responses correctly implements the HTTP specification where HEAD responses should not include a response body.
263-269: Proper HEAD endpoint documentation strategy.Duplicating GET endpoints as HEAD endpoints in the documentation aligns with the HTTP specification and ensures comprehensive API documentation.
example/example.client.ts (5)
42-67: HEAD method implementation looks correct.The HEAD endpoint types properly mirror the GET endpoint with
undefinedpositive response, which correctly follows HTTP HEAD specification where no response body should be sent.
451-451: Method union type correctly updated to include HEAD.The addition of "head" to the Method union type is necessary and properly implemented to support the new HEAD endpoints.
455-624: Interface updates for HEAD method support are comprehensive and consistent.All interfaces have been properly updated to include HEAD endpoints with correct type mappings. The HEAD endpoints correctly use
undefinedfor positive responses and maintain appropriate error response types.
649-649: HEAD method correctly treated as bodyless request.The update to include "head" in the bodyless methods array is correct. HEAD requests should be handled like GET requests - with parameters in the query string rather than request body.
229-248: Remaining HEAD endpoint definitions follow correct pattern.All remaining HEAD endpoint type definitions consistently follow the proper HTTP HEAD specification with matching input parameters to their GET counterparts and undefined positive responses.
Also applies to: 271-290, 383-403
RobinTail
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ready
Due to #2816 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Refactor** * Improved and unified type definitions for HTTP methods, enhancing consistency and clarity in method handling across the application. * Updated method handling logic to better support CORS and HTTP method distinctions. * **Tests** * Removed obsolete tests related to deprecated method types. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Based on #2791
Findings in Express:
app.head()handler, it's automatically set for allapp.get()ones:res.send()— it automatically sendsContent-Lengthheader instead of response body for requests havingHEADmethod:Content-Lengthshould not be set anyway because ofTransfer-encoding: chunked, according to RFC:HEADit SHOULD be there:Summary by CodeRabbit
New Features
Bug Fixes
Tests
Documentation