Rev: preserving depictEnum and depictLiteral fallbacks for backward compatibility
#2699
Conversation
WalkthroughThis update introduces two new depicter functions, Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant DocumentationGenerator
participant Depicters
User->>DocumentationGenerator: Request schema depiction
DocumentationGenerator->>Depicters: Select depicter for enum/literal
Depicters->>DocumentationGenerator: Use depictEnum or depictLiteral
DocumentationGenerator->>User: Return JSON schema with correct type
Possibly related PRs
Poem
✨ Finishing Touches
🪧 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}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
CHANGELOG.md (1)
8-9: Consider standardizing bullet phrasing.To align with other entries, you could revise bullets to use action verbs, e.g.:
- “Add Enum
typesupport in Zod v3.25.45”- “Add Literal
typesupport in Zod v3.25.49”express-zod-api/tests/documentation-helpers.spec.ts (1)
306-315: Consider testing with different data types.The test correctly verifies that
depictEnumsets thetypeproperty based on the enum values. However, it only tests with string enum values. According to the implementation that usestypeof jsonSchema.enum?.[0], the function should work with various data types.Consider adding test cases for different data types:
describe("depictEnum()", () => { - test("should set type", () => { + test.each([ + { enum: ["test", "jest"], expectedType: "string" }, + { enum: [1, 2, 3], expectedType: "number" }, + { enum: [true, false], expectedType: "boolean" }, + ])("should set type based on enum values %#", ({ enum: enumValues }) => { expect( depictEnum( - { zodSchema: z.never(), jsonSchema: { enum: ["test", "jest"] } }, + { zodSchema: z.never(), jsonSchema: { enum: enumValues } }, requestCtx, ), ).toMatchSnapshot(); }); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
express-zod-api/tests/__snapshots__/documentation-helpers.spec.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (3)
CHANGELOG.md(1 hunks)express-zod-api/src/documentation-helpers.ts(2 hunks)express-zod-api/tests/documentation-helpers.spec.ts(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
express-zod-api/tests/documentation-helpers.spec.ts (1)
express-zod-api/src/documentation-helpers.ts (2)
depictEnum(150-153)depictLiteral(159-162)
🔇 Additional comments (4)
express-zod-api/src/documentation-helpers.ts (1)
398-400: Restore backward-compatibility mapping for enum and literal depicters
Re-registeringdepictLiteralanddepictEnumin thedepictersmap ensures that legacy schemas still receive a fallback when Zod’s native depiction is unavailable.CHANGELOG.md (1)
7-9: Changelog entry clarification aligns with new behavior.The updated wording clearly describes prioritizing native Zod depictions via the new
depictEnumanddepictLiteralfunctions, improving backward-compatibility clarity.express-zod-api/tests/documentation-helpers.spec.ts (2)
28-29: LGTM: Import statements are correctly added.The new depicter functions
depictEnumanddepictLiteralare properly imported and will be available for testing.
317-326: Well-structured parameterized test with good coverage.The test correctly verifies that
depictLiteralsets thetypeproperty from bothconstandenumproperties. The parameterized approach effectively tests both code paths in the implementation.However, similar to the
depictEnumtest, consider adding test cases with different data types to ensure thetypeofoperation works correctly across various JavaScript types (numbers, booleans, etc.).
Partially reverts #2697 and #2698
Summary by CodeRabbit
Documentation
New Features
Tests