Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis PR adds configuration-driven routing behavior to the framework. A new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
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. Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (9)
example/routing.ts (1)
22-23: Clarify the comment to reflect method-only routing.The comment states "methods are defined within the route," but with the new
patchsyntax, the method name is the route key itself (method-only routing), not a combined string like"patch /". Consider updating the comment to clarify this distinction, for example:- // syntax 2: methods are defined within the route + // syntax 2: method-only routing (requires methodLikeRouteBehavior config) patch: updateUserEndpoint, // demonstrates authenticationThis helps readers understand that
patchis interpreted as an HTTP method, not a path segment likeretrieveorremove.CHANGELOG.md (2)
7-12: Clarify which keys are recognized as “method-like”.Suggest listing supported lowercase HTTP methods (get, post, put, patch, delete, head, options) and noting case-sensitivity to avoid ambiguity.
21-31: Make it explicit that “create” is a nested path segment.To prevent confusion with HTTP methods, consider annotating the example or using a more obviously non-method key:
-+ "/v1/users": { -+ get: getUserEndpoint, -+ create: makeUserEndpoint -+ }, ++ "/v1/users": { ++ get: getUserEndpoint, // method ++ create: makeUserEndpoint // nested path segment ++ },example/generate-client.ts (1)
8-13: Guard serverUrl when listen is an object.config.http.listen can be number|string|ListenOptions. If it’s ListenOptions, template string will yield “[object Object]”. Optionally normalize:
- serverUrl: `http://localhost:${config.http!.listen}`, + serverUrl: `http://localhost:${ + typeof config.http?.listen === "object" + ? (config.http.listen as { port?: number|string }).port ?? 80 + : config.http?.listen + }`,express-zod-api/tests/integration.spec.ts (1)
24-24: Add a focused test for methodLikeRouteBehavior="path".Current tests pass config but don’t assert the “method-like key as path segment” mode. Add one snapshot where a key like “get” under a path object is forced to be treated as a nested segment via config.methodLikeRouteBehavior="path". This hardens the new behavior.
Also applies to: 29-47, 52-70, 75-89, 112-126, 138-163
express-zod-api/src/config-type.ts (1)
49-57: New config option looks good; document recognized methods.Consider specifying recognized lowercase method names (get, post, put, patch, delete, head, options) in the JSDoc for clarity. Ensure routing-walker applies a default of "method" when unset.
express-zod-api/src/routing.ts (1)
19-21: Update example to match “DependsOnMethod removed”.Replace the stale “dependsOnMethod” example with method-key routing:
- * @example { dependsOnMethod: { get: retrieveEndpoint, post: createEndpoint } } + * @example { users: { get: retrieveEndpoint, post: createEndpoint } } // methods as keys on an objectREADME.md (2)
308-314: Consider documenting method-only routing conditions.The method-based routing example clearly shows the new syntax. To help users avoid confusion, consider adding a note that method-only routing (where keys like
get,postare treated as methods rather than path segments) only applies when:
- The key is a valid HTTP method name
- The value is an
Endpoint(not nested routing)methodLikeRouteBehavioris set to"method"(which is the default)For nested routing under method keys, users should either use explicit syntax like
"get /path"or setmethodLikeRouteBehavior: "path".
1088-1092: Clarify whetherconfigparameter is required.The documentation shows that
configis now a parameter for theIntegrationconstructor, and the PR description mentions "Integration now requires aconfigparameter" as a breaking change. Consider making this explicit in the documentation by:
- Adding a comment indicating it's required, or
- Showing both the old usage (deprecated) and new usage with a migration note
This will help users understand the breaking change when upgrading.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
CHANGELOG.md(1 hunks)README.md(2 hunks)example/generate-client.ts(1 hunks)example/routing.ts(1 hunks)express-zod-api/src/config-type.ts(1 hunks)express-zod-api/src/documentation.ts(2 hunks)express-zod-api/src/integration.ts(4 hunks)express-zod-api/src/routing-walker.ts(5 hunks)express-zod-api/src/routing.ts(2 hunks)express-zod-api/tests/integration.spec.ts(5 hunks)express-zod-api/tests/routing.spec.ts(5 hunks)
🧰 Additional context used
🧠 Learnings (7)
📚 Learning: 2025-07-20T11:09:58.980Z
Learnt from: RobinTail
Repo: RobinTail/express-zod-api PR: 2833
File: express-zod-api/src/method.ts:3-3
Timestamp: 2025-07-20T11:09:58.980Z
Learning: In express-zod-api, the `SomeMethod` type (defined as `Lowercase<string>`) is intentionally broad to represent raw, unvalidated HTTP methods from requests. The narrower `Method` type is used after validation with `isMethod()`. This defensive programming pattern separates raw external input types from validated types, allowing graceful handling of unknown methods.
Applied to files:
express-zod-api/src/config-type.tsexpress-zod-api/src/routing-walker.ts
📚 Learning: 2025-05-28T18:58:10.064Z
Learnt from: RobinTail
Repo: RobinTail/express-zod-api PR: 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.
Applied to files:
express-zod-api/src/config-type.tsexpress-zod-api/src/integration.tsexpress-zod-api/tests/integration.spec.tsexpress-zod-api/src/routing.tsexpress-zod-api/tests/routing.spec.ts
📚 Learning: 2025-06-14T16:42:52.972Z
Learnt from: RobinTail
Repo: RobinTail/express-zod-api PR: 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.
Applied to files:
express-zod-api/src/config-type.tsexpress-zod-api/src/documentation.tsexpress-zod-api/src/routing.tsexpress-zod-api/tests/routing.spec.ts
📚 Learning: 2025-06-02T21:08:56.475Z
Learnt from: RobinTail
Repo: RobinTail/express-zod-api PR: 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.
Applied to files:
express-zod-api/tests/integration.spec.ts
📚 Learning: 2025-10-02T17:42:48.840Z
Learnt from: RobinTail
Repo: RobinTail/express-zod-api PR: 0
File: :0-0
Timestamp: 2025-10-02T17:42:48.840Z
Learning: In express-zod-api v25 (ESM-only), the `.example()` method error occurs when user code runs as CommonJS. Express Zod API patches Zod's ESM bundle with `.example()`, but CommonJS code requires a separate CJS bundle instance that lacks this patch. Users must run their code as ESM by: (1) setting `"type": "module"` in package.json, (2) using `.mts` or `.mjs` file extensions, or (3) using tools like `tsx` or `vite-node` that provide their own ESM-compatible compilation.
Applied to files:
express-zod-api/src/routing.ts
📚 Learning: 2025-05-27T20:03:34.213Z
Learnt from: RobinTail
Repo: RobinTail/express-zod-api PR: 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.
Applied to files:
express-zod-api/src/routing.ts
📚 Learning: 2025-05-27T19:35:57.357Z
Learnt from: RobinTail
Repo: RobinTail/express-zod-api PR: 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.
Applied to files:
express-zod-api/tests/routing.spec.ts
🔇 Additional comments (10)
express-zod-api/src/routing.ts (1)
81-82: Passing config into walkRouting — looks good.express-zod-api/src/documentation.ts (1)
48-54: Config plumbed into Documentation — verified.The required config and forwarding to walkRouting are correct. Verified that routing-walker properly defaults
methodLikeRouteBehaviorto"method"via destructuring default inrouting-walker.ts:41.express-zod-api/src/integration.ts (1)
28-31: Config now required in Integration — implementation verified complete.Verification confirms all Integration instantiations in production code and tests supply the required
configparameter. No call sites missing the property.express-zod-api/tests/routing.spec.ts (3)
34-94: LGTM! Test correctly validates path-based interpretation.The test properly validates that when
methodLikeRouteBehavior: "path"is set, method-like keys (e.g.,get,post) are treated as path segments rather than HTTP methods. The expectations correctly verify that thegetkey results in the path/v1/user/get.Note: This test uses the non-default behavior. The default (
methodLikeRouteBehavior: "method") is tested in subsequent tests where method keys route to the parent path without adding a segment.
112-155: LGTM! Test validates the new method-only routing feature.This test correctly validates the default behavior (
methodLikeRouteBehavior: "method") where method-like keys (get,post,put,patch) are interpreted as HTTP methods rather than path segments. The expectations confirm that all methods route to the parent path/v1/userinstead of creating nested paths like/v1/user/get.This is the core feature introduced by this PR and the test coverage is appropriate.
157-182: LGTM! Proper validation of method support.The test correctly validates that when using method-only routing, the framework verifies that the endpoint actually supports the assigned method. The intentional assignment to
post(line 169) of an endpoint that only supportsputandpatchproperly triggers an error, ensuring type safety at runtime.express-zod-api/src/routing-walker.ts (4)
22-27: LGTM! Type signature updated to support config-driven routing.The addition of
config: CommonConfigtoRoutingWalkerParamsenables the routing walker to accessmethodLikeRouteBehaviorconfiguration. This is a necessary change for the method-only routing feature.Note: This is part of the breaking change mentioned in the PR description, as all call sites of
walkRoutingmust now provide aconfigparameter.
40-56: LGTM! Core logic correctly implements method-only routing.The implementation properly handles both routing modes:
Method mode (default,
methodLikeRouteBehavior: "method"): When a method-like key (e.g.,get) is assigned to anAbstractEndpoint, the key is interpreted as the HTTP method and the segment is"/", resulting in routing to the parent path.Path mode (
methodLikeRouteBehavior: "path"): Method-like keys are treated as path segments, falling through todetachMethodwhich returns them as regular path components.The three-part condition on lines 49-50 ensures method-only routing only activates when:
- The key is a valid HTTP method
- The config enables method interpretation
- The value is an actual endpoint (not nested routing)
This prevents confusion when method-like keys are used with nested routing structures.
40-42: Verify: Default value is breaking change.The default
methodLikeRouteBehavior = "method"enables the new method-only routing behavior. This means that existing route definitions using method names as path segments (e.g.,{ get: { nested: endpoint } }intending/get/nested) will now be interpreted differently.Ensure this breaking change is:
- ✅ Documented in the PR description
- ✅ Mentioned in CHANGELOG/migration guide
- Communicated in release notes
Users upgrading will need to either:
- Set
methodLikeRouteBehavior: "path"to restore old behavior, or- Rename their routes to avoid method names
86-116: LGTM! Config properly propagated through routing traversal.The
walkRoutingfunction correctly threads theconfigparameter through both the initialprocessEntriescall (line 92) and the recursive call when expanding nested routing structures (line 113). This ensures thatmethodLikeRouteBehavioris consistently applied at all nesting levels.The function maintains all existing validation logic (duplicate checking, method support checking) while integrating the new config-driven behavior seamlessly.
RobinTail
force-pushed
the
feat-method-only
branch
from
b77e2f1 to
6825c71
Compare
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
<img width="283" height="353" alt="image" src="https://github.com/user-attachments/assets/5355c68c-e807-4b61-83a8-17e66a117f7d" /> [Lia Smith](https://en.wikipedia.org/wiki/Suicide_of_Lia_Smith) was a 21 years young transgender woman. She attended Middlebury College in Middlebury, Vermont, majoring in computer science and statistics. She was a member of the college's chess club, bridge club, LGBTQ+ club, and Japanese club. She was a diver on the Middlebury Panthers women's swimming and diving team. In February 2025 Middlebury College had removed Lia Smith diving profile from their website, following President Donald Trump's executive order, which attempts to ban transgender women and girls from competing in women's sports. On October 18, 2025 Lia Smith committed suicide. The tragedy of Lia Smith highlights that the push for exclusion of transgender women from public women's spaces and sports is not an issue of theoretical fairness, but one with devastating consequences for the dignity, safety, and lives of trans individuals. Smith, who was an openly transgender student athlete, described the immense difficulty and feeling of being unwelcome in locker rooms, emphasizing the lack of a clear, safe space for her. Her plea that transgender people are "not trying to get into women's spaces to be perverts... we're just being ourselves" underscores that inclusion is a matter of allowing trans women to exist and access necessary public facilities with basic dignity, rather than an invasion of privacy or a threat. Denying access to these spaces does not eliminate their need for them, but instead subjects them to alienation and heightened psychological distress. Furthermore, Lia's experience as a diver, where she was reportedly targeted by anti-trans websites and ultimately left her team citing difficulties, demonstrates the severe emotional and social toll of exclusion in sports. Critics of anti-trans policies explicitly linked her death by suicide to the hostile climate and constant public scrutiny and harassment directed at trans athletes. Allowing trans women to participate in women's sports and access women's facilities is therefore a vital component of protecting the mental health and physical safety of transgender people. When a society creates systems that reject a person’s identity and deny them opportunities for community, activity, and belonging—all essential parts of a functioning life—it creates an environment of intense hostility and vulnerability. Inclusion, in contrast, affirms the full humanity of trans women and is a necessary measure against the systemic discrimination that contributed to her anguish. -------------------------------- - #2938 - #3049 - #2934 - #3086 - #3087 - #3094 - ❔ #3102 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Config-driven routing behavior; Integration instances accept per-instance config. * Zod plugin adds CJS compatibility and new schema metadata helpers. * Release bumped to v26 (beta). * **Bug Fixes** * Restored Zod examples/metadata in CommonJS environments. * **Breaking Changes** * Handler/middleware parameter renamed: options → ctx. * Removed DependsOnMethod; use inline per-route objects. * EndpointsFactory.addOptions() renamed to addContext(). * **Docs** * README and CHANGELOG updated for v26 migration and examples. * **Tests** * Added CJS runtime tests and updated tests to use ctx. <sub>✏️ Tip: You can customize this high-level summary in your review settings.</sub> <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
1 participant
Due to #2938
This will set us free from the requirement to write slash to the keys when we imply method depending routing.
Breaking change:
Integrationrequiresconfigparameter.Summary by CodeRabbit
New Features
methodLikeRouteBehaviorconfiguration option to control interpretation of method-like routing keys ("method" or "path")hasHeadMethodoption for endpoint documentation configurationRefactor
configparameter during initialization