Adjust objectProperties with key in that check (#1541)

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));
 });

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<string, unknown>)[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)

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 }));
 });