fix(core): respect undefined when assigning to object properties

Closes #4428
mikro-orm · Jun 9, 2023 · 217ff8f · 217ff8f
1 parent ef22b21
commit 217ff8f
Show file tree

Hide file tree

Showing 13 changed files with 275 additions and 16 deletions.
diff --git a/packages/better-sqlite/src/BetterSqliteConnection.ts b/packages/better-sqlite/src/BetterSqliteConnection.ts
@@ -32,7 +32,7 @@ export class BetterSqliteConnection extends AbstractSqlConnection {
   }
 
   protected getKnexOptions(type: string): Knex.Config {
-    return Utils.merge({
+    return Utils.mergeConfig({
       client: type,
       connection: {
         filename: this.config.get('dbName'),

diff --git a/packages/core/src/MikroORM.ts b/packages/core/src/MikroORM.ts
@@ -35,7 +35,7 @@ export class MikroORM<D extends IDatabaseDriver = IDatabaseDriver> {
     }
 
     let opts = options instanceof Configuration ? options.getAll() : options;
-    opts = Utils.merge(opts, env);
+    opts = Utils.mergeConfig(opts, env);
     await ConfigurationLoader.commonJSCompat(opts as object);
 
     if ('DRIVER' in this && !opts.driver && !opts.type) {

diff --git a/packages/core/src/decorators/Entity.ts b/packages/core/src/decorators/Entity.ts
@@ -6,7 +6,7 @@ import type { FindOptions } from '../drivers/IDatabaseDriver';
 export function Entity(options: EntityOptions<any> = {}) {
   return function <T>(target: T & Dictionary) {
     const meta = MetadataStorage.getMetadataFromDecorator(target);
-    Utils.merge(meta, options);
+    Utils.mergeConfig(meta, options);
     meta.class = target as unknown as Constructor<T>;
 
     if (!options.abstract || meta.discriminatorColumn) {

diff --git a/packages/core/src/metadata/JavaScriptMetadataProvider.ts b/packages/core/src/metadata/JavaScriptMetadataProvider.ts
@@ -16,7 +16,7 @@ export class JavaScriptMetadataProvider extends MetadataProvider {
       }
     });
 
-    Utils.merge(meta, schema);
+    Utils.mergeConfig(meta, schema);
     Object.entries(meta.properties).forEach(([name, prop]) => {
       this.initProperty(prop, name);
     });
@@ -26,7 +26,7 @@ export class JavaScriptMetadataProvider extends MetadataProvider {
    * Re-hydrates missing attributes like `onUpdate` (functions are lost when caching to JSON)
    */
   loadFromCache(meta: EntityMetadata, cache: EntityMetadata): void {
-    Utils.merge(meta, cache);
+    Utils.mergeConfig(meta, cache);
     const schema = this.getSchema(meta);
 
     Object.entries(schema.properties).forEach(([name, prop]) => {

diff --git a/packages/core/src/metadata/MetadataProvider.ts b/packages/core/src/metadata/MetadataProvider.ts
@@ -28,7 +28,7 @@ export abstract class MetadataProvider {
       }
     });
 
-    Utils.merge(meta, cache);
+    Utils.mergeConfig(meta, cache);
   }
 
   useCache(): boolean {

diff --git a/packages/core/src/utils/Configuration.ts b/packages/core/src/utils/Configuration.ts
@@ -154,7 +154,7 @@ export class Configuration<D extends IDatabaseDriver = IDatabaseDriver> {
       Utils.setDynamicImportProvider(options.dynamicImportProvider);
     }
 
-    this.options = Utils.merge({}, Configuration.DEFAULTS, options);
+    this.options = Utils.mergeConfig({}, Configuration.DEFAULTS, options);
     this.options.baseDir = Utils.absolutePath(this.options.baseDir);
 
     if (validate) {

diff --git a/packages/core/src/utils/ConfigurationLoader.ts b/packages/core/src/utils/ConfigurationLoader.ts
@@ -40,12 +40,12 @@ export class ConfigurationLoader {
 
         const esmConfigOptions = await this.isESM() ? { entityGenerator: { esmImport: true } } : {};
 
-        return new Configuration(Utils.merge({}, esmConfigOptions, tmp, options, env), validate);
+        return new Configuration(Utils.mergeConfig({}, esmConfigOptions, tmp, options, env), validate);
       }
     }
 
     if (Utils.hasObjectKeys(env)) {
-      return new Configuration(Utils.merge({}, options, env), validate);
+      return new Configuration(Utils.mergeConfig({}, options, env), validate);
     }
 
     throw new Error(`MikroORM config file not found in ['${paths.join(`', '`)}']`);

diff --git a/packages/core/src/utils/Utils.ts b/packages/core/src/utils/Utils.ts
@@ -311,6 +311,20 @@ export class Utils {
    * Merges all sources into the target recursively.
    */
   static merge(target: any, ...sources: any[]): any {
+    return Utils._merge(target, sources, false);
+  }
+
+  /**
+   * Merges all sources into the target recursively. Ignores `undefined` values.
+   */
+  static mergeConfig(target: any, ...sources: any[]): any {
+    return Utils._merge(target, sources, true);
+  }
+
+  /**
+   * Merges all sources into the target recursively.
+   */
+  private static _merge(target: any, sources: any[], ignoreUndefined: boolean): any {
     if (!sources.length) {
       return target;
     }
@@ -319,7 +333,7 @@ export class Utils {
 
     if (Utils.isObject(target) && Utils.isPlainObject(source)) {
       Object.entries(source).forEach(([key, value]) => {
-        if (typeof value === 'undefined') {
+        if (ignoreUndefined && typeof value === 'undefined') {
           return;
         }
 
@@ -333,14 +347,14 @@ export class Utils {
             Object.assign(target, { [key]: {} });
           }
 
-          Utils.merge(target[key], value);
+          Utils._merge(target[key], [value], ignoreUndefined);
         } else {
           Object.assign(target, { [key]: value });
         }
       });
     }
 
-    return Utils.merge(target, ...sources);
+    return Utils._merge(target, sources, ignoreUndefined);
   }
 
   static getRootEntity(metadata: IMetadataStorage, meta: EntityMetadata): EntityMetadata {

diff --git a/packages/knex/src/AbstractSqlConnection.ts b/packages/knex/src/AbstractSqlConnection.ts
@@ -150,7 +150,7 @@ export abstract class AbstractSqlConnection extends Connection {
   }
 
   protected getKnexOptions(type: string): Knex.Config {
-    const config = Utils.merge({
+    const config = Utils.mergeConfig({
       client: type,
       connection: this.getConnectionOptions(),
       pool: this.config.get('pool'),

diff --git a/packages/mongodb/src/MongoConnection.ts b/packages/mongodb/src/MongoConnection.ts
@@ -123,7 +123,7 @@ export class MongoConnection extends Connection {
       ret.maxPoolSize = pool.max;
     }
 
-    return Utils.merge(ret, this.config.get('driverOptions'));
+    return Utils.mergeConfig(ret, this.config.get('driverOptions'));
   }
 
   getClientUrl(): string {

diff --git a/packages/sqlite/src/SqliteConnection.ts b/packages/sqlite/src/SqliteConnection.ts
@@ -31,7 +31,7 @@ export class SqliteConnection extends AbstractSqlConnection {
   }
 
   protected getKnexOptions(type: string): Knex.Config {
-    return Utils.merge({
+    return Utils.mergeConfig({
       client: type,
       connection: {
         filename: this.config.get('dbName'),

diff --git a/tests/Utils.test.ts b/tests/Utils.test.ts
@@ -96,8 +96,9 @@ describe('Utils', () => {
     expect(Utils.merge({ a: 'a', b: ['c'] }, { b: [] })).toEqual({ a: 'a', b: [] });
     expect(Utils.merge({ a: 'a', b: ['c'] }, { a: 'b' })).toEqual({ a: 'b', b: ['c'] });
     expect(Utils.merge({ a: 'a', b: ['c'] }, { a: null })).toEqual({ a: null, b: ['c'] });
-    expect(Utils.merge({ a: 'a', b: ['c'] }, { a: undefined })).toEqual({ a: 'a', b: ['c'] });
+    expect(Utils.merge({ a: 'a', b: ['c'] }, { a: undefined })).toEqual({ a: undefined, b: ['c'] });
     expect(Utils.merge('a', 'b')).toEqual('a');
+    expect(Utils.mergeConfig({ a: 'a', b: ['c'] }, { a: undefined })).toEqual({ a: 'a', b: ['c'] });
 
     // GH #4101
     const source = {