From a519e577d13f7ff9ff8699dda7bb05fa8aa4f6cc 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: Fri, 31 Mar 2023 15:25:08 +0200 Subject: [PATCH 1/2] fix(core): Augmented objects should use existing property descriptors whenever possible --- packages/workflow/src/AugmentObject.ts | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/packages/workflow/src/AugmentObject.ts b/packages/workflow/src/AugmentObject.ts index a56fd7cb06bcd..5b063a20b8366 100644 --- a/packages/workflow/src/AugmentObject.ts +++ b/packages/workflow/src/AugmentObject.ts @@ -1,6 +1,9 @@ import type { IDataObject } from './Interfaces'; +const defaultPropertyDescriptor = Object.freeze({ enumerable: true, configurable: true }); + const augmentedObjects = new WeakSet(); + function augment(value: T): T { if ( typeof value !== 'object' || @@ -54,7 +57,8 @@ export function augmentArray(data: T[]): T[] { if (key === 'length') { return Reflect.getOwnPropertyDescriptor(newData, key); } - return { configurable: true, enumerable: true }; + + return Object.getOwnPropertyDescriptor(data, key) ?? defaultPropertyDescriptor; }, has(target, key) { return Reflect.has(newData !== undefined ? newData : target, key); @@ -130,18 +134,17 @@ export function augmentObject(data: T): T { return true; }, + ownKeys(target) { - return [...new Set([...Reflect.ownKeys(target), ...Object.keys(newData)])].filter( + const originalKeys = Reflect.ownKeys(target); + const newKeys = Object.keys(newData); + return [...new Set([...originalKeys, ...newKeys])].filter( (key) => deletedProperties.indexOf(key) === -1, ); }, - // eslint-disable-next-line @typescript-eslint/no-unused-vars - getOwnPropertyDescriptor(k) { - return { - enumerable: true, - configurable: true, - }; + getOwnPropertyDescriptor(target, key) { + return Object.getOwnPropertyDescriptor(data, key) ?? defaultPropertyDescriptor; }, }); } From 2c011870ba5e81c50667690ecfba69179f0c2fde 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: Mon, 3 Apr 2023 16:48:22 +0200 Subject: [PATCH 2/2] add a test for non-enumerable keys --- packages/workflow/test/AugmentObject.test.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/packages/workflow/test/AugmentObject.test.ts b/packages/workflow/test/AugmentObject.test.ts index f30989aec3b1c..e9fe6ee948697 100644 --- a/packages/workflow/test/AugmentObject.test.ts +++ b/packages/workflow/test/AugmentObject.test.ts @@ -520,5 +520,13 @@ describe('AugmentObject', () => { expect(timeAugmented).toBeLessThan(timeCopied); }); + + test('should ignore non-enumerable keys', () => { + const originalObject = { a: 1, b: 2 }; + Object.defineProperty(originalObject, '__hiddenProp', { enumerable: false }); + + const augmentedObject = augmentObject(originalObject); + expect(Object.keys(augmentedObject)).toEqual(['a', 'b']); + }); }); });