mergeDeep improvements (#3201)

* enhance(delegate): use Object.assign instead of mergeDeep in mergeExternalObjects * Avoid isScalarType in mergeDeep * No need for isScalarType
ardatan · Jul 13, 2021 · 6877b91 · 6877b91
1 parent bcdbba3
commit 6877b91
Show file tree

Hide file tree

Showing 7 changed files with 63 additions and 55 deletions.
diff --git a/.changeset/green-rocks-swim.md b/.changeset/green-rocks-swim.md
@@ -0,0 +1,8 @@
+---
+'@graphql-tools/utils': major
+---
+
+BREAKING CHANGES;
+
+`mergeDeep` now takes an array of sources instead of set of parameters as input and it takes an additional flag to enable prototype merging
+Instead of `mergeDeep(...sources)` => `mergeDeep(sources)`
diff --git a/packages/delegate/src/externalObjects.ts b/packages/delegate/src/externalObjects.ts
@@ -1,6 +1,6 @@
 import { GraphQLSchema, GraphQLError, GraphQLObjectType, SelectionSetNode, locatedError } from 'graphql';
 
-import { mergeDeep, relocatedError, GraphQLExecutionContext, collectFields } from '@graphql-tools/utils';
+import { relocatedError, GraphQLExecutionContext, collectFields } from '@graphql-tools/utils';
 
 import { SubschemaConfig, ExternalObject } from './types';
 import { OBJECT_SUBSCHEMA_SYMBOL, FIELD_SUBSCHEMA_MAP_SYMBOL, UNPATHED_ERRORS_SYMBOL } from './symbols';
@@ -73,16 +73,17 @@ export function mergeExternalObjects(
     }
   }
 
-  const combinedResult: ExternalObject = results.reduce(mergeDeep, target);
+  const combinedResult: ExternalObject = Object.assign({}, target, ...results);
 
-  const newFieldSubschemaMap = results.reduce((newFieldSubschemaMap, source) => {
+  const newFieldSubschemaMap = target[FIELD_SUBSCHEMA_MAP_SYMBOL] ?? Object.create(null);
+
+  for (const source of results) {
     const objectSubschema = source[OBJECT_SUBSCHEMA_SYMBOL];
     const fieldSubschemaMap = source[FIELD_SUBSCHEMA_MAP_SYMBOL];
     for (const responseKey in source) {
       newFieldSubschemaMap[responseKey] = fieldSubschemaMap?.[responseKey] ?? objectSubschema;
     }
-    return newFieldSubschemaMap;
-  }, target[FIELD_SUBSCHEMA_MAP_SYMBOL] ?? Object.create(null));
+  }
 
   combinedResult[FIELD_SUBSCHEMA_MAP_SYMBOL] = newFieldSubschemaMap;
   combinedResult[OBJECT_SUBSCHEMA_SYMBOL] = target[OBJECT_SUBSCHEMA_SYMBOL];

diff --git a/packages/merge/src/extensions.ts b/packages/merge/src/extensions.ts
@@ -140,10 +140,7 @@ export function travelSchemaPossibleExtensions(
 }
 
 export function mergeExtensions(extensions: SchemaExtensions[]): SchemaExtensions {
-  return extensions.reduce(
-    (result, extensionObj) => [result, extensionObj].reduce<SchemaExtensions>(mergeDeep, {} as SchemaExtensions),
-    {} as SchemaExtensions
-  );
+  return mergeDeep(extensions);
 }
 
 function applyExtensionObject(
@@ -154,7 +151,7 @@ function applyExtensionObject(
     return;
   }
 
-  obj.extensions = [obj.extensions || {}, extensions || {}].reduce(mergeDeep, {});
+  obj.extensions = mergeDeep([obj.extensions || {}, extensions || {}]);
 }
 
 export function applyExtensions(schema: GraphQLSchema, extensions: SchemaExtensions): GraphQLSchema {

diff --git a/packages/merge/src/merge-resolvers.ts b/packages/merge/src/merge-resolvers.ts
@@ -62,7 +62,7 @@ export function mergeResolvers<TSource, TContext>(
       resolvers.push(resolversDefinition);
     }
   }
