diff --git a/packages/util/src/object/property.spec.ts b/packages/util/src/object/property.spec.ts index 4080ba904..ddb976aff 100644 --- a/packages/util/src/object/property.spec.ts +++ b/packages/util/src/object/property.spec.ts @@ -1,6 +1,7 @@ // Copyright 2017-2022 @polkadot/util authors & contributors // SPDX-License-Identifier: Apache-2.0 +import { performance } from '../test/performance'; import { objectProperties, objectProperty } from '.'; describe('objectProperty/objectProperties', (): void => { @@ -11,6 +12,8 @@ describe('objectProperty/objectProperties', (): void => { objectProperty(test, 'a', () => 2); + expect(Object.prototype.hasOwnProperty.call(test, 'a')).toBe(true); + expect('a' in test).toEqual(true); expect(test.a).toEqual(2); }); @@ -117,4 +120,7 @@ describe('objectProperty/objectProperties', (): void => { expect(test.b).toEqual(123); expect(getter).toHaveBeenCalledWith('b', 1); }); + + performance('objectProperties (obj)', 50_000, [[]], () => objectProperties({}, ['foo', 'bar', 'baz'], (k) => k)); + performance('objectProperties (map)', 50_000, [[]], () => objectProperties(new Map(), ['foo', 'bar', 'baz'], (k) => k)); }); diff --git a/packages/util/src/object/property.ts b/packages/util/src/object/property.ts index e19df81e6..6060e685a 100644 --- a/packages/util/src/object/property.ts +++ b/packages/util/src/object/property.ts @@ -1,19 +1,20 @@ // Copyright 2017-2022 @polkadot/util authors & contributors // SPDX-License-Identifier: Apache-2.0 -import { isUndefined } from '../is/undefined'; - /** * @name objectProperty * @summary Assign a get property on the input object */ export function objectProperty (that: object, key: string, getter: (k: string) => unknown): void { - // We use both the hasOwnProperty as well as isUndefined checks here, since it may be set - // in inherited classes and _Own_ properties refers to the class only, not only parents - if (!Object.prototype.hasOwnProperty.call(that, key) && isUndefined((that as Record)[key])) { + // There are 3 approaches here - + // - Object.prototype.hasOwnProperty.call(that, key) - this only checks the current class, i.e + // will retuirn false if the property is set in the parent class + // - isUndefined(...) - this may yield a false positive when the property is there, but not set. + // Additionally, on pre-defined getters it may make a call + // - key in that - Does not need to be combined with either of the above and checks the full chain + if (!(key in that)) { Object.defineProperty(that, key, { enumerable: true, - // Since we don't use any additional this internally, we can use arrow (unlike lazy) // Unlike in lazy, we always call into the upper function, i.e. this method // does not cache old values (it is expected to be used for dynamic values) get: () => getter(key) diff --git a/packages/util/src/object/spread.spec.ts b/packages/util/src/object/spread.spec.ts index df235663b..e18e8f939 100644 --- a/packages/util/src/object/spread.spec.ts +++ b/packages/util/src/object/spread.spec.ts @@ -17,5 +17,5 @@ describe('objectSpread', (): void => { ).toEqual({ a: 2, b: 2, c: { d: 3, e: 4 } }); }); - performance('objectSpread', 500000, [[]], () => objectSpread({}, { a: 1, b: 2, c: 3 }, { d: 4, e: 7, f: 6 })); + performance('objectSpread', 500_000, [[]], () => objectSpread({}, { a: 1, b: 2, c: 3 }, { d: 4, e: 7, f: 6 })); });