fix(core): make deepCopy backward compatible (#4505)

* fix(core): make `deepCopy` backward compatible

`JSON.parse(JSON.stringify())`  uses `.toJSON` when available. so should `deepCopy`

* fix(core): prevent double quotes on luxon datetimes (#4508)

* 🐛 Prevent double quotes on luxon datetimes

* ⚡ Generalize solution

* update the types in packages/workflow/src/utils.ts

* add `toJSON` check to NodeErrors.isTraversableObject as well

* move the toJSON check before the cyclic dependency check

* fix(core): keep backward compatibility in deepCopy by calling `toJSON` on objects that have it

* fix(core): updating deepCopy typings

* Revert "fix(core): updating deepCopy typings"

This reverts commit 100a0f1f3d.

* fix(core): temporarily removing Date cloning from deepCopy

* fix(core): updating deepCopy types

* fix(core): updating deepCopy

* fix(core): updating deepCopy get prototype of object

Co-authored-by: Iván Ovejero <ivov.src@gmail.com>
Co-authored-by: Csaba Tuncsik <csaba@n8n.io>

packages/nodes-base/nodes/Code/utils.ts

@@ -21,7 +21,7 @@ export function isObject(maybe: unknown): maybe is { [key: string]: unknown } {
 }
 
 function isTraversable(maybe: unknown): maybe is IDataObject {
-	return isObject(maybe) && Object.keys(maybe).length > 0;
+	return isObject(maybe) && typeof maybe.toJSON !== 'function' && Object.keys(maybe).length > 0;
 }
 
 export type CodeNodeMode = 'runOnceForAllItems' | 'runOnceForEachItem';

packages/workflow/src/NodeErrors.ts

@@ -176,8 +176,12 @@ abstract class NodeError extends ExecutionBaseError {
 	// eslint-disable-next-line @typescript-eslint/no-explicit-any
 	protected isTraversableObject(value: any): value is JsonObject {
 		return (
+			value &&
+			typeof value === 'object' &&
+			!Array.isArray(value) &&
+			typeof value.toJSON !== 'function' &&
 			// eslint-disable-next-line @typescript-eslint/no-unsafe-argument
-			value && typeof value === 'object' && !Array.isArray(value) && !!Object.keys(value).length
+			!!Object.keys(value).length
 		);
 	}
 

packages/workflow/src/utils.ts

@@ -1,34 +1,38 @@
 /* eslint-disable @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-unsafe-member-access, @typescript-eslint/no-unsafe-return, @typescript-eslint/no-unsafe-argument */
-export const deepCopy = <T>(source: T, hash = new WeakMap(), path = ''): T => {
-	let clone: any;
-	let i: any;
+type Primitives = string | number | boolean | bigint | symbol | null | undefined;
+export const deepCopy = <T extends ((object | Date) & { toJSON?: () => string }) | Primitives>(
+	source: T,
+	hash = new WeakMap(),
+	path = '',
+): T => {
 	const hasOwnProp = Object.prototype.hasOwnProperty.bind(source);
 	// Primitives & Null & Function
-	if (typeof source !== 'object' || source === null || source instanceof Function) {
+	if (typeof source !== 'object' || source === null || typeof source === 'function') {
 		return source;
 	}
+	// Date and other objects with toJSON method
+	// TODO: remove this when other code parts not expecting objects with `.toJSON` method called and add back checking for Date and cloning it properly
+	if (typeof source.toJSON === 'function') {
+		return source.toJSON() as T;
+	}
 	if (hash.has(source)) {
 		return hash.get(source);
 	}
-	// Date
-	if (source instanceof Date) {
-		return new Date(source.getTime()) as T;
-	}
 	// Array
 	if (Array.isArray(source)) {
-		clone = [];
+		const clone = [];
 		const len = source.length;
-		for (i = 0; i < len; i++) {
-			clone[i] = deepCopy(source[i], hash, path + `[${i as string}]`);
+		for (let i = 0; i < len; i++) {
+			clone[i] = deepCopy(source[i], hash, path + `[${i}]`);
 		}
-		return clone;
+		return clone as T;
 	}
 	// Object
-	clone = {};
+	const clone = Object.create(Object.getPrototypeOf({}));
 	hash.set(source, clone);
-	for (i in source) {
+	for (const i in source) {
 		if (hasOwnProp(i)) {
-			clone[i] = deepCopy((source as any)[i], hash, path + `.${i as string}`);
+			clone[i] = deepCopy((source as any)[i], hash, path + `.${i}`);
 		}
 	}
 	return clone;

packages/workflow/test/utils.test.ts

@@ -19,6 +19,11 @@ describe('jsonParse', () => {
 
 describe('deepCopy', () => {
 	it('should deep copy an object', () => {
+		const serializable = {
+			x: 1,
+			y: 2,
+			toJSON: () => 'x:1,y:2',
+		};
 		const object = {
 			deep: {
 				props: {
@@ -26,6 +31,7 @@ describe('deepCopy', () => {
 				},
 				arr: [1, 2, 3],
 			},
+			serializable,
 			arr: [
 				{
 					prop: {
@@ -34,17 +40,18 @@ describe('deepCopy', () => {
 				},
 			],
 			func: () => {},
-			date: new Date(),
+			date: new Date(1667389172201),
 			undef: undefined,
 			nil: null,
 			bool: true,
 			num: 1,
 		};
 		const copy = deepCopy(object);
-		expect(copy).toEqual(object);
 		expect(copy).not.toBe(object);
 		expect(copy.arr).toEqual(object.arr);
 		expect(copy.arr).not.toBe(object.arr);
+		expect(copy.date).toBe('2022-11-02T11:39:32.201Z');
+		expect(copy.serializable).toBe(serializable.toJSON());
 		expect(copy.deep.props).toEqual(object.deep.props);
 		expect(copy.deep.props).not.toBe(object.deep.props);
 	});
@@ -65,7 +72,7 @@ describe('deepCopy', () => {
 				},
 			],
 			func: () => {},
-			date: new Date(),
+			date: new Date(1667389172201),
 			undef: undefined,
 			nil: null,
 			bool: true,
@@ -74,14 +81,16 @@ describe('deepCopy', () => {
 
 		object.circular = object;
 		object.deep.props.circular = object;
-		object.deep.arr.push(object)
+		object.deep.arr.push(object);
 
 		const copy = deepCopy(object);
-		expect(copy).toEqual(object);
 		expect(copy).not.toBe(object);
 		expect(copy.arr).toEqual(object.arr);
 		expect(copy.arr).not.toBe(object.arr);
-		expect(copy.deep.props).toEqual(object.deep.props);
-		expect(copy.deep.props).not.toBe(object.deep.props);
+		expect(copy.date).toBe('2022-11-02T11:39:32.201Z');
+		expect(copy.deep.props.circular).toBe(copy);
+		expect(copy.deep.props.circular).not.toBe(object);
+		expect(copy.deep.arr.slice(-1)[0]).toBe(copy);
+		expect(copy.deep.arr.slice(-1)[0]).not.toBe(object);
 	});
 });