-  const result = resolvers.reduce(mergeDeep, {});
+  const result = mergeDeep(resolvers, true);
 
   if (options?.exclusions) {
     for (const exclusion of options.exclusions) {

diff --git a/packages/stitching-directives/src/stitchingDirectivesTransformer.ts b/packages/stitching-directives/src/stitchingDirectivesTransformer.ts
@@ -329,10 +329,10 @@ export function stitchingDirectivesTransformer(
 
             const additionalArgs = mergeDirective['additionalArgs'];
             if (additionalArgs != null) {
-              parsedMergeArgsExpr.args = mergeDeep(
+              parsedMergeArgsExpr.args = mergeDeep([
                 parsedMergeArgsExpr.args,
-                valueFromASTUntyped(parseValue(`{ ${additionalArgs} }`, { noLocation: true }))
-              );
+                valueFromASTUntyped(parseValue(`{ ${additionalArgs} }`, { noLocation: true })),
+              ]);
             }
 
             mergedTypesResolversInfo[typeName] = {
@@ -473,13 +473,13 @@ function generateArgsFromKeysFn(
 ): (keys: ReadonlyArray<any>) => Record<string, any> {
   const { expansions, args } = mergedTypeResolverInfo;
   return (keys: ReadonlyArray<any>): Record<string, any> => {
-    const newArgs = mergeDeep({}, args);
+    const newArgs = mergeDeep([{}, args]);
     if (expansions) {
       for (const expansion of expansions) {
         const mappingInstructions = expansion.mappingInstructions;
         const expanded: Array<any> = [];
         for (const key of keys) {
-          let newValue = mergeDeep({}, expansion.valuePath);
+          let newValue = mergeDeep([{}, expansion.valuePath]);
           for (const { destinationPath, sourcePath } of mappingInstructions) {
             if (destinationPath.length) {
               addProperty(newValue, destinationPath, getProperty(key, sourcePath));
@@ -500,7 +500,7 @@ function generateArgsFn(mergedTypeResolverInfo: MergedTypeResolverInfo): (origin
   const { mappingInstructions, args, usedProperties } = mergedTypeResolverInfo;
 
   return (originalResult: any): Record<string, any> => {
-    const newArgs = mergeDeep({}, args);
+    const newArgs = mergeDeep([{}, args]);
     const filteredResult = getProperties(originalResult, usedProperties);
     if (mappingInstructions) {
       for (const mappingInstruction of mappingInstructions) {
@@ -532,7 +532,7 @@ function buildKeyExpr(key: Array<string>): string {
     for (const aliasPart of aliasParts.reverse()) {
       object = { [aliasPart]: object };
     }
-    mergedObject = mergeDeep(mergedObject, object);
+    mergedObject = mergeDeep([mergedObject, object]);
   }
 
   return JSON.stringify(mergedObject).replace(/"/g, '');

diff --git a/packages/utils/src/mergeDeep.ts b/packages/utils/src/mergeDeep.ts
@@ -1,28 +1,29 @@
 import { isSome } from './helpers';
-import { isScalarType } from 'graphql';
 
 type BoxedTupleTypes<T extends any[]> = { [P in keyof T]: [T[P]] }[Exclude<keyof T, keyof any[]>];
 type UnionToIntersection<U> = (U extends any ? (k: U) => void : never) extends (k: infer I) => void ? I : never;
 type UnboxIntersection<T> = T extends { 0: infer U } ? U : never;
 // eslint-disable-next-line @typescript-eslint/ban-types
-export function mergeDeep<T extends object, S extends any[]>(
-  target: T,
-  ...sources: S
-): T & UnboxIntersection<UnionToIntersection<BoxedTupleTypes<S>>> & any {
-  if (isScalarType(target)) {
-    return target;
-  }
+export function mergeDeep<S extends any[]>(
+  sources: S,
+  respectPrototype = false
+): UnboxIntersection<UnionToIntersection<BoxedTupleTypes<S>>> & any {
+  const target = sources[0] || {};
   const output = {};
-  Object.setPrototypeOf(output, Object.create(Object.getPrototypeOf(target)));
-  for (const source of [target, ...sources]) {
+  if (respectPrototype) {
+    Object.setPrototypeOf(output, Object.create(Object.getPrototypeOf(target)));
+  }
+  for (const source of sources) {
     if (isObject(target) && isObject(source)) {
-      const outputPrototype = Object.getPrototypeOf(output);
-      const sourcePrototype = Object.getPrototypeOf(source);
-      if (sourcePrototype) {
-        for (const key of Object.getOwnPropertyNames(sourcePrototype)) {
-          const descriptor = Object.getOwnPropertyDescriptor(sourcePrototype, key);
-          if (isSome(descriptor)) {
-            Object.defineProperty(outputPrototype, key, descriptor);
+      if (respectPrototype) {
+        const outputPrototype = Object.getPrototypeOf(output);
+        const sourcePrototype = Object.getPrototypeOf(source);
+        if (sourcePrototype) {
+          for (const key of Object.getOwnPropertyNames(sourcePrototype)) {
+            const descriptor = Object.getOwnPropertyDescriptor(sourcePrototype, key);
+            if (isSome(descriptor)) {
+              Object.defineProperty(outputPrototype, key, descriptor);
+            }
           }
         }
       }
@@ -32,7 +33,7 @@ export function mergeDeep<T extends object, S extends any[]>(
           if (!(key in output)) {
             Object.assign(output, { [key]: source[key] });
           } else {
-            output[key] = mergeDeep(output[key], source[key]);
+            output[key] = mergeDeep([output[key], source[key]] as S, respectPrototype);
           }
         } else {
           Object.assign(output, { [key]: source[key] });

diff --git a/packages/utils/tests/mergeDeep.test.ts b/packages/utils/tests/mergeDeep.test.ts
@@ -1,6 +1,24 @@
 import { mergeDeep } from '@graphql-tools/utils'
 
 describe('mergeDeep', () => {
+
+  test('merges deeply', () => {
+    const x = { a: { one: 1 } }
+    const y = { a: { two: 2 } }
+    expect(mergeDeep([x, y])).toEqual({ a: { one: 1, two: 2 } })
+  })
+
+  test('strips property symbols', () => {
+    const x = {}
+    const symbol = Symbol('symbol')
+    x[symbol] = 'value'
+    const y = { a: 2 }
+
+    const merged = mergeDeep([x, y])
+    expect(merged).toStrictEqual({ a: 2 })
+    expect(Object.getOwnPropertySymbols(merged)).toEqual([])
+  })
+
   test('merges prototypes', () => {
     const ClassA = class {
       a() {
@@ -13,17 +31,11 @@ describe('mergeDeep', () => {
       }
     }
 
-    const merged = mergeDeep(new ClassA(), new ClassB())
+    const merged = mergeDeep([new ClassA(), new ClassB()], true)
     expect(merged.a()).toEqual('a')
     expect(merged.b()).toEqual('b')
   })
 
-  test('merges deeply', () => {
-    const x = { a: { one: 1 } }
-    const y = { a: { two: 2 } }
-    expect(mergeDeep(x, y)).toEqual({ a: { one: 1, two: 2 } })
-  })
-
   test('merges prototype deeply', () => {
     const ClassA = class {
       a() {
@@ -36,20 +48,9 @@ describe('mergeDeep', () => {
       }
     }
 
-    const merged = mergeDeep({ one: new ClassA()}, { one: new ClassB()})
+    const merged = mergeDeep([{ one: new ClassA() }, { one: new ClassB() }], true)
     expect(merged.one.a()).toEqual('a')
     expect(merged.one.b()).toEqual('b')
     expect(merged.a).toBeUndefined()
   })
-
-  test('strips property symbols', () => {
-    const x = {}
-    const symbol = Symbol('symbol')
-    x[symbol] = 'value'
-    const y = { a: 2 }
-
-    const merged = mergeDeep(x, y)
-    expect(merged).toStrictEqual({ a: 2 })
-    expect(Object.getOwnPropertySymbols(merged)).toEqual([])
-  })
 })