Skip to content

Conversation

@AkaHarshit
Copy link
Contributor

Summary

This PR adds the enumValues property to SchemaNumber.prototype.enum() to match the behavior of SchemaString.prototype.enum(). This fixes an API inconsistency where users could access schema.path('field').enumValues for String enums but not for Number enums.

Problem → Root Cause → Solution

Problem:
SchemaNumber.prototype.enum() does not expose an enumValues property on the schema type instance, while SchemaString.prototype.enum() does. This inconsistency prevents users from accessing enum values for Number schema types.

Root Cause:
In /lib/schema/number.js:

  1. The SchemaNumber constructor doesn't initialize this.enumValues = [] (String enum does this on line 29 of string.js).
  2. The enum() method stores enum values only in the validator object, not as a persistent property on the schema type instance.
  3. The method doesn't handle clearing enum when called with false/undefined, unlike String enum.

Solution:

  1. Initialize this.enumValues = [] in the SchemaNumber constructor.
  2. Update enum() to maintain this.enumValues by casting and pushing values (matching String enum behavior).
  3. Use this.enumValues in the validator closure instead of the local values variable.
  4. Add support for clearing enum when called with false/undefined (matching String enum).

Before vs After Behavior

Before:

const schema = new Schema({ status: { type: Number, enum: [1, 2, 3] } });
console.log(schema.path('status').enumValues); // undefined ❌

After:

const schema = new Schema({ status: { type: Number, enum: [1, 2, 3] } });
console.log(schema.path('status').enumValues); // [1, 2, 3] ✅

// Additive behavior (matches String enum)
schema.path('status').enum(4, 5);
console.log(schema.path('status').enumValues); // [1, 2, 3, 4, 5] ✅

Test Cases Added

Added comprehensive test in /test/schema.validation.test.js:

  • it('number enum', async function() { ... })

The test verifies:

  1. enumValues property exists and is accessible
  2. ✅ Initial enum values are correctly stored
  3. ✅ Additive behavior when calling enum() multiple times
  4. ✅ Support for object syntax with custom messages
  5. ✅ Validation works correctly (rejects invalid values, accepts valid ones)
  6. ✅ Null and undefined values are handled correctly

All tests pass:

✔ number enum (56ms)
✔ string enum (55ms)  # Existing test still passes

Why This Should Be Merged

  1. Fixes API inconsistency - Makes Number enum behavior consistent with String enum
  2. Non-breaking change - Adds functionality without changing existing behavior
  3. Small, focused change - Only touches the Number enum implementation
  4. Well-tested - Includes comprehensive test coverage
  5. Follows existing patterns - Matches String enum implementation exactly
  6. Improves developer experience - Consistent API across schema types

Files Changed

  1. /lib/schema/number.js - Added enumValues initialization and maintenance
  2. /test/schema.validation.test.js - Added comprehensive test for Number enum enumValues property

Reviewer Checklist

  • Code follows Mongoose's coding style (2 spaces, no trailing whitespace)
  • Tests added and passing
  • No breaking changes
  • Matches existing patterns (String enum implementation)
  • Handles edge cases (null, undefined, multiple enum calls)
  • Backwards compatible
  • Clear commit message following conventional commits

…ith String enum

This change makes Number enum behavior consistent with String enum by:
- Initializing enumValues array in SchemaNumber constructor
- Maintaining enumValues property when enum() is called
- Supporting clearing enum when called with false/undefined
- Using enumValues in validator closure for consistency

Fixes inconsistency where schema.path('field').enumValues was available
for String enums but not for Number enums.

Includes comprehensive test coverage matching String enum test patterns.
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds the enumValues property to SchemaNumber.prototype.enum() to achieve API consistency with SchemaString.prototype.enum(). The change allows developers to access enum values programmatically via schema.path('field').enumValues for Number schema types, which was previously only available for String schema types.

Key Changes

  • Added enumValues array initialization in the SchemaNumber constructor
  • Modified the enum() method to populate and maintain this.enumValues with cast values
  • Added support for clearing enum validators when called with false or undefined

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
lib/schema/number.js Added enumValues initialization in constructor and modified enum() method to maintain the enumValues property, matching String enum implementation
test/schema.validation.test.js Added comprehensive test suite for Number enum enumValues property, verifying initialization, additive behavior, object syntax, and validation

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Collaborator

@vkarpov15 vkarpov15 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks 👍

@vkarpov15 vkarpov15 added this to the 9.0.1 milestone Dec 1, 2025
@vkarpov15 vkarpov15 merged commit 32366cc into Automattic:master Dec 1, 2025
38 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants