-
-
Couldn't load subscription status.
- Fork 370
feat: add t.NumericEnum() for numeric enum query handling
#1503
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
base: main
Are you sure you want to change the base?
Conversation
WalkthroughIntroduces NumericEnum support to the Elysia type system, enabling validation and type-safe handling of numeric enums. Adds a new NumericEnum method to ElysiaType with corresponding public API exposure, defines AssertNumericEnum type constraints, and includes test coverage for transform detection and query validation. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes The changes introduce a focused, self-contained feature with straightforward type system additions and corresponding test cases. The implementation follows established patterns, involves no complex logic density, and the homogeneous nature of changes (type definition + method addition + matching tests) requires minimal review reasoning per file. Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
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 (2)
src/type-system/types.ts (1)
242-250: Consider adding documentation for the AssertNumericEnum type.The
AssertNumericEnumtype uses complex conditional mapping to validate numeric enum structures. Adding a JSDoc comment explaining its purpose and how it handles TypeScript numeric enum forward/reverse mappings would improve maintainability.+/** + * Type constraint for numeric enums that validates the enum structure. + * TypeScript numeric enums generate both forward mappings (name -> value) + * and reverse mappings (value -> name) at runtime. + * + * @example + * enum Gender { MALE = 1, FEMALE = 2 } + * // Runtime: { MALE: 1, FEMALE: 2, 1: "MALE", 2: "FEMALE" } + */ export type AssertNumericEnum<T extends Record<string, string | number>> = {test/validator/query.test.ts (1)
145-167: Consider adding test cases for error scenarios.The test correctly validates the happy path for numeric enum parsing. Consider adding test cases for:
- Invalid enum values (e.g.,
gender=999)- Non-numeric strings (e.g.,
gender=invalid)- Missing enum values
This would ensure comprehensive coverage of NumericEnum validation behavior.
Example additional test:
it('reject invalid numeric enum', async () => { enum Gender { MALE = 1, FEMALE = 2, UNKNOWN = 3 } const app = new Elysia().get('/', ({ query }) => query, { query: t.Object({ name: t.String(), gender: t.NumericEnum(Gender) }) }) const res = await app.handle(req(`/?name=sucrose&gender=999`)) expect(res.status).toBe(422) })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/type-system/index.ts(4 hunks)src/type-system/types.ts(1 hunks)test/aot/has-transform.test.ts(1 hunks)test/validator/query.test.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
test/aot/has-transform.test.ts (1)
src/schema.ts (3)
hasTransform(292-338)hasTransform(1105-1109)hasTransform(1382-1386)
test/validator/query.test.ts (1)
test/utils.ts (1)
req(1-2)
src/type-system/index.ts (2)
src/type-system/types.ts (1)
AssertNumericEnum(242-250)src/type-system/utils.ts (1)
compile(38-73)
🔇 Additional comments (5)
test/aot/has-transform.test.ts (1)
101-108: LGTM!The test correctly verifies that
hasTransformdetects the NumericEnum schema as having a transform, following the established pattern of other transform detection tests.src/type-system/index.ts (4)
23-25: LGTM!The
TEnumimport is correctly added to support the return type of the NumericEnum function.
44-46: LGTM!The
AssertNumericEnumimport is correctly added and used as a type constraint in the NumericEnum function.
156-171: Verify the stricter error handling is intentional.The
NumericEnumdecode function always throws an error when the value is NaN or invalid, whereas theNumericfunction (lines 127-154) returns the original value if NaN:// Numeric (line 145) if (isNaN(number)) return value // NumericEnum (line 166) if (isNaN(number)) throw compiler.Error(number)This stricter validation is likely appropriate for enum values (which must be valid enum members), but it differs from the
Numericpattern. Confirm this is the intended behavior.
629-629: LGTM!The public API export is correctly added, following the established pattern for other type system utilities.
The existing t.Enum() has limitations when handling numeric enums in query parameters: since query values are inherently strings, direct use with numeric enums causes type mismatches (string vs. number) during validation.
But using t.Numeric() loses enum-specific type hints.
This PR adds t.NumericEnum() , transform 'string' to 'number' and validate, as
t.Numeric()does.Summary by CodeRabbit
New Features
Tests