[Core] dereference combine sources recursively

packages/core/src/utils.ts

@@ -80,32 +80,59 @@ const isContext = (value: ContextValue): value is Context =>
   !Array.isArray(value) && typeof value === 'object' && value !== null
 
 /**
- * Performs a deep merge of objects and arrays
- * - arrays values are merged index by index
- * - objects are merged by keys
- * - values get replaced, unless undefined
- *
- * ⚠️ this method does not prevent infinite loops while merging circular references ⚠️
+ * Performs a deep merge of objects and arrays.
+ * - Sources won't be mutated
+ * - Object and arrays in the output value are dereferenced ("deep cloned")
+ * - Arrays values are merged index by index
+ * - Objects are merged by keys
+ * - Values get replaced, unless undefined
  *
+ * ⚠️ This function does not prevent infinite loops while merging circular references
  */
-function deepMerge(destination: ContextValue, ...toMerge: ContextValue[]): ContextValue {
-  return toMerge.reduce((value1: ContextValue, value2: ContextValue): ContextValue => {
-    if (isContextArray(value1) && isContextArray(value2)) {
-      return [...Array(Math.max(value1.length, value2.length))].map((_, index) =>
-        deepMerge(value1[index], value2[index])
-      )
+function deepMerge(...sources: ContextValue[]): ContextValue {
+  let destination: ContextValue
+
+  for (let i = sources.length - 1; i >= 0; i -= 1) {
+    const source = sources[i]
+
+    if (source === undefined) {
+      // Ignore any undefined source.
+      continue
+    }
+
+    if (destination === undefined) {
+      // This is the first defined source.  If it is "mergeable" (array or object), initialize the
+      // destination with an empty value that will be populated with all sources sub values.  Else,
+      // just return its value.
+      if (isContext(source)) {
+        destination = {}
+      } else if (isContextArray(source)) {
+        destination = []
+      } else {
+        destination = source
+        break
+      }
     }
-    if (isContext(value1) && isContext(value2)) {
-      return Object.keys(value2).reduce(
-        (merged, key) => ({
-          ...merged,
-          [key]: deepMerge(value1[key], value2[key]),
-        }),
-        value1
-      )
+
+    // At this point, 'destination' is either an array or an object.  If the current 'source' has
+    // the same type we can merge it.  Else, don't try to merge it or any other source.
+    if (isContext(destination) && isContext(source)) {
+      for (const key in source) {
+        if (Object.prototype.hasOwnProperty.call(source, key)) {
+          destination[key] = deepMerge(source[key], destination[key])
+        }
+      }
+    } else if (isContextArray(destination) && isContextArray(source)) {
+      destination.length = Math.max(destination.length, source.length)
+      for (let index = 0; index < source.length; index += 1) {
+        destination[index] = deepMerge(source[index], destination[index])
+      }
+    } else {
+      break
     }
-    return value2 === undefined ? value1 : value2
-  }, destination)
+  }
+
+  return destination
 }
 
 export function combine<A, B>(a: A, b: B): A & B
@@ -115,6 +142,10 @@ export function combine(destination: Context, ...toMerge: Array<Context | null>)
   return deepMerge(destination, ...toMerge.filter((object) => object !== null)) as Context
 }
 
+export function deepClone<T extends ContextValue>(context: T): T {
+  return deepMerge(context) as T
+}
+
 interface Assignable {
   [key: string]: any
 }

packages/core/test/utils.spec.ts

@@ -1,5 +1,6 @@
 import {
   combine,
+  deepClone,
   findCommaSeparatedValue,
   jsonStringify,
   performDraw,
@@ -13,9 +14,9 @@ import {
 describe('utils', () => {
   describe('combine', () => {
     it('should deeply add and replace keys', () => {
-      const target = { a: { b: 'toBeReplaced', c: 'target' } }
-      const source = { a: { b: 'replaced', d: 'source' } }
-      expect(combine(target, source)).toEqual({ a: { b: 'replaced', c: 'target', d: 'source' } })
+      const sourceA = { a: { b: 'toBeReplaced', c: 'source a' } }
+      const sourceB = { a: { b: 'replaced', d: 'source b' } }
+      expect(combine(sourceA, sourceB)).toEqual({ a: { b: 'replaced', c: 'source a', d: 'source b' } })
     })
 
     it('should not replace with undefined', () => {
@@ -33,9 +34,50 @@ describe('utils', () => {
     })
 
     it('should merge arrays', () => {
-      const target = [{ a: 'target' }, 'extraString'] as any
-      const source = [{ b: 'source' }] as any
-      expect(combine(target, source)).toEqual([{ a: 'target', b: 'source' }, 'extraString'])
+      const sourceA = [{ a: 'source a' }, 'extraString'] as any
+      const sourceB = [{ b: 'source b' }] as any
+      expect(combine(sourceA, sourceB)).toEqual([{ a: 'source a', b: 'source b' }, 'extraString'])
+    })
+
+    it('should merge multiple objects', () => {
+      expect(combine({ a: 1 }, { b: 2 }, { c: 3 })).toEqual({ a: 1, b: 2, c: 3 })
+    })
+
+    it('should not keep references on objects', () => {
+      const source = { a: { b: 1 } }
+      const result = combine({}, source)
+      expect(result.a).not.toBe(source.a)
+    })
+
+    it('should not keep references on arrays', () => {
+      const source = { a: [1] }
+      const result = combine({}, source)
+      expect(result.a).not.toBe(source.a)
+    })
+  })
+
+  describe('deepClone', () => {
+    it('should return a result deeply equal to the source', () => {
+      const clonedValue = deepClone({ a: 1 })
+      expect(clonedValue).toEqual({ a: 1 })
+    })
+
+    it('should return a different reference', () => {
+      const value = { a: 1 }
+      const clonedValue = deepClone(value)
+      expect(clonedValue).not.toBe(value)
+    })
+
+    it('should return different references for objects sub values', () => {
+      const value = { a: { b: 1 } }
+      const clonedValue = deepClone(value)
+      expect(clonedValue.a).not.toBe(value.a)
+    })
+
+    it('should return different references for arrays items', () => {
+      const value = { a: [1] }
+      const clonedValue = deepClone(value)
+      expect(clonedValue.a).not.toBe(value.a)
     })
   })
 

packages/logs/src/logger.ts

@@ -48,7 +48,7 @@ export class Logger {
           this.sendLog({
             message,
             status,
-            ...combine({}, this.contextManager.get(), messageContext),
+            ...combine(this.contextManager.get(), messageContext),
           })
           break
         case HandlerType.console:
@@ -78,7 +78,7 @@ export class Logger {
         origin: ErrorOrigin.LOGGER,
       },
     }
-    this.log(message, combine({}, errorOrigin, messageContext), StatusType.error)
+    this.log(message, combine(errorOrigin, messageContext), StatusType.error)
   }
 
   setContext(context: Context) {

packages/logs/src/logs.ts

@@ -49,7 +49,7 @@ export function doStartLogs(
   errorObservable.subscribe((e: ErrorMessage) =>
     errorLogger.error(
       e.message,
-      combine({ date: getTimestamp(e.startTime), ...e.context }, getRUMInternalContext(e.startTime))
+      combine({ date: getTimestamp(e.startTime) }, e.context, getRUMInternalContext(e.startTime))
     )
   )
 

packages/rum/src/rum.entry.ts

@@ -6,6 +6,7 @@ import {
   Context,
   ContextValue,
   createContextManager,
+  deepClone,
   getGlobalObject,
   isPercentage,
   makeGlobal,
@@ -56,7 +57,7 @@ export function makeRumGlobal(startRumImpl: StartRum) {
   }
   const beforeInitAddUserAction = new BoundedBuffer<[CustomUserAction, Context]>()
   let addUserActionStrategy: ReturnType<StartRum>['addUserAction'] = (action) => {
-    beforeInitAddUserAction.add([action, combine({}, globalContextManager.get())])
+    beforeInitAddUserAction.add([action, deepClone(globalContextManager.get())])
   }
 
   return makeGlobal({
@@ -94,7 +95,7 @@ export function makeRumGlobal(startRumImpl: StartRum) {
     addUserAction: monitor((name: string, context?: Context) => {
       addUserActionStrategy({
         name,
-        context: combine({}, context),
+        context: deepClone(context),
         startTime: performance.now(),
         type: UserActionType.CUSTOM,
       })

packages/rum/src/userActionCollection.ts

@@ -21,7 +21,7 @@ export interface CustomUserAction {
   type: UserActionType.CUSTOM
   name: string
   startTime: number
-  context: Context
+  context?: Context
 }
 
 export interface AutoUserAction {