fix(mongoose): fix schema when default value is used on collection #2971
Conversation
WalkthroughThe pull request introduces modifications to the Mongoose ORM schema creation utilities and schema decorators. The changes primarily focus on enhancing the schema creation process, improving type handling, and addressing issues with default values for array properties. The modifications span across multiple files in the Changes
Assessment against linked issues
Possibly related PRs
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
packages/orm/mongoose/src/utils/createSchema.spec.ts(6 hunks)packages/orm/mongoose/src/utils/createSchema.ts(4 hunks)packages/specs/schema/src/decorators/common/default.ts(2 hunks)packages/specs/schema/src/domain/JsonSchema.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: test-specs (20.12.2)
- GitHub Check: test-orm (20.12.2)
🔇 Additional comments (15)
packages/orm/mongoose/src/utils/createSchema.ts (7)
1-1: Good import usage for new utilities
This import now includesclassOfandisClassObject, which are pivotal for handling enum types and class checks.
5-5: SchemaDefinitionProperty import
ImportingSchemaDefinitionPropertyallows a more precise return type when creating schema definitions, aligning with Mongoose's standard.
145-145: Signature change to return SchemaDefinitionProperty
Switching from a more genericSchemaTypeOptions<T>variant toSchemaDefinitionPropertystrengthens typing fidelity. Ensure all calling sites and external references are updated to match the new signature.
149-149: Required property logic
Using a function forrequiredis a clever approach to dynamically evaluate field requirement. This aligns well with Ts.ED’s advanced use cases.
174-174: Direct enum assignment
Populatingenum: jsonSchema["enum"]helps unify the schema definition for enumerated properties. This looks correct.
186-189: Array type definition
Returning an array structure throughtype: [schemaTypeOptions.type]is the proper Mongoose approach for nested arrays and subdocuments.
197-203: Map of subdocuments
DecomposingdefaultValueandrequiredensures clarity, whileof: otherOptscaptures subdocument configuration. Confirm that this default logic for Map aligns with intended usage (e.g., returning an empty Map if no default is specified).packages/orm/mongoose/src/utils/createSchema.spec.ts (6)
468-471: Validating array of subdocuments
The test confirms the schema is constructed with an array of subdocuments, verifying that each element references thechildrenSchema.
500-547: Testing default undefined for subdocument array
This comprehensive test ensures that having@Default(undefined)correctly results in an optional array. The logic verifying an array ofenum: [String]is also nicely covered.
585-589: Array of refs test
The test accurately verifies an array of references toChildren3. The final schema shape matches Mongoose’s standard usage.
679-681: Map of subdocuments test
Validates theMapstructure is generated correctly, with each map value referencingchildrenSchema. This aligns with the refactored logic increateSchemaTypeOptions.
776-776: Discriminator key test
Confirms the schema properly sets thediscriminatorKeyfrom@DiscriminatorKey(). This ensures correct usage of Mongoose’s discriminator feature.
789-789: Version key test
Verifies@VersionKey()is applied and recognized by the schema. Ensures that the version key aligns with user-defined property names.packages/specs/schema/src/decorators/common/default.ts (2)
1-2: Use typed import from 'json-schema'
ImportingJSONSchema6Typeas a type-only import is consistent with best practices and aligns with the updated default value requirements.
45-45: 🧹 Nitpick (assertive)Consider omitting the
defaultproperty when the value is undefined
Ifundefinedis passed as the default value, it may produce a property"default": undefinedin the final JSON Schema, which is not valid JSON. If the intention is to have "no default," you might avoid setting this property altogether.Below is a suggested approach:
export function Default(defaultValue: JSONSchema6Type | undefined) { return JsonEntityFn((store) => { - store.itemSchema.default(defaultValue); + if (defaultValue !== undefined) { + store.itemSchema.default(defaultValue); + } }); }Additionally, consider verifying occurrences of
@Default(...)calls to ensure no incompatible types (like functions) are being passed:✅ Verification successful
The review comment is correct - undefined defaults should be omitted
The implementation directly stores the undefined value in the schema using
Map.set(), which would result in invalid JSON when serialized. The suggested approach to skip setting the default property when the value is undefined is the correct solution, as it:
- Prevents invalid JSON Schema generation
- Better represents the semantic meaning of "no default value"
- Follows JSON Schema best practices
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash rg -A 5 '@Default'Length of output: 33577
Script:
#!/bin/bash # Check the Default decorator implementation and related tests rg -A 10 "export function Default" packages/specs/schema/src/decorators/common/default.ts rg -A 10 "describe.*Default" packages/specs/schema/src/decorators/common/default.spec.tsLength of output: 523
Script:
#!/bin/bash # Check JsonSchema store implementation and schema output tests rg -A 10 "class JsonSchema" packages/specs/schema/src/domain/JsonSchema.ts rg -B 5 -A 10 "default.*undefined" packages/specs/schema/src/domain/JsonSchema.tsLength of output: 1156
|
🎉 This PR is included in version 8.4.4 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
Closes: #2968
Information
Todos
Summary by CodeRabbit
Tests
Refactor
Documentation