refactor: rewrite merge to handle recursion & be cleaner

typeorm · Jun 21, 2021 · d7e0dca · d7e0dca
1 parent ffcc621
commit d7e0dca
Show file tree

Hide file tree

Showing 2 changed files with 53 additions and 30 deletions.
diff --git a/src/util/OrmUtils.ts b/src/util/OrmUtils.ts
@@ -60,22 +60,47 @@ export class OrmUtils {
 
     // Checks if it's an object made by Object.create(null), {} or new Object()
     private static isPlainObject(item: any) {
-        // Filters out undefined, null, function, and primitive array, number, string
-        // Less expensive call than toString
-        const isObject = item != null && typeof item === "object" && !Array.isArray(item);
-        if (!isObject) {
+        if (item === null || item === undefined) {
             return false;
         }
 
-        // Filters out Date, Set, Map, String, Math, Number and a few others
-        const isObjectObject = Object.prototype.toString.call(item) === "[object Object]";
-        if (!isObjectObject) {
-            return false;
+        return !item.constructor || item.constructor === Object;
+    }
+
+    private static mergeKey(target: any, key: string, value: any, memo: Map<any, any>) {
+        // Have we seen this before?  Prevent infinite recursion.
+        if (memo.has(value)) {
+            Object.assign(target, { [key]: memo.get(value) });
+            return;
         }
 
-        // Filters out Buffer and custom instances of classes
-        const prototype = Object.getPrototypeOf(item);
-        return prototype === null || prototype === Object.getPrototypeOf({});
+        let newValue = value;
+
+        if (this.isPlainObject(value)) {
+            const newValue = Object.create(Object.getPrototypeOf(value));
+
+            if (!target[key]) {
+                Object.assign(target, { [key]: newValue });
+            }
+
+            memo.set(value, newValue);
+            this.merge(target[key], value);
+            memo.delete(value);
+        }
+
+        Object.assign(target, { [key]: newValue });
+    }
+
+    private static merge(target: any, source: any, memo: Map<any, any> = new Map()): any {
+        let keys = [];
+
+        if (this.isPlainObject(target) && this.isPlainObject(source)) {
+            keys.push(...Object.keys(source));
+        }
+
+        for (const key of keys) {
+            this.mergeKey(target, key, source[key], memo);
+        }
     }
 
     /**
@@ -84,28 +109,15 @@ export class OrmUtils {
      * @see http://stackoverflow.com/a/34749873
      */
     static mergeDeep(target: any, ...sources: any[]): any {
-        if (!sources.length) return target;
-        const source = sources.shift();
-
-        if (this.isPlainObject(target) && this.isPlainObject(source)) {
-            for (const key in source) {
-                const value = source[key];
-                if (key === "__proto__" || value instanceof Promise)
-                    continue;
-
-                if (this.isPlainObject(value)) {
-                    if (!target[key]) {
-                        Object.assign(target, { [key]: Object.create(Object.getPrototypeOf(value)) });
-                    }
+        if (!sources.length) {
+            return target;
+        }
 
-                    this.mergeDeep(target[key], value);
-                } else {
-                    Object.assign(target, { [key]: value });
-                }
-            }
+        for (const source of sources) {
+            OrmUtils.merge(target, source);
         }
 
-        return this.mergeDeep(target, ...sources);
+        return target;
     }
 
     /**

diff --git a/test/functional/util/OrmUtils.ts b/test/functional/util/OrmUtils.ts
@@ -54,6 +54,17 @@ describe("OrmUtils.mergeDeep", () => {
         expect(mergeDeep(b, a, b)).to.deep.equal(c);
     });
 
+    it("should merge recursively deep objects correctly", () => {
+        let a: Record<string, any> = {};
+        let b: Record<string, any> = {};
+
+        a['b'] = b;
+        a['a'] = a;
+        b['a'] = a;
+
+        expect(mergeDeep({}, a));
+    });
+
     it("should reference copy complex instances of classes.", () => {
         class Foo {
             recursive: Foo;