From b282c7e5d98c94b13a861167fef8af204267c5bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E0=A4=95=E0=A4=BE=E0=A4=B0=E0=A4=A4=E0=A5=8B=E0=A4=AB?= =?UTF-8?q?=E0=A5=8D=E0=A4=AB=E0=A5=87=E0=A4=B2=E0=A4=B8=E0=A5=8D=E0=A4=95?= =?UTF-8?q?=E0=A5=8D=E0=A4=B0=E0=A4=BF=E0=A4=AA=E0=A5=8D=E0=A4=9F=E2=84=A2?= Date: Wed, 2 Nov 2022 17:44:12 +0100 Subject: [PATCH] fix(core): make `deepCopy` backward compatible (#4505) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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) * :bug: Prevent double quotes on luxon datetimes * :zap: 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 100a0f1f3d7ddac5425ccc8498381324f418d7a2. * 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 Co-authored-by: Csaba Tuncsik --- packages/nodes-base/nodes/Code/utils.ts | 2 +- packages/workflow/src/NodeErrors.ts | 6 ++++- packages/workflow/src/utils.ts | 34 ++++++++++++++----------- packages/workflow/test/utils.test.ts | 23 ++++++++++++----- 4 files changed, 41 insertions(+), 24 deletions(-) diff --git a/packages/nodes-base/nodes/Code/utils.ts b/packages/nodes-base/nodes/Code/utils.ts index 7d92c9eddbfc3..0b389af9c145d 100644 --- a/packages/nodes-base/nodes/Code/utils.ts +++ b/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'; diff --git a/packages/workflow/src/NodeErrors.ts b/packages/workflow/src/NodeErrors.ts index ad921e5a33f07..834111088c0b1 100644 --- a/packages/workflow/src/NodeErrors.ts +++ b/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 ); } diff --git a/packages/workflow/src/utils.ts b/packages/workflow/src/utils.ts index 87fa050a5ce39..8201567c90c89 100644 --- a/packages/workflow/src/utils.ts +++ b/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 = (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 = 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; diff --git a/packages/workflow/test/utils.test.ts b/packages/workflow/test/utils.test.ts index bd83309bba9f8..9fe8a6d49000c 100644 --- a/packages/workflow/test/utils.test.ts +++ b/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); }); });