Improving methods inheritance #2833
Merged
Conversation
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
Contributor
|
Warning Rate limit exceeded@RobinTail has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 5 minutes and 28 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughThe changes refactor HTTP method type definitions and usages throughout the codebase. The Changes
Poem
✨ 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
express-zod-api/src/common-helpers.ts(2 hunks)express-zod-api/src/endpoint.ts(2 hunks)express-zod-api/src/method.ts(1 hunks)express-zod-api/src/routing.ts(3 hunks)express-zod-api/tests/method.spec.ts(0 hunks)
💤 Files with no reviewable changes (1)
- express-zod-api/tests/method.spec.ts
🧰 Additional context used
🧠 Learnings (5)
📓 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/endpoint.ts (6)
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/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#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#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.
express-zod-api/src/common-helpers.ts (11)
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/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#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/src/json-schema-helpers.ts:1-3
Timestamp: 2025-05-27T20:27:17.015Z
Learning: The JSONSchema type is not exported from the main "zod" module and must be imported from "zod/v4/core" when using Zod v4. This is acceptable for type-only imports as they don't create runtime dependencies.
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: The JSONSchema type is not exported from the main "zod" module and must be imported from "zod/v4/core" when using Zod v4. This is acceptable for type-only imports as they don't create runtime dependencies.
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/src/io-schema.ts:5-8
Timestamp: 2025-05-28T05:04:40.327Z
Learning: The Base type `object & { [Symbol.iterator]?: never }` in express-zod-api correctly excludes arrays and other iterables from IOSchema. Arrays are properly rejected because they have a defined Symbol.iterator method which conflicts with the never constraint, as demonstrated by TypeScript compiler errors when attempting to assign z.array() to IOSchema.
Learnt from: RobinTail
PR: RobinTail/express-zod-api#2546
File: express-zod-api/src/metadata.ts:5-13
Timestamp: 2025-05-27T20:08:50.699Z
Learning: The `_zod` property in Zod v4 schemas is officially documented and recommended for library authors to differentiate between Zod 3 and Zod 4 schemas at runtime, despite the underscore prefix. This is explicitly mentioned in Zod's library authors documentation and is not a private internal property.
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.
express-zod-api/src/routing.ts (5)
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/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.
express-zod-api/src/method.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.
🧬 Code Graph Analysis (3)
express-zod-api/src/endpoint.ts (1)
express-zod-api/src/method.ts (1)
SomeMethod(3-3)
express-zod-api/src/routing.ts (2)
express-zod-api/src/method.ts (3)
methods(10-16)CORSMethod(43-43)isMethod(45-46)express-zod-api/src/endpoint.ts (1)
methods(143-145)
express-zod-api/src/method.ts (2)
express-zod-api/src/endpoint.ts (1)
methods(143-145)example/example.client.ts (1)
Method(451-451)
🔇 Additional comments (6)
express-zod-api/src/endpoint.ts (1)
26-26: LGTM! Type system improvement aligns with the broader refactor.The replacement of
AuxMethodwithSomeMethodand simplification from the union typeMethod | AuxMethodto the single typeSomeMethodimproves type consistency across the codebase.Also applies to: 216-216
express-zod-api/src/common-helpers.ts (2)
7-7: LGTM! Import updates and naming improvement.The import changes align with the new method type system, and renaming
fallbackInputSourcetofallbackInputSourcesimproves naming consistency with its plural nature.Also applies to: 46-46
49-49: Excellent enhancement to method handling logic.The type changes from
Method | AuxMethodtoSomeMethodsimplify the type system, and the enhanced logic properly:
- Maps "head" method to "get" with correct typing
- Uses the
isMethodtype guard for validation- Provides appropriate fallback behavior for unrecognized methods
This is a well-structured improvement over the previous union type approach.
Also applies to: 52-65
express-zod-api/src/routing.ts (1)
9-9: LGTM! Consistent type system unification.The replacement of
Method | AuxMethodwithCORSMethodacross all function signatures provides a more semantically accurate and unified type system for CORS-related operations. The changes maintain existing logic while improving type clarity.Also applies to: 27-27, 35-35, 45-45, 51-51
express-zod-api/src/method.ts (2)
5-8: Excellent type hierarchy design.The
FamiliarMethodtype properly excludes non-HTTP method properties fromIRouter, and the separation betweenmethodsandclientMethodsarrays clearly distinguishes server-side supported methods from client-side usable methods (including "head").Also applies to: 16-21
37-43: Well-designedCORSMethodtype with comprehensive documentation.The
CORSMethodtype appropriately combines client methods with "options" for CORS operations, and the JSDoc documentation clearly explains its purpose and usage context.
RobinTail
commented
Jul 20, 2025
RobinTail
commented
Jul 20, 2025
RobinTail
commented
Jul 20, 2025
RobinTail
commented
Jul 20, 2025
This was referenced Jul 25, 2025
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.
Due to #2816
Summary by CodeRabbit
Refactor
Tests