fix: reuse objects when possible; delete test that protects against this (now desired) behaviour

Author: haggholm

package-lock.json

@@ -68,13 +68,15 @@
     "eslint-plugin-prettier": "^3.1.4",
     "eslint-plugin-promise": "^4.2.1",
     "eslint-plugin-standard": "^4.0.2",
+    "json-schema-merge-allof-old": "npm:json-schema-merge-allof@0.6.0",
     "json-schema-ref-parser": "^9.0.6",
     "json-stringify-safe": "^5.0.1",
     "mocha": "^4.0.1",
     "npm-run-all": "^4.1.5",
     "nyc": "^11.2.1",
     "prettier": "^2.1.2",
     "sinon": "^4.0.1",
+    "sizeof": "^1.0.0",
     "typescript": "^4.0.3"
   }
 }

package.json

@@ -0,0 +1,39 @@
+'use strict';
+
+const mergeOld = require('json-schema-merge-allof-old');
+const mergeNew = require('.');
+const { inspect } = require('util');
+const { sizeof } = require('sizeof');
+const { cloneDeep } = require('lodash');
+
+const schema = {
+  allOf: [
+    {
+      anyOf: [
+        { type: 'object', properties: { foo: { type: 'string' } } },
+        { type: 'object', properties: { bar: { type: 'string' } } }
+      ]
+    },
+    {
+      allOf: [
+        { type: 'object', properties: { one: { type: 'string' } } },
+        { type: 'object', properties: { two: { type: 'string' } } }
+      ]
+    }
+  ]
+};
+
+const m = mergeNew(schema);
+const n = [m, m, m.anyOf, m.anyOf];
+console.log(sizeof(m), sizeof(n), sizeof([mergeNew(schema), mergeNew(schema)]));
+const mergedNew = [mergeNew(schema), mergeNew(schema), mergeNew(schema)];
+const mergedOld = [
+  mergeOld(cloneDeep(schema)),
+  mergeOld(cloneDeep(schema)),
+  mergeOld(cloneDeep(schema))
+];
+
+console.log(inspect(mergedNew));
+
+console.log('Old:', sizeof(mergedOld));
+console.log('New:', sizeof(mergedNew));

sizecmp.js

@@ -180,7 +180,8 @@ function cleanupReturnValue(returnObject) {
       hasOwnProperty.call(returnObject, prop) &&
       isEmptySchema(returnObject[prop])
     ) {
-      delete returnObject[prop];
+      const { [prop]: dummy, ...rest } = returnObject;
+      returnObject = rest;
     }
   }
   return returnObject;
@@ -212,7 +213,7 @@ function callGroupResolver(
       throw new Error('No resolver found for ' + resolverName);
     }
 
-    const compacted = uniqWith(
+    let compacted = uniqWith(
       schemas
         .map(function (schema) {
           return keys.reduce(function (all, key) {
@@ -225,6 +226,7 @@ function callGroupResolver(
         .filter(notUndefined),
       compare
     );
+    if (isEqual(compacted, schemas)) compacted = schemas;
 
     const related =
       resolverName === 'properties' ? propertyRelated : itemsRelated;
@@ -256,7 +258,7 @@ function callGroupResolver(
       };
     }
 
-    const result = resolver(
+    let result = resolver(
       compacted,
       parents.concat(resolverName),
       mergers,
@@ -267,7 +269,18 @@ function callGroupResolver(
       throwIncompatible(compacted, parents.concat(resolverName));
     }
 
-    return cleanupReturnValue(result);
+    result = cleanupReturnValue(result);
+    if (Object.keys(result).length === 1) {
+      if (
+        'properties' in result &&
+        isEqual(result.properties, schemas[0].properties)
+      ) {
+        return { properties: schemas[0].properties };
+      } else if ('items' in result && isEqual(result.items, schemas[0].items)) {
+        return { items: schemas[0].items };
+      }
+    }
+    return result;
   }
 }
 
@@ -279,8 +292,7 @@ function mergeSchemaGroup(group, mergeSchemas, source?: JSONSchema[]) {
     function (all, key) {
       const schemas = extractor(group, key);
       const compacted = uniqWith(schemas.filter(notUndefined), compare);
-      all = setProp(all, key, mergeSchemas(compacted, key));
-      return all;
+      return setProp(all, key, mergeSchemas(compacted, key));
     },
     source ? [] : {}
   );
@@ -772,10 +784,13 @@ function merger<T extends JSONSchema>(
     );
 
     function addToAllOf(unresolvedSchemas) {
-      merged = {
-        ...merged,
-        allOf: mergeWithArray(merged.allOf, unresolvedSchemas)
-      };
+      const mergedAllOf = mergeWithArray(merged.allOf, unresolvedSchemas);
+      if (mergedAllOf !== merged.allOf) {
+        merged = {
+          ...merged,
+          allOf: mergeWithArray(merged.allOf, unresolvedSchemas)
+        };
+      }
     }
 
     return merged;
@@ -792,18 +807,21 @@ merger.options = {
 };
 
 function setProp<T>(ob, key, value): T {
-  if (Array.isArray(ob)) {
-    ob = [...ob];
-    ob[key] = value;
-    return ob;
-  } else {
-    return ob[key] === value ? ob : { ...ob, [key]: value };
+  if (ob[key] !== value) {
+    if (Array.isArray(ob)) {
+      ob = ob.slice();
+      ob[key] = value;
+    } else {
+      ob = { ...ob, [key]: value };
+    }
   }
+  return ob;
 }
 
 function setProps<T>(ob, values): T {
   for (const k in values) {
-    ob = setProp(ob, k, values[k]);
+    const val = values[k];
+    if (val !== ob[k]) ob = setProp(ob, k, val);
   }
   return ob;
 }

src/index.ts

@@ -80,43 +80,6 @@ describe('module', function() {
     })
   })
 
-  it('does not use any original objects or arrays', () => {
-    const schema = {
-      properties: {
-        arr: {
-          type: 'array',
-          items: {
-            type: 'object'
-          },
-          additionalItems: [{
-            type: 'array'
-          }]
-        }
-      },
-      allOf: [{
-        properties: {
-          test: true
-        }
-      }]
-    }
-
-    function innerDeconstruct(schema) {
-      const allChildObj = Object.entries(schema).map(([key, val]) => {
-        if (_.isObject(val)) return innerDeconstruct(val)
-      })
-      return [schema, ..._.flatten(allChildObj)]
-    }
-
-    const getAllObjects = schema => _(innerDeconstruct(schema)).compact().value()
-    const inputObjects = getAllObjects(schema)
-
-    const result = merger(schema)
-    const resultObjects = getAllObjects(result)
-
-    const commonObjects = _.intersection(inputObjects, resultObjects)
-    expect(commonObjects).to.have.length(0)
-  })
-
   it('combines simple usecase', function() {
     var result = merger({
       allOf: [{