From 7b25d550e15d9f51c62cd783fbe195573cc9ae84 Mon Sep 17 00:00:00 2001 From: Jan Oberhauser Date: Wed, 15 Feb 2023 11:36:37 +0100 Subject: [PATCH 1/9] feat(core): Augment data instead of copying it --- packages/workflow/src/AugmentObject.ts | 80 +++++ packages/workflow/test/AugmentObject.test.ts | 331 +++++++++++++++++++ 2 files changed, 411 insertions(+) create mode 100644 packages/workflow/src/AugmentObject.ts create mode 100644 packages/workflow/test/AugmentObject.test.ts diff --git a/packages/workflow/src/AugmentObject.ts b/packages/workflow/src/AugmentObject.ts new file mode 100644 index 0000000000000..ab1c97be5be02 --- /dev/null +++ b/packages/workflow/src/AugmentObject.ts @@ -0,0 +1,80 @@ +import type { IDataObject } from './Interfaces'; + +export function augmentArray(data: T[]): T[] { + return new Proxy(data, {}); +} + +export function augmentObject(data: T): T { + const newData = {} as IDataObject; + const deletedProperies: Array = []; + + return new Proxy(data, { + get(target, key, receiver): unknown { + if (deletedProperies.indexOf(key) !== -1) { + return undefined; + } + + if (newData[key as string]) { + return newData[key as string]; + } + + // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment + const value = Reflect.get(target, key, receiver); + + if (typeof value === 'object') { + if (Array.isArray(value)) { + newData[key as string] = augmentArray(value); + } else { + newData[key as string] = augmentObject(value as IDataObject); + } + + return newData[key as string]; + } + + return value as string; + }, + deleteProperty(target, key) { + if (key in newData) { + delete newData[key as string]; + } + if (key in target) { + deletedProperies.push(key); + } + + return true; + }, + set(target, key, newValue: unknown) { + if (newValue === undefined) { + if (key in newData) { + delete newData[key as string]; + } + if (key in target) { + deletedProperies.push(key); + } + return true; + } + + newData[key as string] = newValue as IDataObject; + + const deleteIndex = deletedProperies.indexOf(key); + if (deleteIndex !== -1) { + deletedProperies.splice(deleteIndex, 1); + } + + return true; + }, + ownKeys(target) { + return [...new Set([...Reflect.ownKeys(target), ...Object.keys(newData)])].filter( + (key) => deletedProperies.indexOf(key) === -1, + ); + }, + + // eslint-disable-next-line @typescript-eslint/no-unused-vars + getOwnPropertyDescriptor(k) { + return { + enumerable: true, + configurable: true, + }; + }, + }); +} diff --git a/packages/workflow/test/AugmentObject.test.ts b/packages/workflow/test/AugmentObject.test.ts new file mode 100644 index 0000000000000..800b07ded927b --- /dev/null +++ b/packages/workflow/test/AugmentObject.test.ts @@ -0,0 +1,331 @@ +import type { IDataObject } from '@/Interfaces'; +import { augmentObject } from '@/AugmentObject'; +import { deepCopy } from '@/utils'; + +describe('AugmentObject', () => { + describe('augmentObject', () => { + test('should work with simple values on first level', () => { + const originalObject: IDataObject = { + 1: 11, + 2: '22', + a: 111, + b: '222', + }; + const copyOriginal = JSON.parse(JSON.stringify(originalObject)); + + const augmentedObject = augmentObject(originalObject); + + augmentedObject[1] = 911; + expect(originalObject[1]).toEqual(11); + expect(augmentedObject[1]).toEqual(911); + + augmentedObject[2] = '922'; + expect(originalObject[2]).toEqual('22'); + expect(augmentedObject[2]).toEqual('922'); + + augmentedObject.a = 9111; + expect(originalObject.a).toEqual(111); + expect(augmentedObject.a).toEqual(9111); + + augmentedObject.b = '9222'; + expect(originalObject.b).toEqual('222'); + expect(augmentedObject.b).toEqual('9222'); + + augmentedObject.c = 3; + + expect(originalObject).toEqual(copyOriginal); + + expect(augmentedObject).toEqual({ + 1: 911, + 2: '922', + a: 9111, + b: '9222', + c: 3, + }); + }); + + test('should work with simple values on sub-level', () => { + const originalObject = { + a: { + b: { + cc: '3', + }, + bb: '2', + }, + aa: '1', + }; + const copyOriginal = JSON.parse(JSON.stringify(originalObject)); + + const augmentedObject = augmentObject(originalObject); + + // @ts-ignore + augmentedObject.a!.bb = '92'; + expect(originalObject.a.bb).toEqual('2'); + // @ts-ignore + expect(augmentedObject.a!.bb!).toEqual('92'); + + // @ts-ignore + augmentedObject.a!.b!.cc = '93'; + expect(originalObject.a.b.cc).toEqual('3'); + // @ts-ignore + expect(augmentedObject.a!.b!.cc).toEqual('93'); + // expect(augmentedObject[1]).toEqual(911); + + // @ts-ignore + augmentedObject.a!.b!.ccc = { + d: '4', + }; + + // @ts-ignore + expect(augmentedObject.a!.b!.ccc).toEqual({ d: '4' }); + + // @ts-ignore + augmentedObject.a!.b!.ccc.d = '94'; + // @ts-ignore + expect(augmentedObject.a!.b!.ccc.d).toEqual('94'); + + expect(originalObject).toEqual(copyOriginal); + + expect(augmentedObject).toEqual({ + a: { + b: { + cc: '93', + ccc: { + d: '94', + }, + }, + bb: '92', + }, + aa: '1', + }); + }); + + test('should work with complex values on first level', () => { + const originalObject = { + a: { + b: { + cc: '3', + }, + bb: '2', + }, + aa: '1', + }; + const copyOriginal = JSON.parse(JSON.stringify(originalObject)); + + const augmentedObject = augmentObject(originalObject); + + augmentedObject.a = { new: 'NEW' }; + expect(originalObject.a).toEqual({ + b: { + cc: '3', + }, + bb: '2', + }); + expect(augmentedObject.a).toEqual({ new: 'NEW' }); + + augmentedObject.aa = '11'; + expect(originalObject.aa).toEqual('1'); + expect(augmentedObject.aa).toEqual('11'); + + augmentedObject.aaa = { + bbb: { + ccc: '333', + }, + }; + + expect(originalObject).toEqual(copyOriginal); + expect(augmentedObject).toEqual({ + a: { + new: 'NEW', + }, + aa: '11', + aaa: { + bbb: { + ccc: '333', + }, + }, + }); + }); + + test('should work with delete and reset', () => { + const originalObject = { + a: { + b: { + c: { + d: '4' as string | undefined, + } as { d?: string; dd?: string } | undefined, + cc: '3' as string | undefined, + }, + bb: '2' as string | undefined, + }, + aa: '1' as string | undefined, + }; + const copyOriginal = JSON.parse(JSON.stringify(originalObject)); + + const augmentedObject = augmentObject(originalObject); + + // Remove multiple values + delete augmentedObject.a.b.c!.d; + expect(augmentedObject.a.b.c!.d).toEqual(undefined); + expect(originalObject.a.b.c!.d).toEqual('4'); + + expect(augmentedObject).toEqual({ + a: { + b: { + c: {}, + cc: '3', + }, + bb: '2', + }, + aa: '1', + }); + expect(originalObject).toEqual(copyOriginal); + + delete augmentedObject.a.b.c; + expect(augmentedObject.a.b.c).toEqual(undefined); + expect(originalObject.a.b.c).toEqual({ d: '4' }); + + expect(augmentedObject).toEqual({ + a: { + b: { + cc: '3', + }, + bb: '2', + }, + aa: '1', + }); + expect(originalObject).toEqual(copyOriginal); + + // Set deleted values again + augmentedObject.a.b.c = { dd: '444' }; + expect(augmentedObject.a.b.c).toEqual({ dd: '444' }); + expect(originalObject).toEqual(copyOriginal); + + augmentedObject.a.b.c.d = '44'; + expect(augmentedObject).toEqual({ + a: { + b: { + c: { + d: '44', + dd: '444', + }, + cc: '3', + }, + bb: '2', + }, + aa: '1', + }); + expect(originalObject).toEqual(copyOriginal); + }); + + // Is almost identical to above test + test('should work with setting to undefined and reset', () => { + const originalObject = { + a: { + b: { + c: { + d: '4' as string | undefined, + } as { d?: string; dd?: string } | undefined, + cc: '3' as string | undefined, + }, + bb: '2' as string | undefined, + }, + aa: '1' as string | undefined, + }; + const copyOriginal = JSON.parse(JSON.stringify(originalObject)); + + const augmentedObject = augmentObject(originalObject); + + // Remove multiple values + augmentedObject.a.b.c!.d = undefined; + expect(augmentedObject.a.b.c!.d).toEqual(undefined); + expect(originalObject.a.b.c!.d).toEqual('4'); + + expect(augmentedObject).toEqual({ + a: { + b: { + c: {}, + cc: '3', + }, + bb: '2', + }, + aa: '1', + }); + expect(originalObject).toEqual(copyOriginal); + + augmentedObject.a.b.c = undefined; + expect(augmentedObject.a.b.c).toEqual(undefined); + expect(originalObject.a.b.c).toEqual({ d: '4' }); + + expect(augmentedObject).toEqual({ + a: { + b: { + cc: '3', + }, + bb: '2', + }, + aa: '1', + }); + expect(originalObject).toEqual(copyOriginal); + + // Set deleted values again + augmentedObject.a.b.c = { dd: '444' }; + expect(augmentedObject.a.b.c).toEqual({ dd: '444' }); + expect(originalObject).toEqual(copyOriginal); + + augmentedObject.a.b.c.d = '44'; + expect(augmentedObject).toEqual({ + a: { + b: { + c: { + d: '44', + dd: '444', + }, + cc: '3', + }, + bb: '2', + }, + aa: '1', + }); + expect(originalObject).toEqual(copyOriginal); + }); + + test('should be faster than doing a deepCopy', () => { + const iterations = 100; + const originalObject: IDataObject = { + a: { + b: { + c: { + d: { + e: { + f: 12345, + }, + }, + }, + }, + }, + }; + for (let i = 0; i < 10; i++) { + originalObject[i.toString()] = deepCopy(originalObject); + } + + let startTime = new Date().getTime(); + for (let i = 0; i < iterations; i++) { + const augmentedObject = augmentObject(originalObject); + augmentedObject.a.b.c.d.e.f++; + } + const timeAugmented = new Date().getTime() - startTime; + + for (let i = 0; i < iterations; i++) { + const copiedObject = deepCopy(originalObject); + copiedObject.a.b.c.d.e.f++; + } + const timeCopied = new Date().getTime() - startTime; + + console.log('timeAugmented', timeAugmented); + console.log('timeCopied', timeCopied); + + expect(timeAugmented).toBeLessThan(timeCopied); + }); + }); +}); From ea249196a28eeca063c12372fb55e14033715193 Mon Sep 17 00:00:00 2001 From: Jan Oberhauser Date: Wed, 15 Feb 2023 11:45:31 +0100 Subject: [PATCH 2/9] :zap: Use augmentObject --- packages/workflow/src/WorkflowDataProxy.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/workflow/src/WorkflowDataProxy.ts b/packages/workflow/src/WorkflowDataProxy.ts index ec7224fe48c50..8aa9641734327 100644 --- a/packages/workflow/src/WorkflowDataProxy.ts +++ b/packages/workflow/src/WorkflowDataProxy.ts @@ -28,7 +28,7 @@ import type { import * as NodeHelpers from './NodeHelpers'; import { ExpressionError } from './ExpressionError'; import type { Workflow } from './Workflow'; -import { deepCopy } from './utils'; +import { augmentObject } from './AugmentObject'; export function isResourceLocatorValue(value: unknown): value is INodeParameterResourceLocator { return Boolean( @@ -96,11 +96,13 @@ export class WorkflowDataProxy { this.workflow = workflow; this.runExecutionData = isScriptingNode(activeNodeName, workflow) - ? deepCopy(runExecutionData) + ? runExecutionData !== null + ? augmentObject(runExecutionData) + : null : runExecutionData; this.connectionInputData = isScriptingNode(activeNodeName, workflow) - ? deepCopy(connectionInputData) + ? augmentObject(connectionInputData) : connectionInputData; this.defaultReturnRunIndex = defaultReturnRunIndex; From 2010798b8e52fec298d355cc4388c6cececce866 Mon Sep 17 00:00:00 2001 From: Jan Oberhauser Date: Fri, 17 Feb 2023 18:56:23 +0100 Subject: [PATCH 3/9] :zap: Add array support --- packages/workflow/src/AugmentObject.ts | 65 +++++- packages/workflow/test/AugmentObject.test.ts | 197 ++++++++++++++++++- 2 files changed, 251 insertions(+), 11 deletions(-) diff --git a/packages/workflow/src/AugmentObject.ts b/packages/workflow/src/AugmentObject.ts index ab1c97be5be02..0047821b1786c 100644 --- a/packages/workflow/src/AugmentObject.ts +++ b/packages/workflow/src/AugmentObject.ts @@ -1,7 +1,70 @@ import type { IDataObject } from './Interfaces'; +import util from 'util'; export function augmentArray(data: T[]): T[] { - return new Proxy(data, {}); + let newData: unknown[] | undefined = undefined; + + function getData(): unknown[] { + if (newData === undefined) { + newData = [...data]; + } + return newData; + } + + return new Proxy(data, { + deleteProperty(target, key) { + return Reflect.deleteProperty(getData(), key); + }, + get(target, key, receiver): unknown { + const value = Reflect.get(newData !== undefined ? newData : target, key, receiver) as unknown; + + if (typeof value === 'object') { + if (util.types.isProxy(value)) { + return Reflect.get(newData as unknown[], key); + } + + newData = getData(); + + if (Array.isArray(value)) { + Reflect.set(newData, key, augmentArray(value)); + } else { + // eslint-disable-next-line @typescript-eslint/no-use-before-define + Reflect.set(newData, key, augmentObject(value as IDataObject)); + } + + return Reflect.get(newData, key); + } + + return value; + }, + getOwnPropertyDescriptor(target, key) { + if (newData === undefined) { + return Reflect.getOwnPropertyDescriptor(target, key); + } + + if (key === 'length') { + return Reflect.getOwnPropertyDescriptor(newData, key); + } + return { configurable: true, enumerable: true }; + }, + has(target, key) { + return Reflect.has(newData !== undefined ? newData : target, key); + }, + ownKeys(target) { + return Reflect.ownKeys(newData !== undefined ? newData : target); + }, + set(target, key: string, newValue: unknown) { + if (typeof newValue === 'object') { + // Always proxy all objects. Like that we can check in get simply if it + // is a proxy and it does then not matter if it was already there from the + // beginning and it got proxied at some point or set later and so theoretically + // does not have to get proxied + newValue = new Proxy(newValue as object, {}); + } + + return Reflect.set(getData(), key, newValue); + }, + }); } export function augmentObject(data: T): T { diff --git a/packages/workflow/test/AugmentObject.test.ts b/packages/workflow/test/AugmentObject.test.ts index 800b07ded927b..970b49fef914a 100644 --- a/packages/workflow/test/AugmentObject.test.ts +++ b/packages/workflow/test/AugmentObject.test.ts @@ -1,8 +1,193 @@ import type { IDataObject } from '@/Interfaces'; -import { augmentObject } from '@/AugmentObject'; +import { augmentArray, augmentObject } from '@/AugmentObject'; import { deepCopy } from '@/utils'; describe('AugmentObject', () => { + describe('augmentArray', () => { + test('should work with arrays', () => { + const originalObject = [1, 2, 3, 4]; + const copyOriginal = JSON.parse(JSON.stringify(originalObject)); + + const augmentedObject = augmentArray(originalObject); + + expect(augmentedObject.push(5)).toEqual(5); + expect(augmentedObject).toEqual([1, 2, 3, 4, 5]); + expect(originalObject).toEqual(copyOriginal); + + expect(augmentedObject.pop()).toEqual(5); + expect(augmentedObject).toEqual([1, 2, 3, 4]); + expect(originalObject).toEqual(copyOriginal); + + expect(augmentedObject.shift()).toEqual(1); + expect(augmentedObject).toEqual([2, 3, 4]); + expect(originalObject).toEqual(copyOriginal); + + expect(augmentedObject.unshift(1)).toEqual(4); + expect(augmentedObject).toEqual([1, 2, 3, 4]); + expect(originalObject).toEqual(copyOriginal); + + expect(augmentedObject.splice(1, 1)).toEqual([2]); + expect(augmentedObject).toEqual([1, 3, 4]); + expect(originalObject).toEqual(copyOriginal); + + expect(augmentedObject.slice(1)).toEqual([3, 4]); + expect(originalObject).toEqual(copyOriginal); + + expect(augmentedObject.reverse()).toEqual([4, 3, 1]); + expect(originalObject).toEqual(copyOriginal); + }); + + test('should work with arrays on any level', () => { + const originalObject = { + a: { + b: { + c: [ + { + a3: { + b3: { + c3: '03', + }, + }, + aa3: '01', + }, + { + a3: { + b3: { + c3: '13', + }, + }, + aa3: '11', + }, + ], + }, + }, + aa: [ + { + a3: { + b3: '2', + }, + aa3: '1', + }, + ], + }; + const copyOriginal = JSON.parse(JSON.stringify(originalObject)); + + const augmentedObject = augmentObject(originalObject); + + // On first level + augmentedObject.aa[0].a3.b3 = '22'; + expect(augmentedObject.aa[0].a3.b3).toEqual('22'); + expect(originalObject.aa[0].a3.b3).toEqual('2'); + + // Make sure that also array operations as push and length work as expected + // On lower levels + augmentedObject.a.b.c[0].a3!.b3.c3 = '033'; + expect(augmentedObject.a.b.c[0].a3!.b3.c3).toEqual('033'); + expect(originalObject.a.b.c[0].a3!.b3.c3).toEqual('03'); + + augmentedObject.a.b.c[1].a3!.b3.c3 = '133'; + expect(augmentedObject.a.b.c[1].a3!.b3.c3).toEqual('133'); + expect(originalObject.a.b.c[1].a3!.b3.c3).toEqual('13'); + + augmentedObject.a.b.c.push({ + a3: { + b3: { + c3: '23', + }, + }, + aa3: '21', + }); + augmentedObject.a.b.c[2].a3.b3.c3 = '233'; + expect(augmentedObject.a.b.c[2].a3.b3.c3).toEqual('233'); + + augmentedObject.a.b.c[2].a3.b3.c3 = '2333'; + expect(augmentedObject.a.b.c[2].a3.b3.c3).toEqual('2333'); + + expect(originalObject).toEqual(copyOriginal); + + expect(augmentedObject.a.b.c.length).toEqual(3); + + expect(augmentedObject.aa).toEqual([ + { + a3: { + b3: '22', + }, + aa3: '1', + }, + ]); + + expect(augmentedObject.a.b.c).toEqual([ + { + a3: { + b3: { + c3: '033', + }, + }, + aa3: '01', + }, + { + a3: { + b3: { + c3: '133', + }, + }, + aa3: '11', + }, + { + a3: { + b3: { + c3: '2333', + }, + }, + aa3: '21', + }, + ]); + + expect(augmentedObject).toEqual({ + a: { + b: { + c: [ + { + a3: { + b3: { + c3: '033', + }, + }, + aa3: '01', + }, + { + a3: { + b3: { + c3: '133', + }, + }, + aa3: '11', + }, + { + a3: { + b3: { + c3: '2333', + }, + }, + aa3: '21', + }, + ], + }, + }, + aa: [ + { + a3: { + b3: '22', + }, + aa3: '1', + }, + ], + }); + + expect(originalObject).toEqual(copyOriginal); + }); + }); + describe('augmentObject', () => { test('should work with simple values on first level', () => { const originalObject: IDataObject = { @@ -58,18 +243,13 @@ describe('AugmentObject', () => { const augmentedObject = augmentObject(originalObject); - // @ts-ignore - augmentedObject.a!.bb = '92'; + augmentedObject.a.bb = '92'; expect(originalObject.a.bb).toEqual('2'); - // @ts-ignore expect(augmentedObject.a!.bb!).toEqual('92'); - // @ts-ignore augmentedObject.a!.b!.cc = '93'; expect(originalObject.a.b.cc).toEqual('3'); - // @ts-ignore expect(augmentedObject.a!.b!.cc).toEqual('93'); - // expect(augmentedObject[1]).toEqual(911); // @ts-ignore augmentedObject.a!.b!.ccc = { @@ -322,9 +502,6 @@ describe('AugmentObject', () => { } const timeCopied = new Date().getTime() - startTime; - console.log('timeAugmented', timeAugmented); - console.log('timeCopied', timeCopied); - expect(timeAugmented).toBeLessThan(timeCopied); }); }); From ec95cf18e9858b8de375f3d2c28af27e2b930c40 Mon Sep 17 00:00:00 2001 From: Jan Oberhauser Date: Sat, 18 Feb 2023 22:13:32 +0100 Subject: [PATCH 4/9] :zap: Improve and fix issues with null --- packages/workflow/src/AugmentObject.ts | 12 ++++---- packages/workflow/src/WorkflowDataProxy.ts | 4 +-- packages/workflow/test/AugmentObject.test.ts | 31 ++++++++++++-------- 3 files changed, 26 insertions(+), 21 deletions(-) diff --git a/packages/workflow/src/AugmentObject.ts b/packages/workflow/src/AugmentObject.ts index 0047821b1786c..cde2063f29805 100644 --- a/packages/workflow/src/AugmentObject.ts +++ b/packages/workflow/src/AugmentObject.ts @@ -19,8 +19,8 @@ export function augmentArray(data: T[]): T[] { const value = Reflect.get(newData !== undefined ? newData : target, key, receiver) as unknown; if (typeof value === 'object') { - if (util.types.isProxy(value)) { - return Reflect.get(newData as unknown[], key); + if (value === null || util.types.isProxy(value)) { + return value; } newData = getData(); @@ -54,12 +54,12 @@ export function augmentArray(data: T[]): T[] { return Reflect.ownKeys(newData !== undefined ? newData : target); }, set(target, key: string, newValue: unknown) { - if (typeof newValue === 'object') { + if (newValue !== null && typeof newValue === 'object') { // Always proxy all objects. Like that we can check in get simply if it // is a proxy and it does then not matter if it was already there from the // beginning and it got proxied at some point or set later and so theoretically // does not have to get proxied - newValue = new Proxy(newValue as object, {}); + newValue = new Proxy(newValue, {}); } return Reflect.set(getData(), key, newValue); @@ -77,14 +77,14 @@ export function augmentObject(data: T): T { return undefined; } - if (newData[key as string]) { + if (newData[key as string] !== undefined) { return newData[key as string]; } // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment const value = Reflect.get(target, key, receiver); - if (typeof value === 'object') { + if (value !== null && typeof value === 'object') { if (Array.isArray(value)) { newData[key as string] = augmentArray(value); } else { diff --git a/packages/workflow/src/WorkflowDataProxy.ts b/packages/workflow/src/WorkflowDataProxy.ts index 8aa9641734327..8b6ede1915169 100644 --- a/packages/workflow/src/WorkflowDataProxy.ts +++ b/packages/workflow/src/WorkflowDataProxy.ts @@ -28,7 +28,7 @@ import type { import * as NodeHelpers from './NodeHelpers'; import { ExpressionError } from './ExpressionError'; import type { Workflow } from './Workflow'; -import { augmentObject } from './AugmentObject'; +import { augmentArray, augmentObject } from './AugmentObject'; export function isResourceLocatorValue(value: unknown): value is INodeParameterResourceLocator { return Boolean( @@ -102,7 +102,7 @@ export class WorkflowDataProxy { : runExecutionData; this.connectionInputData = isScriptingNode(activeNodeName, workflow) - ? augmentObject(connectionInputData) + ? augmentArray(connectionInputData) : connectionInputData; this.defaultReturnRunIndex = defaultReturnRunIndex; diff --git a/packages/workflow/test/AugmentObject.test.ts b/packages/workflow/test/AugmentObject.test.ts index 970b49fef914a..d1c58466066df 100644 --- a/packages/workflow/test/AugmentObject.test.ts +++ b/packages/workflow/test/AugmentObject.test.ts @@ -5,35 +5,35 @@ import { deepCopy } from '@/utils'; describe('AugmentObject', () => { describe('augmentArray', () => { test('should work with arrays', () => { - const originalObject = [1, 2, 3, 4]; + const originalObject = [1, 2, 3, 4, null]; const copyOriginal = JSON.parse(JSON.stringify(originalObject)); const augmentedObject = augmentArray(originalObject); - expect(augmentedObject.push(5)).toEqual(5); - expect(augmentedObject).toEqual([1, 2, 3, 4, 5]); + expect(augmentedObject.push(5)).toEqual(6); + expect(augmentedObject).toEqual([1, 2, 3, 4, null, 5]); expect(originalObject).toEqual(copyOriginal); expect(augmentedObject.pop()).toEqual(5); - expect(augmentedObject).toEqual([1, 2, 3, 4]); + expect(augmentedObject).toEqual([1, 2, 3, 4, null]); expect(originalObject).toEqual(copyOriginal); expect(augmentedObject.shift()).toEqual(1); - expect(augmentedObject).toEqual([2, 3, 4]); + expect(augmentedObject).toEqual([2, 3, 4, null]); expect(originalObject).toEqual(copyOriginal); - expect(augmentedObject.unshift(1)).toEqual(4); - expect(augmentedObject).toEqual([1, 2, 3, 4]); + expect(augmentedObject.unshift(1)).toEqual(5); + expect(augmentedObject).toEqual([1, 2, 3, 4, null]); expect(originalObject).toEqual(copyOriginal); expect(augmentedObject.splice(1, 1)).toEqual([2]); - expect(augmentedObject).toEqual([1, 3, 4]); + expect(augmentedObject).toEqual([1, 3, 4, null]); expect(originalObject).toEqual(copyOriginal); - expect(augmentedObject.slice(1)).toEqual([3, 4]); + expect(augmentedObject.slice(1)).toEqual([3, 4, null]); expect(originalObject).toEqual(copyOriginal); - expect(augmentedObject.reverse()).toEqual([4, 3, 1]); + expect(augmentedObject.reverse()).toEqual([null, 4, 3, 1]); expect(originalObject).toEqual(copyOriginal); }); @@ -45,7 +45,7 @@ describe('AugmentObject', () => { { a3: { b3: { - c3: '03', + c3: '03' as string | null, }, }, aa3: '01', @@ -103,6 +103,9 @@ describe('AugmentObject', () => { augmentedObject.a.b.c[2].a3.b3.c3 = '2333'; expect(augmentedObject.a.b.c[2].a3.b3.c3).toEqual('2333'); + augmentedObject.a.b.c[2].a3.b3.c3 = null; + expect(augmentedObject.a.b.c[2].a3.b3.c3).toEqual(null); + expect(originalObject).toEqual(copyOriginal); expect(augmentedObject.a.b.c.length).toEqual(3); @@ -136,7 +139,7 @@ describe('AugmentObject', () => { { a3: { b3: { - c3: '2333', + c3: null, }, }, aa3: '21', @@ -166,7 +169,7 @@ describe('AugmentObject', () => { { a3: { b3: { - c3: '2333', + c3: null, }, }, aa3: '21', @@ -285,6 +288,7 @@ describe('AugmentObject', () => { a: { b: { cc: '3', + c2: null, }, bb: '2', }, @@ -297,6 +301,7 @@ describe('AugmentObject', () => { augmentedObject.a = { new: 'NEW' }; expect(originalObject.a).toEqual({ b: { + c2: null, cc: '3', }, bb: '2', From 94cc9b610349e31abe7b6401f4dabc16fc70c589 Mon Sep 17 00:00:00 2001 From: Jan Oberhauser Date: Sun, 19 Feb 2023 17:17:51 +0100 Subject: [PATCH 5/9] :zap: Improve speed test --- packages/workflow/test/AugmentObject.test.ts | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/packages/workflow/test/AugmentObject.test.ts b/packages/workflow/test/AugmentObject.test.ts index d1c58466066df..e038779f1a607 100644 --- a/packages/workflow/test/AugmentObject.test.ts +++ b/packages/workflow/test/AugmentObject.test.ts @@ -497,13 +497,18 @@ describe('AugmentObject', () => { let startTime = new Date().getTime(); for (let i = 0; i < iterations; i++) { const augmentedObject = augmentObject(originalObject); - augmentedObject.a.b.c.d.e.f++; + for (let i = 0; i < 5000; i++) { + augmentedObject.a!.b.c.d.e.f++; + } } const timeAugmented = new Date().getTime() - startTime; + startTime = new Date().getTime(); for (let i = 0; i < iterations; i++) { const copiedObject = deepCopy(originalObject); - copiedObject.a.b.c.d.e.f++; + for (let i = 0; i < 5000; i++) { + copiedObject.a!.b.c.d.e.f++; + } } const timeCopied = new Date().getTime() - startTime; From 86fe1f7dfdfc7f3f081656136d63887774dd8110 Mon Sep 17 00:00:00 2001 From: Jan Oberhauser Date: Mon, 6 Mar 2023 17:47:21 +0100 Subject: [PATCH 6/9] :zap: Use new augmentation only where it provides speed/memory advantage --- packages/workflow/src/WorkflowDataProxy.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/workflow/src/WorkflowDataProxy.ts b/packages/workflow/src/WorkflowDataProxy.ts index 8b6ede1915169..5b93dda409757 100644 --- a/packages/workflow/src/WorkflowDataProxy.ts +++ b/packages/workflow/src/WorkflowDataProxy.ts @@ -28,7 +28,8 @@ import type { import * as NodeHelpers from './NodeHelpers'; import { ExpressionError } from './ExpressionError'; import type { Workflow } from './Workflow'; -import { augmentArray, augmentObject } from './AugmentObject'; +import { augmentObject } from './AugmentObject'; +import { deepCopy } from './utils'; export function isResourceLocatorValue(value: unknown): value is INodeParameterResourceLocator { return Boolean( @@ -101,8 +102,10 @@ export class WorkflowDataProxy { : null : runExecutionData; + // This is slower with augmentArray than deepCopy because literally all + // data gets always accessed, so leave for now this.connectionInputData = isScriptingNode(activeNodeName, workflow) - ? augmentArray(connectionInputData) + ? deepCopy(connectionInputData) : connectionInputData; this.defaultReturnRunIndex = defaultReturnRunIndex; From 8289134b07c5a62f0c8fc5bb158081fd9b49a1cf 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: Thu, 16 Mar 2023 12:01:31 +0100 Subject: [PATCH 7/9] correct a typo --- packages/workflow/src/AugmentObject.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/workflow/src/AugmentObject.ts b/packages/workflow/src/AugmentObject.ts index cde2063f29805..095caca73284f 100644 --- a/packages/workflow/src/AugmentObject.ts +++ b/packages/workflow/src/AugmentObject.ts @@ -69,11 +69,11 @@ export function augmentArray(data: T[]): T[] { export function augmentObject(data: T): T { const newData = {} as IDataObject; - const deletedProperies: Array = []; + const deletedProperties: Array = []; return new Proxy(data, { get(target, key, receiver): unknown { - if (deletedProperies.indexOf(key) !== -1) { + if (deletedProperties.indexOf(key) !== -1) { return undefined; } @@ -101,7 +101,7 @@ export function augmentObject(data: T): T { delete newData[key as string]; } if (key in target) { - deletedProperies.push(key); + deletedProperties.push(key); } return true; @@ -112,23 +112,23 @@ export function augmentObject(data: T): T { delete newData[key as string]; } if (key in target) { - deletedProperies.push(key); + deletedProperties.push(key); } return true; } newData[key as string] = newValue as IDataObject; - const deleteIndex = deletedProperies.indexOf(key); + const deleteIndex = deletedProperties.indexOf(key); if (deleteIndex !== -1) { - deletedProperies.splice(deleteIndex, 1); + deletedProperties.splice(deleteIndex, 1); } return true; }, ownKeys(target) { return [...new Set([...Reflect.ownKeys(target), ...Object.keys(newData)])].filter( - (key) => deletedProperies.indexOf(key) === -1, + (key) => deletedProperties.indexOf(key) === -1, ); }, From c0fa7f5b85745c96ef51a7c4ba5f0365dcf860ab 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: Thu, 16 Mar 2023 12:02:05 +0100 Subject: [PATCH 8/9] force type the key in the handler signature instead of using `as string` multiple times --- packages/workflow/src/AugmentObject.ts | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/packages/workflow/src/AugmentObject.ts b/packages/workflow/src/AugmentObject.ts index 095caca73284f..cc65285601cc6 100644 --- a/packages/workflow/src/AugmentObject.ts +++ b/packages/workflow/src/AugmentObject.ts @@ -12,10 +12,10 @@ export function augmentArray(data: T[]): T[] { } return new Proxy(data, { - deleteProperty(target, key) { + deleteProperty(target, key: string) { return Reflect.deleteProperty(getData(), key); }, - get(target, key, receiver): unknown { + get(target, key: string, receiver): unknown { const value = Reflect.get(newData !== undefined ? newData : target, key, receiver) as unknown; if (typeof value === 'object') { @@ -72,13 +72,13 @@ export function augmentObject(data: T): T { const deletedProperties: Array = []; return new Proxy(data, { - get(target, key, receiver): unknown { + get(target, key: string, receiver): unknown { if (deletedProperties.indexOf(key) !== -1) { return undefined; } - if (newData[key as string] !== undefined) { - return newData[key as string]; + if (newData[key] !== undefined) { + return newData[key]; } // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment @@ -86,19 +86,19 @@ export function augmentObject(data: T): T { if (value !== null && typeof value === 'object') { if (Array.isArray(value)) { - newData[key as string] = augmentArray(value); + newData[key] = augmentArray(value); } else { - newData[key as string] = augmentObject(value as IDataObject); + newData[key] = augmentObject(value as IDataObject); } - return newData[key as string]; + return newData[key]; } return value as string; }, - deleteProperty(target, key) { + deleteProperty(target, key: string) { if (key in newData) { - delete newData[key as string]; + delete newData[key]; } if (key in target) { deletedProperties.push(key); @@ -106,10 +106,10 @@ export function augmentObject(data: T): T { return true; }, - set(target, key, newValue: unknown) { + set(target, key: string, newValue: unknown) { if (newValue === undefined) { if (key in newData) { - delete newData[key as string]; + delete newData[key]; } if (key in target) { deletedProperties.push(key); @@ -117,7 +117,7 @@ export function augmentObject(data: T): T { return true; } - newData[key as string] = newValue as IDataObject; + newData[key] = newValue as IDataObject; const deleteIndex = deletedProperties.indexOf(key); if (deleteIndex !== -1) { From 12250f7e9f9a83a37017ae171ffd311f3f1d1a8f 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: Thu, 16 Mar 2023 12:45:00 +0100 Subject: [PATCH 9/9] use `augmentArray` for `connectionInputData` --- packages/workflow/src/WorkflowDataProxy.ts | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/packages/workflow/src/WorkflowDataProxy.ts b/packages/workflow/src/WorkflowDataProxy.ts index 0abad5d26a6ba..31b0e8c66635d 100644 --- a/packages/workflow/src/WorkflowDataProxy.ts +++ b/packages/workflow/src/WorkflowDataProxy.ts @@ -28,8 +28,7 @@ import type { import * as NodeHelpers from './NodeHelpers'; import { ExpressionError } from './ExpressionError'; import type { Workflow } from './Workflow'; -import { augmentObject } from './AugmentObject'; -import { deepCopy } from './utils'; +import { augmentArray, augmentObject } from './AugmentObject'; export function isResourceLocatorValue(value: unknown): value is INodeParameterResourceLocator { return Boolean( @@ -102,10 +101,8 @@ export class WorkflowDataProxy { : null : runExecutionData; - // This is slower with augmentArray than deepCopy because literally all - // data gets always accessed, so leave for now this.connectionInputData = isScriptingNode(activeNodeName, workflow) - ? deepCopy(connectionInputData) + ? augmentArray(connectionInputData) : connectionInputData; this.defaultReturnRunIndex = defaultReturnRunIndex;