-
-
Notifications
You must be signed in to change notification settings - Fork 4k
types(schema): avoid treating paths with default: null as required #15889
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
Conversation
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.
Pull request overview
This PR fixes TypeScript type inference for Mongoose schema paths with default: null. Previously, paths with default: null were incorrectly treated as required in TypeScript, while default: undefined correctly treated them as optional. The fix extends the type checking logic to treat both null and undefined defaults consistently as optional paths.
Key Changes:
- Renamed
IsPathDefaultUndefinedtoIsPathDefaultNullishto reflect expanded scope - Extended type checking to handle
default: nullanddefault: () => nullpatterns - Added comprehensive TypeScript test coverage for the new behavior
Reviewed changes
Copilot reviewed 1 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| types/inferschematype.d.ts | Renamed and expanded the IsPathDefaultNullish type utility to check for both null and undefined defaults (including function-based defaults), and updated all usage sites |
| test/types/schema.test.ts | Added TypeScript test case gh15878() validating that paths with default: null and default: () => null are correctly inferred as optional types |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
hasezoey
left a 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.
Aside from the benchmark, looks good to me
|
I was able to make some small improvements to reduce number of typescript instantiations, but not enough to get back down to 300k. Which is fine because the ts benchmark is failing on 8.x right now with 338k instantiations, and this PR knocks that down to 334k. Small improvement, and I increased the instantiations limit to make the benchmark pass. |
Fix #15878
Summary
Currently, defining a path as
{ type: String, default: undefined }will correctly treat the path as not required in TypeScript... but{ type: String, default: null }will treat the path as required, which is incorrect.Examples