BREAKING CHANGE: allow null for optional fields in TypeScript

[vkarpov15]

Summary

Apply #12748, #12781 to Mongoose 8. At runtime, Mongoose lets you store null for optional fields. TypeScript types should reflect this.

Examples

[hasezoey]

i have some questions, mainly is adding | null now required for ?: properties which should already have a | undefined added by ??

[hasezoey]

why is null here but not undefined?

[vkarpov15]

@hasezoey "is adding | null now required for ?: properties which should already have a | undefined added by ??" <-- yes. ?: just adds | undefined, | null is separate.

[hasezoey]

?: just adds | undefined, | null is separate.

i know that they are separate and i know that the "more correct" way is |null|undefined, but those that do not want to deal with null and only undefined (at least in terms of types) is it still required to be added? like is there some kind of type incompatibility to not add |null?

[vkarpov15]

@hasezoey I don't understand "those that do not want to deal with null and only undefined (at least in terms of types) is it still required to be added? like is there some kind of type incompatibility to not add |null?". Can you please clarify with some examples?

[hasezoey]

I don't understand "those that do not want to deal with null and only undefined (at least in terms of types) is it still required to be added? like is there some kind of type incompatibility to not add |null?". Can you please clarify with some examples?

i mean is the following still possible (/ supported) without type errors?

interface SomeSchema {
  prop1?: string;
  prop2?: number;
}
// instead of now
interface SomeSchema {
  prop1?: string | null;
  prop2?: number | null;
}

which would trickle down to something like typegoose

class SomeSchema {
  @prop()
  public prop1?: string;

  @prop()
  public prop2?: number;
}
// instead of
class SomeSchema {
  @prop()
  public prop1?: string | null;

  @prop()
  public prop2?: number | null;
}

personally even if the more correct way would be string | undefined | null, i would go with string | undefined (unless null is explicitly needed) because that makes type debugging (like hover) a lot more cleaner, especially because i have common functions like isNullOrUndefined (which is deprecated in utils nodejs) which would take care of both at runtime, but would only need to deal with undefined for the "undefined" case, where essentially undefined and null are treated the same

TL;DR: prop?: string is a lot more cleaner overall than prop?: string | null (or the "expanded type" prop: string | undefined | null) and i am asking if the "cleaner" way is still supported or will it give type-errors?

[vkarpov15]

@hasezoey you shouldn't need to change prop?: string to prop?: string | null in raw doc definitions. The following script compiles fine even with these changes.

import mongoose from 'mongoose';

interface IUser {
  prop?: string;
}

const schema = new mongoose.Schema<IUser>({ prop: String });
const TestModel = mongoose.model<IUser>('Test', schema);

const doc = new TestModel();
const foo: string | undefined | null = doc.prop;

Can you please try this PR out with Typegoose and see if anything breaks?

[hasezoey]

Can you please try this PR out with Typegoose and see if anything breaks?

seems to run fine (both runtime and types), though i will need to test it a little more in depth (against the 8.0 branch in general)