From 68c0329f908a284e93afafede0bd42bd73c70efa Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Mon, 7 Nov 2022 16:07:55 -0500 Subject: [PATCH 1/4] WIP inferring timestamps from schema options, 1 test case still failing re: #12069 --- test/types/schema.test.ts | 18 +++++++---- types/index.d.ts | 11 +++---- types/inferschematype.d.ts | 61 ++++++++++++++++++++++---------------- types/models.d.ts | 12 ++++++-- types/schemaoptions.d.ts | 12 ++++++-- types/utility.d.ts | 2 ++ 6 files changed, 74 insertions(+), 42 deletions(-) diff --git a/test/types/schema.test.ts b/test/types/schema.test.ts index 1eddc97363d..e5644ac908f 100644 --- a/test/types/schema.test.ts +++ b/test/types/schema.test.ts @@ -625,7 +625,7 @@ function gh11997() { } function gh12003() { - const baseSchemaOptions: SchemaOptions = { + const baseSchemaOptions = { versionKey: false }; @@ -635,6 +635,8 @@ function gh12003() { type BaseSchemaType = InferSchemaType; + expectType<'type'>({} as ObtainSchemaGeneric['typeKey']); + expectType<{ name?: string }>({} as BaseSchemaType); } @@ -792,8 +794,7 @@ function gh12205() { type: new Types.ObjectId(), required: true } - }, - { timestamps: true } + } ); const Campaign = model('Campaign', campaignSchema); @@ -803,8 +804,6 @@ function gh12205() { type ICampaign = InferSchemaType; expectType<{ client: Types.ObjectId }>({} as ICampaign); - expectType<'type'>({} as ObtainSchemaGeneric); - type A = ObtainDocumentType<{ client: { type: Schema.Types.ObjectId, required: true } }>; expectType<{ client: Types.ObjectId }>({} as A); @@ -860,3 +859,12 @@ function gh12242() { type Example = InferSchemaType; expectType<0 | 1>({} as Example['active']); } + +function testInferTimestamps() { + const schema = new Schema({ + name: String + }, { timestamps: true }); + + type WithTimestamps = InferSchemaType; + expectType<{ name?: string; createdAt: Date, updatedAt: Date }>({} as WithTimestamps); +} \ No newline at end of file diff --git a/types/index.d.ts b/types/index.d.ts index 3c1e7c2563e..07f2d9715de 100644 --- a/types/index.d.ts +++ b/types/index.d.ts @@ -185,13 +185,14 @@ declare module 'mongoose' { TQueryHelpers = {}, TVirtuals = {}, TStaticMethods = {}, - TPathTypeKey extends TypeKeyBaseType = DefaultTypeKey, - DocType extends ObtainDocumentType = ObtainDocumentType> + TSchemaOptions extends ResolveSchemaOptions = DefaultSchemaOptions, + DocType extends ApplySchemaOptions, TSchemaOptions> = ApplySchemaOptions, TSchemaOptions>, + > extends events.EventEmitter { /** * Create a new schema */ - constructor(definition?: SchemaDefinition> | DocType, options?: SchemaOptions, TInstanceMethods, TQueryHelpers, TStaticMethods, TVirtuals>); + constructor(definition?: SchemaDefinition> | DocType, options?: SchemaOptions, TInstanceMethods, TQueryHelpers, TStaticMethods, TVirtuals> | TSchemaOptions); /** Adds key path / schema type pairs to this schema. */ add(obj: SchemaDefinition> | Schema, prefix?: string): this; @@ -550,10 +551,6 @@ declare module 'mongoose' { export type SchemaDefinitionType = T extends Document ? Omit> : T; - // Helpers to simplify checks - type IfAny = 0 extends (1 & IFTYPE) ? THENTYPE : ELSETYPE; - type IfUnknown = unknown extends IFTYPE ? THENTYPE : IFTYPE; - // tests for these two types are located in test/types/lean.test.ts export type DocTypeFromUnion = T extends (Document & infer U) ? [U] extends [Document & infer U] ? IfUnknown, false> : false : false; diff --git a/types/inferschematype.d.ts b/types/inferschematype.d.ts index a0406388f00..746dfb99b6c 100644 --- a/types/inferschematype.d.ts +++ b/types/inferschematype.d.ts @@ -12,7 +12,8 @@ import { ObtainDocumentType, DefaultTypeKey, ObjectIdSchemaDefinition, - IfEquals + IfEquals, + DefaultSchemaOptions } from 'mongoose'; declare module 'mongoose' { @@ -23,10 +24,10 @@ declare module 'mongoose' { * @param {EnforcedDocType} EnforcedDocType A generic type enforced by user "provided before schema constructor". * @param {TypeKey} TypeKey A generic of literal string type."Refers to the property used for path type definition". */ - type ObtainDocumentType = + type ObtainDocumentType = DefaultSchemaOptions> = IsItRecordAndNotAny extends true ? EnforcedDocType : { - [K in keyof (RequiredPaths & - OptionalPaths)]: ObtainDocumentPathType; + [K in keyof (RequiredPaths & + OptionalPaths)]: ObtainDocumentPathType; }; /** @@ -45,19 +46,27 @@ declare module 'mongoose' { * @param {TSchema} TSchema A generic of schema type instance. * @param {alias} alias Targeted generic alias. */ - type ObtainSchemaGeneric = - TSchema extends Schema - ? { - EnforcedDocType: EnforcedDocType; - M: M; - TInstanceMethods: TInstanceMethods; - TQueryHelpers: TQueryHelpers; - TVirtuals: TVirtuals; - TStaticMethods: TStaticMethods; - TPathTypeKey: TPathTypeKey; - DocType: DocType; - }[alias] - : unknown; + type ObtainSchemaGeneric = + TSchema extends Schema + ? { + EnforcedDocType: EnforcedDocType; + M: M; + TInstanceMethods: TInstanceMethods; + TQueryHelpers: TQueryHelpers; + TVirtuals: TVirtuals; + TStaticMethods: TStaticMethods; + TSchemaOptions: TSchemaOptions; + DocType: DocType; + }[alias] + : unknown; + + type ResolveSchemaOptions = Omit, 'statics' | 'methods' | 'query' | 'virtuals'>; + + type ApplySchemaOptions = ResolveTimestamps; + + type ResolveTimestamps = O extends { timestamps: true } + ? T & { createdAt: Date; updatedAt: Date; } + : T; } /** @@ -65,7 +74,7 @@ declare module 'mongoose' { * @param {P} P Document path. * @param {TypeKey} TypeKey A generic of literal string type."Refers to the property used for path type definition". */ -type IsPathRequired = +type IsPathRequired = P extends { required: true | [true, string | undefined] } | ArrayConstructor | any[] ? true : P extends (Record) @@ -83,7 +92,7 @@ type IsPathRequired = * @description It helps to check if a path is defined by TypeKey OR not. * @param {TypeKey} TypeKey A literal string refers to path type property key. */ -type PathWithTypePropertyBaseType = { [k in TypeKey]: any }; +type PathWithTypePropertyBaseType = { [k in TypeKey]: any }; /** * @summary A Utility to obtain schema's required path keys. @@ -91,7 +100,7 @@ type PathWithTypePropertyBaseType = { +type RequiredPathKeys = { [K in keyof T]: IsPathRequired extends true ? IfEquals : never; }[keyof T]; @@ -101,7 +110,7 @@ type RequiredPathKeys = { * @param {TypeKey} TypeKey A generic of literal string type."Refers to the property used for path type definition". * @returns a record contains required paths with the corresponding type. */ -type RequiredPaths = { +type RequiredPaths = { [K in RequiredPathKeys]: T[K]; }; @@ -111,7 +120,7 @@ type RequiredPaths = { * @param {TypeKey} TypeKey A generic of literal string type."Refers to the property used for path type definition". * @returns optional paths keys of document definition. */ -type OptionalPathKeys = { +type OptionalPathKeys = { [K in keyof T]: IsPathRequired extends true ? never : K; }[keyof T]; @@ -121,7 +130,7 @@ type OptionalPathKeys = { * @param {TypeKey} TypeKey A generic of literal string type."Refers to the property used for path type definition". * @returns a record contains optional paths with the corresponding type. */ -type OptionalPaths = { +type OptionalPaths = { [K in OptionalPathKeys]?: T[K]; }; @@ -131,7 +140,7 @@ type OptionalPaths = { * @param {PathValueType} PathValueType Document definition path type. * @param {TypeKey} TypeKey A generic refers to document definition. */ -type ObtainDocumentPathType = PathValueType extends Schema +type ObtainDocumentPathType = PathValueType extends Schema ? InferSchemaType : ResolvePathType< PathValueType extends PathWithTypePropertyBaseType ? PathValueType[TypeKey] : PathValueType, @@ -152,7 +161,7 @@ type PathEnumOrString['enum']> = T extends ( * @param {TypeKey} TypeKey A generic of literal string type."Refers to the property used for path type definition". * @returns Number, "Number" or "number" will be resolved to number type. */ -type ResolvePathType = {}, TypeKey extends TypeKeyBaseType = DefaultTypeKey> = +type ResolvePathType = {}, TypeKey extends string = DefaultSchemaOptions['typeKey']> = PathValueType extends Schema ? InferSchemaType : PathValueType extends (infer Item)[] ? IfEquals> : ObtainDocumentPathType[]> : PathValueType extends StringSchemaDefinition ? PathEnumOrString : @@ -177,5 +186,5 @@ type ResolvePathType extends true ? any: IfEquals extends true ? any: PathValueType extends typeof SchemaType ? PathValueType['prototype'] : - PathValueType extends Record ? ObtainDocumentType : + PathValueType extends Record ? ObtainDocumentType : unknown; diff --git a/types/models.d.ts b/types/models.d.ts index 1f4cd3cc2ce..93f3fd9139e 100644 --- a/types/models.d.ts +++ b/types/models.d.ts @@ -128,8 +128,16 @@ declare module 'mongoose' { AcceptsDiscriminator, IndexManager, SessionStarter { - new (doc?: DocType, fields?: any | null, options?: boolean | AnyObject): HydratedDocument, TVirtuals>> & ObtainSchemaGeneric; + new (doc?: DocType, fields?: any | null, options?: boolean | AnyObject): HydratedDocument< + T, + TMethodsAndOverrides, + IfEquals< + TVirtuals, + {}, + ObtainSchemaGeneric, + TVirtuals + > + > & ObtainSchemaGeneric; aggregate(pipeline?: PipelineStage[], options?: mongodb.AggregateOptions, callback?: Callback): Aggregate>; aggregate(pipeline: PipelineStage[], callback?: Callback): Aggregate>; diff --git a/types/schemaoptions.d.ts b/types/schemaoptions.d.ts index c3d35bd4272..71ccaa87aba 100644 --- a/types/schemaoptions.d.ts +++ b/types/schemaoptions.d.ts @@ -10,7 +10,7 @@ declare module 'mongoose' { type TypeKeyBaseType = string; type DefaultTypeKey = 'type'; - interface SchemaOptions { + interface SchemaOptions { /** * By default, Mongoose's init() function creates all the indexes defined in your model's schema by * calling Model.createIndexes() after you successfully connect to MongoDB. If you want to disable @@ -139,7 +139,7 @@ declare module 'mongoose' { * type declaration. However, for applications like geoJSON, the 'type' property is important. If you want to * control which key mongoose uses to find type declarations, set the 'typeKey' schema option. */ - typeKey?: PathTypeKey; + typeKey?: string; /** * By default, documents are automatically validated before they are saved to the database. This is to @@ -214,4 +214,12 @@ declare module 'mongoose' { */ virtuals?: SchemaOptionsVirtualsPropertyType, } + + interface DefaultSchemaOptions { + typeKey: 'type'; + id: true; + _id: true; + timestamps: false; + versionKey: '__v' + } } diff --git a/types/utility.d.ts b/types/utility.d.ts index bfc531aa8f2..69fe73a7be3 100644 --- a/types/utility.d.ts +++ b/types/utility.d.ts @@ -1,4 +1,6 @@ declare module 'mongoose' { + type IfAny = 0 extends (1 & IFTYPE) ? THENTYPE : ELSETYPE; + type IfUnknown = unknown extends IFTYPE ? THENTYPE : IFTYPE; type Unpacked = T extends (infer U)[] ? U : From 69a2b22f5165660895c4678f4ddf26fc9d57a100 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Wed, 9 Nov 2022 21:12:07 -0500 Subject: [PATCH 2/4] fix(document): handle setting array to itself after saving and pushing a new value Fix #12069 --- lib/document.js | 34 +++++++++++++++++++++++++--------- test/document.test.js | 23 ++++++++++++++++++++++- test/types/schema.test.ts | 2 +- 3 files changed, 48 insertions(+), 11 deletions(-) diff --git a/lib/document.js b/lib/document.js index 00c50cec613..76ba04cb460 100644 --- a/lib/document.js +++ b/lib/document.js @@ -1468,13 +1468,7 @@ Document.prototype.$set = function $set(path, val, type, options) { const doc = this.$isSubdocument ? this.ownerDocument() : this; savedState = doc.$__.savedState; savedStatePath = this.$isSubdocument ? this.$__.fullPath + '.' + path : path; - if (savedState != null) { - const firstDot = savedStatePath.indexOf('.'); - const topLevelPath = firstDot === -1 ? savedStatePath : savedStatePath.slice(0, firstDot); - if (!savedState.hasOwnProperty(topLevelPath)) { - savedState[topLevelPath] = utils.clone(doc.$__getValue(topLevelPath)); - } - } + doc.$__saveInitialState(savedStatePath); } this.$__set(pathToMark, path, options, constructing, parts, schema, val, priorVal); @@ -1583,6 +1577,10 @@ Document.prototype.$__shouldModify = function(pathToMark, path, options, constru if (this.$isNew) { return true; } + // Is path already modified? If so, always modify. We may unmark modified later. + if (path in this.$__.activePaths.getStatePaths('modify')) { + return true; + } // Re: the note about gh-7196, `val` is the raw value without casting or // setters if the full path is under a single nested subdoc because we don't @@ -1780,11 +1778,10 @@ Document.prototype.$inc = function $inc(path, val) { const currentValue = this.$__getValue(path) || 0; - this.$__setValue(path, currentValue + val); - this.$__.primitiveAtomics = this.$__.primitiveAtomics || {}; this.$__.primitiveAtomics[path] = { $inc: val }; this.markModified(path); + this.$__setValue(path, currentValue + val); return this; }; @@ -1927,6 +1924,8 @@ Document.prototype.$__path = function(path) { */ Document.prototype.markModified = function(path, scope) { + this.$__saveInitialState(path); + this.$__.activePaths.modify(path); if (scope != null && !this.$isSubdocument) { this.$__.pathsToScopes = this.$__pathsToScopes || {}; @@ -1934,6 +1933,22 @@ Document.prototype.markModified = function(path, scope) { } }; +/*! + * ignore + */ + +Document.prototype.$__saveInitialState = function $__saveInitialState(path) { + const savedState = this.$__.savedState; + const savedStatePath = path; + if (savedState != null) { + const firstDot = savedStatePath.indexOf('.'); + const topLevelPath = firstDot === -1 ? savedStatePath : savedStatePath.slice(0, firstDot); + if (!savedState.hasOwnProperty(topLevelPath)) { + savedState[topLevelPath] = utils.clone(this.$__getValue(topLevelPath)); + } + } +}; + /** * Clears the modified state on the specified path. * @@ -3379,6 +3394,7 @@ Document.prototype.$__dirty = function() { schema: _this.$__path(path) }; }); + // gh-2558: if we had to set a default and the value is not undefined, // we have to save as well all = all.concat(this.$__.activePaths.map('default', function(path) { diff --git a/test/document.test.js b/test/document.test.js index b0f4ea132c3..d20a589b48e 100644 --- a/test/document.test.js +++ b/test/document.test.js @@ -9191,7 +9191,6 @@ describe('document', function() { const Test = db.model('Test', schema); - const foo = new Test({ bar: 'bar' }); await foo.save(); assert.ok(!foo.isModified('bar')); @@ -12006,6 +12005,28 @@ describe('document', function() { title: 'The power of JavaScript' }); }); + + it('handles setting array to itself after saving and pushing a new value (gh-12656)', async function() { + const Test = db.model('Test', new Schema({ + list: [{ + a: Number + }] + })); + await Test.create({ list: [{ a: 1, b: 11 }] }); + + let doc = await Test.findOne(); + doc.list.push({ a: 2 }); + doc.list = [...doc.list]; // Same behavior if line replaced with: doc.list = [...doc.list] + await doc.save(); + + doc.list.push({ a: 3 }); + doc.list = [...doc.list]; // Same behavior if line replaced with: doc.list = [...doc.list] + await doc.save(); + + doc = await Test.findOne(); + assert.equal(doc.list.length, 3); + assert.deepStrictEqual(doc.list.map(el => el.a), [1, 2, 3]); + }); }); describe('Check if instance function that is supplied in schema option is availabe', function() { diff --git a/test/types/schema.test.ts b/test/types/schema.test.ts index e5644ac908f..01c8ef539c4 100644 --- a/test/types/schema.test.ts +++ b/test/types/schema.test.ts @@ -867,4 +867,4 @@ function testInferTimestamps() { type WithTimestamps = InferSchemaType; expectType<{ name?: string; createdAt: Date, updatedAt: Date }>({} as WithTimestamps); -} \ No newline at end of file +} From cc801039827d23f2fed97fb57831088f16333772 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Sun, 27 Nov 2022 13:57:44 -0500 Subject: [PATCH 3/4] wip fixing merge conflicts re: #12069 --- types/inferschematype.d.ts | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/types/inferschematype.d.ts b/types/inferschematype.d.ts index ddd92c5ff67..cf5d601a27f 100644 --- a/types/inferschematype.d.ts +++ b/types/inferschematype.d.ts @@ -65,7 +65,7 @@ declare module 'mongoose' { type ApplySchemaOptions = ResolveTimestamps; type ResolveTimestamps = O extends { timestamps: true } - ? T & { createdAt: Date; updatedAt: Date; } + ? { createdAt: Date; updatedAt: Date; } & T : T; } @@ -187,11 +187,13 @@ type ResolvePathType extends true ? Types.Decimal128 : IfEquals extends true ? Types.Decimal128 : - PathValueType extends MapConstructor ? Map> : - PathValueType extends ArrayConstructor ? any[] : - PathValueType extends typeof Schema.Types.Mixed ? any: - IfEquals extends true ? any: - IfEquals extends true ? any: - PathValueType extends typeof SchemaType ? PathValueType['prototype'] : - PathValueType extends Record ? ObtainDocumentType : - unknown; + PathValueType extends 'uuid' | 'UUID' | typeof Schema.Types.UUID ? Buffer : + IfEquals extends true ? Buffer : + PathValueType extends MapConstructor ? Map> : + PathValueType extends ArrayConstructor ? any[] : + PathValueType extends typeof Schema.Types.Mixed ? any: + IfEquals extends true ? any: + IfEquals extends true ? any: + PathValueType extends typeof SchemaType ? PathValueType['prototype'] : + PathValueType extends Record ? ObtainDocumentType : + unknown; From accb7e53fa3c09be1a491bb476cfdae92f9a4016 Mon Sep 17 00:00:00 2001 From: Valeri Karpov Date: Sun, 27 Nov 2022 14:29:34 -0500 Subject: [PATCH 4/4] fix(types): fix tests for #12069 --- test/types/schema.test.ts | 6 +++++- types/index.d.ts | 2 +- types/inferschematype.d.ts | 4 ++-- types/models.d.ts | 16 ++++++++-------- 4 files changed, 16 insertions(+), 12 deletions(-) diff --git a/test/types/schema.test.ts b/test/types/schema.test.ts index e95b07b425e..e4bf39b313c 100644 --- a/test/types/schema.test.ts +++ b/test/types/schema.test.ts @@ -868,7 +868,11 @@ function testInferTimestamps() { }, { timestamps: true }); type WithTimestamps = InferSchemaType; - expectType<{ name?: string; createdAt: Date, updatedAt: Date }>({} as WithTimestamps); + // For some reason, expectType<{ createdAt: Date, updatedAt: Date, name?: string }> throws + // an error "Parameter type { createdAt: Date; updatedAt: Date; name?: string | undefined; } + // is not identical to argument type { createdAt: NativeDate; updatedAt: NativeDate; } & + // { name?: string | undefined; }" + expectType<{ createdAt: Date, updatedAt: Date } & { name?: string }>({} as WithTimestamps); } function gh12431() { diff --git a/types/index.d.ts b/types/index.d.ts index 51aad535e29..1ee7f833fb2 100644 --- a/types/index.d.ts +++ b/types/index.d.ts @@ -187,7 +187,7 @@ declare module 'mongoose' { TStaticMethods = {}, TSchemaOptions extends ResolveSchemaOptions = DefaultSchemaOptions, DocType extends ApplySchemaOptions, TSchemaOptions> = ApplySchemaOptions, TSchemaOptions>, - > + > extends events.EventEmitter { /** * Create a new schema diff --git a/types/inferschematype.d.ts b/types/inferschematype.d.ts index cf5d601a27f..e8aff0539fc 100644 --- a/types/inferschematype.d.ts +++ b/types/inferschematype.d.ts @@ -65,7 +65,7 @@ declare module 'mongoose' { type ApplySchemaOptions = ResolveTimestamps; type ResolveTimestamps = O extends { timestamps: true } - ? { createdAt: Date; updatedAt: Date; } & T + ? { createdAt: NativeDate; updatedAt: NativeDate; } & T : T; } @@ -195,5 +195,5 @@ type ResolvePathType extends true ? any: IfEquals extends true ? any: PathValueType extends typeof SchemaType ? PathValueType['prototype'] : - PathValueType extends Record ? ObtainDocumentType : + PathValueType extends Record ? ObtainDocumentType : unknown; diff --git a/types/models.d.ts b/types/models.d.ts index 9f6d9ae39d8..744088d1968 100644 --- a/types/models.d.ts +++ b/types/models.d.ts @@ -130,14 +130,14 @@ declare module 'mongoose' { IndexManager, SessionStarter { new (doc?: DocType, fields?: any | null, options?: boolean | AnyObject): HydratedDocument< - T, - TMethodsAndOverrides, - IfEquals< - TVirtuals, - {}, - ObtainSchemaGeneric, - TVirtuals - > + T, + TMethodsAndOverrides, + IfEquals< + TVirtuals, + {}, + ObtainSchemaGeneric, + TVirtuals + > > & ObtainSchemaGeneric; aggregate(pipeline?: PipelineStage[], options?: mongodb.AggregateOptions, callback?: Callback): Aggregate>;