feat(core): infer property type from default value (#4150)

Type inference with reflect-metadata fails when a runtime value is used without an explicit type annotation, e.g. `created = new Date` instead of `created: Data = new Date`. This PR infers the default value from the property initializer (if possible) and uses its type in this automatically, so `created = new Date` will work fine. Closes #4060
mikro-orm · Apr 25, 2023 · b989574 · b989574
1 parent 6e5a2d5
commit b989574
Show file tree

Hide file tree

Showing 30 changed files with 195 additions and 175 deletions.
diff --git a/packages/core/src/EntityManager.ts b/packages/core/src/EntityManager.ts
@@ -1110,7 +1110,7 @@ export class EntityManager<D extends IDatabaseDriver = IDatabaseDriver> {
     Object.keys(data).forEach(k => {
       const prop = meta.properties[k];
 
-      if (prop && prop.kind === ReferenceKind.SCALAR && SCALAR_TYPES.includes(prop.type) && (prop.setter || !prop.getter)) {
+      if (prop && prop.kind === ReferenceKind.SCALAR && SCALAR_TYPES.includes(prop.type.toLowerCase()) && (prop.setter || !prop.getter)) {
         data[k] = this.validator.validateProperty(prop, data[k], data);
       }
     });

diff --git a/packages/core/src/entity/ArrayCollection.ts b/packages/core/src/entity/ArrayCollection.ts
@@ -222,7 +222,7 @@ export class ArrayCollection<T extends object, O extends object> {
 
       /* istanbul ignore if */
       if (!meta) {
-        throw MetadataError.fromUnknownEntity((this.owner as object).constructor.name, 'Collection.property getter, maybe you just forgot to initialize the ORM?');
+        throw MetadataError.fromUnknownEntity(this.owner.constructor.name, 'Collection.property getter, maybe you just forgot to initialize the ORM?');
       }
 
       const field = Utils.keys(meta.properties).find(k => this.owner[k] === this);
@@ -307,7 +307,7 @@ export class ArrayCollection<T extends object, O extends object> {
     const hidden = ['items', 'owner', '_property', '_count', 'snapshot', '_populated', '_lazyInitialized'];
     hidden.forEach(k => delete object[k]);
     const ret = inspect(object, { depth });
-    const name = `${this.constructor.name}<${this.property.type}>`;
+    const name = `${this.constructor.name}<${this.property?.type ?? 'unknown'}>`;
 
     return ret === '[Object]' ? `[${name}]` : name + ' ' + ret;
   }

diff --git a/packages/core/src/entity/EntityAssigner.ts b/packages/core/src/entity/EntityAssigner.ts
@@ -105,7 +105,7 @@ export class EntityAssigner {
       return EntityAssigner.assignReference<T>(entity, value, prop, options.em, options);
     }
 
-    if (prop?.kind === ReferenceKind.SCALAR && SCALAR_TYPES.includes(prop.type) && (prop.setter || !prop.getter)) {
+    if (prop?.kind === ReferenceKind.SCALAR && SCALAR_TYPES.includes(prop.type.toLowerCase()) && (prop.setter || !prop.getter)) {
       return entity[propName as keyof T] = validator.validateProperty(prop, value, entity);
     }
 

diff --git a/packages/core/src/entity/EntityValidator.ts b/packages/core/src/entity/EntityValidator.ts
@@ -18,9 +18,9 @@ export class EntityValidator {
         this.validateCollection(entity, prop);
       }
 
-      const SCALAR_TYPES = ['string', 'number', 'boolean', 'Date'];
+      const SCALAR_TYPES = ['string', 'number', 'boolean', 'date'];
 
-      if (prop.kind !== ReferenceKind.SCALAR || !SCALAR_TYPES.includes(prop.type)) {
+      if (prop.kind !== ReferenceKind.SCALAR || !SCALAR_TYPES.includes(prop.type.toLowerCase())) {
         return;
       }
 

diff --git a/packages/core/src/entity/Reference.ts b/packages/core/src/entity/Reference.ts
@@ -204,22 +204,22 @@ export function ref<T extends object, PKV extends Primary<T> = Primary<T>>(entit
  */
 export function ref<T extends object, PKV extends Primary<T> = Primary<T>>(entityOrType?: T | Ref<T> | EntityClass<T>, pk?: T | PKV): Ref<T> | undefined | null {
   if (entityOrType == null) {
-    return pk as null;
+    return entityOrType as unknown as null;
   }
 
-  if (Utils.isEntity(pk)) {
-    return (pk as Dictionary).__helper.toReference();
+  if (Utils.isEntity(entityOrType, true)) {
+    return helper(entityOrType).toReference() as Ref<T>;
   }
 
-  if (Utils.isEntityClass(entityOrType)) {
-    if (pk == null) {
-      return pk as null;
-    }
+  if (Utils.isEntity(pk, true)) {
+    return helper(pk).toReference() as Ref<T>;
+  }
 
-    return Reference.createFromPK<T>(entityOrType as EntityClass<T>, pk);
+  if (pk == null) {
+    return pk as null;
   }
 
-  return (entityOrType as Dictionary).__helper.toReference();
+  return Reference.createFromPK<T>(entityOrType as EntityClass<T>, pk);
 }
 
 /**

diff --git a/packages/core/src/enums.ts b/packages/core/src/enums.ts
@@ -96,7 +96,7 @@ export enum QueryFlag {
   AUTO_JOIN_ONE_TO_ONE_OWNER = 'AUTO_JOIN_ONE_TO_ONE_OWNER',
 }
 
-export const SCALAR_TYPES = ['string', 'number', 'boolean', 'Date', 'Buffer', 'RegExp'];
+export const SCALAR_TYPES = ['string', 'number', 'boolean', 'date', 'buffer', 'regexp'];
 
 export enum ReferenceKind {
   SCALAR = 'scalar',

diff --git a/packages/core/src/metadata/MetadataDiscovery.ts b/packages/core/src/metadata/MetadataDiscovery.ts
@@ -290,6 +290,9 @@ export class MetadataDiscovery {
       return;
     }
 
+    // infer default value from property initializer early, as the metatadata provide might use some defaults, e.g. string for reflect-metadata
+    Utils.values(meta.properties).forEach(prop => this.inferDefaultValue(meta, prop));
+
     // if the definition is using EntitySchema we still want it to go through the metadata provider to validate no types are missing
     await this.metadataProvider.loadEntityMetadata(meta, meta.className);
 
@@ -485,7 +488,7 @@ export class MetadataDiscovery {
     for (const prop of Object.values(meta.properties)) {
       this.initNullability(prop);
       this.applyNamingStrategy(meta, prop);
-      this.initDefaultValue(meta, prop);
+      this.initDefaultValue(prop);
       this.initVersionProperty(meta, prop);
       this.initCustomType(meta, prop);
       await this.initColumnType(meta, prop, meta.path);
@@ -951,20 +954,37 @@ export class MetadataDiscovery {
     return '1';
   }
 
-  private initDefaultValue(meta: EntityMetadata, prop: EntityProperty): void {
+  private inferDefaultValue(meta: EntityMetadata, prop: EntityProperty): void {
+    if (!meta.class) {
+      return;
+    }
+
     try {
       // try to create two entity instances to detect the value is stable
       const entity1 = new meta.class();
       const entity2 = new meta.class();
 
-      // we compare the two values by kind, this will discard things like `new Date()`
-      if (entity1[prop.name] != null && entity1[prop.name] === entity2[prop.name]) {
+      // we compare the two values by reference, this will discard things like `new Date()`
+      if (this.config.get('discovery').inferDefaultValues && prop.default === undefined && entity1[prop.name] != null && entity1[prop.name] === entity2[prop.name]) {
         prop.default ??= entity1[prop.name];
       }
+
+      // if the default value is null, infer nullability
+      if (entity1[prop.name] === null) {
+        prop.nullable ??= true;
+      }
+
+      // but still use object values for type inference if not explicitly set, e.g. `createdAt = new Date()`
+      if (prop.kind === ReferenceKind.SCALAR && prop.type == null && entity1[prop.name] != null) {
+        const type = Utils.getObjectType(entity1[prop.name]);
+        prop.type = type === 'object' ? 'string' : type;
+      }
     } catch {
       // ignore
     }
+  }
 
+  private initDefaultValue(prop: EntityProperty): void {
     if (prop.defaultRaw || !('default' in prop)) {
       return;
     }
@@ -980,7 +1000,7 @@ export class MetadataDiscovery {
 
   private initVersionProperty(meta: EntityMetadata, prop: EntityProperty): void {
     if (prop.version) {
-      this.initDefaultValue(meta, prop);
+      this.initDefaultValue(prop);
       meta.versionProperty = prop.name;
       prop.defaultRaw = this.getDefaultVersionValue(prop);
     }

diff --git a/packages/core/src/platforms/Platform.ts b/packages/core/src/platforms/Platform.ts
@@ -261,6 +261,7 @@ export abstract class Platform {
       case 'numeric': return Type.getType(DecimalType);
       case 'boolean': return Type.getType(BooleanType);
       case 'blob':
+      case 'uint8array':
       case 'buffer': return Type.getType(BlobType);
       case 'uuid': return Type.getType(UuidType);
       case 'date': return Type.getType(DateType);

diff --git a/packages/core/src/utils/Configuration.ts b/packages/core/src/utils/Configuration.ts
@@ -53,6 +53,7 @@ export class Configuration<D extends IDatabaseDriver = IDatabaseDriver> {
       requireEntitiesArray: false,
       alwaysAnalyseProperties: true,
       disableDynamicFileAccess: false,
+      inferDefaultValues: true,
     },
     strict: false,
     validate: false,
@@ -490,6 +491,7 @@ export interface MikroORMOptions<D extends IDatabaseDriver = IDatabaseDriver> ex
     requireEntitiesArray?: boolean;
     alwaysAnalyseProperties?: boolean;
     disableDynamicFileAccess?: boolean;
+    inferDefaultValues?: boolean;
     getMappedType?: (type: string, platform: Platform) => Type<unknown> | undefined;
   };
   driver?: { new(config: Configuration): D };

diff --git a/packages/core/src/utils/Utils.ts b/packages/core/src/utils/Utils.ts
@@ -13,7 +13,6 @@ import { simple as walk } from 'acorn-walk';
 import { clone } from './clone';
 import type {
   Dictionary,
-  EntityClass,
   EntityData,
   EntityDictionary,
   EntityKey,
@@ -628,21 +627,6 @@ export class Utils {
     return !!data.__entity;
   }
 
-  /**
-   * Checks whether given object is an entity instance.
-   */
-  static isEntityClass<T = unknown>(data: any, allowReference = false): data is EntityClass<T> {
-    if (!('prototype' in data)) {
-      return false;
-    }
-
-    if (allowReference && !!data.prototype.__reference) {
-      return true;
-    }
-
-    return !!data.prototype.__entity;
-  }
-
   /**
    * Checks whether the argument is ObjectId instance
    */

diff --git a/packages/knex/src/schema/SchemaHelper.ts b/packages/knex/src/schema/SchemaHelper.ts
@@ -163,7 +163,7 @@ export abstract class SchemaHelper {
     if (changedProperties) {
       Utils.runIfNotEmpty(() => col.defaultTo(column.default == null ? null : knex.raw(column.default)), guard('default'));
     } else {
-      Utils.runIfNotEmpty(() => col.defaultTo(column.default == null ? null : knex.raw(column.default)), column.default !== undefined);
+      Utils.runIfNotEmpty(() => col.defaultTo(knex.raw(column.default!)), column.default != null && column.default !== 'null');
     }
 
     return col;

diff --git a/packages/mongodb/src/MongoPlatform.ts b/packages/mongodb/src/MongoPlatform.ts
@@ -14,6 +14,7 @@ export class MongoPlatform extends Platform {
 
   override setConfig(config: Configuration) {
     config.set('autoJoinOneToOneOwner', false);
+    config.get('discovery').inferDefaultValues = false;
     super.setConfig(config);
   }
 

diff --git a/tests/entities-sql/Author2.ts b/tests/entities-sql/Author2.ts
@@ -19,10 +19,10 @@ export class Author2 extends BaseEntity2 {
   static afterDestroyCalled = 0;
 
   @Property({ length: 3, defaultRaw: 'current_timestamp(3)' })
-  createdAt: Date = new Date();
+  createdAt = new Date();
 
   @Property({ onUpdate: () => new Date(), length: 3, defaultRaw: 'current_timestamp(3)' })
-  updatedAt: Date = new Date();
+  updatedAt = new Date();
 
   @Property()
   name: string;
@@ -35,8 +35,8 @@ export class Author2 extends BaseEntity2 {
   age?: number;
 
   @Index()
-  @Property({ default: false })
-  termsAccepted: boolean = false;
+  @Property()
+  termsAccepted = false;
 
   @Property({ nullable: true })
   optional?: boolean;
@@ -51,10 +51,10 @@ export class Author2 extends BaseEntity2 {
   bornTime?: string;
 
   @OneToMany({ entity: () => Book2, mappedBy: 'author', orderBy: { title: QueryOrder.ASC } })
-  books!: Collection<Book2>;
+  books = new Collection<Book2>(this);
 
   @OneToMany({ entity: () => Book2, mappedBy: 'author', strategy: LoadStrategy.JOINED, orderBy: { title: QueryOrder.ASC } })
-  books2!: Collection<Book2>;
+  books2 = new Collection<Book2>(this);
 
   @OneToOne({ entity: () => Address2, mappedBy: address => address.author, cascade: [Cascade.ALL] })
   address?: Address2;

diff --git a/tests/entities-sql/BaseEntity2.ts b/tests/entities-sql/BaseEntity2.ts
@@ -1,22 +1,12 @@
-import { BeforeCreate, Collection, PrimaryKey, Property, ReferenceKind, Utils, wrap } from '@mikro-orm/core';
+import { BeforeCreate, PrimaryKey, Property } from '@mikro-orm/core';
 
 export abstract class BaseEntity2 {
 
   @PrimaryKey()
   id!: number;
 
   @Property({ persist: false })
-  hookTest: boolean = false;
-
-  protected constructor() {
-    const props = wrap(this, true).__meta.properties;
-
-    Utils.keys(props).forEach(prop => {
-      if ([ReferenceKind.ONE_TO_MANY, ReferenceKind.MANY_TO_MANY].includes(props[prop].kind)) {
-        (this as any)[prop] = new Collection(this);
-      }
-    });
-  }
+  hookTest = false;
 
   @BeforeCreate()
   baseBeforeCreate() {

diff --git a/tests/entities-sql/BaseEntity22.ts b/tests/entities-sql/BaseEntity22.ts
@@ -1,18 +1,5 @@
-import type { AnyEntity } from '@mikro-orm/core';
-import { Collection, ReferenceKind, Utils, wrap } from '@mikro-orm/core';
-
 export abstract class BaseEntity22 {
 
   abstract id: number;
 
-  constructor() {
-    const props = wrap(this, true).__meta.properties;
-
-    Utils.keys(props).forEach(prop => {
-      if ([ReferenceKind.ONE_TO_MANY, ReferenceKind.MANY_TO_MANY].includes(props[prop].kind)) {
-        (this as any)[prop] = new Collection(this as AnyEntity);
-      }
-    });
-  }
-
 }
diff --git a/tests/entities-sql/Book2.ts b/tests/entities-sql/Book2.ts
@@ -36,7 +36,7 @@ export class Book2 {
   uuid = v4();
 
   @Property({ defaultRaw: 'current_timestamp(3)', length: 3 })
-  createdAt: Date = new Date();
+  createdAt = new Date();
 
   @Index({ type: 'fulltext' })
   @Property({ nullable: true, default: '' })

diff --git a/tests/entities-sql/BookTag2.ts b/tests/entities-sql/BookTag2.ts
@@ -1,14 +1,4 @@
-import {
-  BigIntType,
-  Collection,
-  Entity,
-  ManyToMany,
-  PrimaryKey,
-  Property,
-  ReferenceKind,
-  Utils,
-  wrap,
-} from '@mikro-orm/core';
+import { BigIntType, Collection, Entity, ManyToMany, PrimaryKey, Property } from '@mikro-orm/core';
 import { Book2 } from './Book2';
 
 @Entity()
@@ -21,20 +11,12 @@ export class BookTag2 {
   name: string;
 
   @ManyToMany(() => Book2, book => book.tags)
-  books!: Collection<Book2>;
+  books = new Collection<Book2>(this);
 
   @ManyToMany(() => Book2, book => book.tagsUnordered)
-  booksUnordered!: Collection<Book2>;
+  booksUnordered = new Collection<Book2>(this);
 
   constructor(name: string) {
-    const props = wrap(this, true).__meta.properties;
-
-    Utils.keys(props).forEach(prop => {
-      if ([ReferenceKind.ONE_TO_MANY, ReferenceKind.MANY_TO_MANY].includes(props[prop].kind)) {
-        (this as any)[prop] = new Collection(this);
-      }
-    });
-
     this.name = name;
   }
 

diff --git a/tests/entities-sql/Publisher2.ts b/tests/entities-sql/Publisher2.ts
@@ -32,10 +32,10 @@ export class Publisher2 extends BaseEntity2 {
   name: string;
 
   @OneToMany(() => Book2, 'publisher', { joinColumn: 'book_uuid', inverseJoinColumn: 'publisher_id' })
-  books!: Collection<Book2>;
+  books = new Collection<Book2>(this);
 
   @ManyToMany({ entity: () => Test2, pivotTable: 'publisher2_tests', fixedOrder: true })
-  tests!: Collection<Test2>;
+  tests = new Collection<Test2>(this);
 
   @Enum(() => PublisherType)
   type = PublisherType.LOCAL;

diff --git a/tests/entities/BaseEntity.ts b/tests/entities/BaseEntity.ts
@@ -23,16 +23,16 @@ export abstract class BaseEntity<T extends object, Optional extends keyof T = ne
   id!: string;
 
   @Property()
-  createdAt?: Date = new Date();
+  createdAt? = new Date();
 
   @Property({ onUpdate: () => new Date() })
-  updatedAt: Date = new Date();
+  updatedAt = new Date();
 
   @Property()
   foo?: string;
 
   @Property({ persist: false })
-  hookTest: boolean = false;
+  hookTest = false;
 
   @BeforeCreate()
   baseBeforeCreate() {