feat: transform path segements to PascalCase in generateOperationId#214
Conversation
this introduces better handling for paths with dashes
matzehecht
changed the title
commit: |
There was a problem hiding this comment.
Pull request overview
This PR improves the operation ID generation for OpenAPI paths by transforming path segments to PascalCase, properly handling dashes and other non-word characters. Previously, paths like /hello-world would generate operation IDs like getHello-world, but now they correctly generate getHelloWorld.
Changes:
- Replaced the simple
capitalizefunction with a more sophisticatedtoPascalCasefunction that handles dashes, underscores, and other non-word characters - Added test coverage for paths with dashes
- Updated snapshots to reflect the new operation ID format
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/utils.ts | Replaced capitalize function with toPascalCase that properly handles dashes and non-word characters in path segments |
| src/tests/zodv3.test.ts | Added test case for operation ID generation with dashed paths |
| src/tests/snapshots/zodv3.test.ts.snap | Added snapshot for the new test showing the expected getSomePath operation ID |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const param = z | ||
| .object({ | ||
| id: z.coerce.number(), | ||
| }) | ||
| .openapi({ ref: "Param" }); | ||
|
|
||
| const app = new Hono().get( | ||
| "/some-path", | ||
| describeRoute({ | ||
| tags: ["test"], | ||
| summary: "Test route", | ||
| description: "This is a test route", | ||
| }), | ||
| validator("param", param), |
There was a problem hiding this comment.
The test defines a path parameter validator but the route path '/some-path' doesn't contain any path parameters. The param validator at lines 186-190 expects an 'id' path parameter, but there's no ':id' in the route path. This appears to be copy-pasted from the 'describeResponse' test above. Either remove the param validator or change the path to include a path parameter like '/:id/some-path' to properly test the dash handling in both static and dynamic path segments.
| it("operation id for path with dash", async () => { | ||
| const query = z.object({ | ||
| limit: z.coerce.number().default(10), | ||
| offset: z.coerce.number().default(0), | ||
| }); | ||
|
|
||
| const param = z | ||
| .object({ | ||
| id: z.coerce.number(), | ||
| }) | ||
| .openapi({ ref: "Param" }); | ||
|
|
||
| const app = new Hono().get( | ||
| "/some-path", | ||
| describeRoute({ | ||
| tags: ["test"], | ||
| summary: "Test route", | ||
| description: "This is a test route", | ||
| }), | ||
| validator("param", param), | ||
| validator("query", query), | ||
| describeResponse( | ||
| async (c) => { | ||
| c.req.valid("query"); | ||
| c.req.valid("param"); | ||
|
|
||
| return c.json({ | ||
| result: [], | ||
| count: 0, | ||
| }); | ||
| }, | ||
| { | ||
| 200: { | ||
| description: "Success", | ||
| content: { | ||
| "application/json": { | ||
| vSchema: z.object({ | ||
| result: z.array(z.string()), | ||
| count: z.number(), | ||
| }), | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| ), | ||
| ); | ||
|
|
||
| const specs = await generateSpecs(app); | ||
|
|
||
| expect(specs).toMatchSnapshot(); | ||
| }); |
There was a problem hiding this comment.
The test only covers a single path segment with a dash. Consider adding test cases for additional scenarios to ensure the toPascalCase function works correctly in all cases, such as: multiple segments with dashes (e.g., '/hello-world/foo-bar'), path parameters with dashes (e.g., '/:user-id'), and combinations of static and dynamic segments with dashes (e.g., '/hello-world/:user-id/foo-bar').
MathurAditya724
force-pushed
the
fix/operationid-pascalcase
branch
from
c14e40d to
a83140a
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const param = z | ||
| .object({ | ||
| id: z.coerce.number(), | ||
| }) | ||
| .openapi({ ref: "Param" }); | ||
|
|
||
| const app = new Hono().get( | ||
| "/some-path", | ||
| describeRoute({ | ||
| tags: ["test"], | ||
| summary: "Test route", | ||
| description: "This is a test route", | ||
| }), | ||
| validator("param", param), | ||
| validator("query", query), |
There was a problem hiding this comment.
The test defines a param validator for a path parameter, but the path /some-path contains no path parameters. This creates an inconsistent test case where the validator is declared but cannot be used. Either remove the param validator and the c.req.valid("param") call, or change the path to include a parameter like /some-path/:id to properly test the PascalCase conversion with both dashes and parameters.
| const toPascalCase = (text: string) => | ||
| text | ||
| .split(/[\W_]+/) | ||
| .filter(Boolean) | ||
| .map((word) => word.charAt(0).toUpperCase() + word.slice(1)) | ||
| .join(""); | ||
|
|
||
| const generateOperationId = (route: RouterRoute) => { |
There was a problem hiding this comment.
The toPascalCase function preserves non-word characters at the beginning of the string due to the (?<!^) negative lookbehind in the regex. This means a path segment like "-test" would be converted to "-Test" instead of "Test", resulting in an invalid operation ID. Consider handling leading non-word characters explicitly by trimming them or including them in the removal pattern.
Resolve merge conflicts in utils.ts and zodv3.test.ts.snap
MathurAditya724
force-pushed
the
fix/operationid-pascalcase
branch
from
ede7565 to
341b6df
Compare
2 participants
this introduces better handling for paths with dashes
paths like
/hello-world/foowould result in operationIds likegetHelloWorldFooinstead ofgetHello-worldFoo