From 6e49da47fa9b668a0d8163c6fbda81c205a33cf6 Mon Sep 17 00:00:00 2001 From: Nev Date: Sat, 7 Nov 2026 16:44:26 -0800 Subject: [PATCH] [Bug] objForEachKey fails if there is a throwing getter for the property #627 --- CHANGELOG.md | 6 + lib/src/object/forEachOwnKey.ts | 48 ++++-- lib/src/object/for_each_key.ts | 61 +++++-- lib/src/polyfills/object/objKeys.ts | 29 +++- lib/test/bundle-size-check.js | 4 +- .../common/object/forEachOwnKeySafe.test.ts | 156 ++++++++++++++++++ .../src/common/object/for_each_key.test.ts | 101 ++++++++++++ lib/test/src/common/polyfills/object.test.ts | 91 ++++++++++ lib/tsconfig.base.json | 1 + 9 files changed, 458 insertions(+), 39 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 15ceac9e..bef6fd67 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,12 @@ ### Bug Fixes +- [#627](https://github.com/nevware21/ts-utils/issues/627) fix(object): `objForEachKey`, `objForEachKeySafe`, `forEachOwnKey` and `forEachOwnKeySafe` no longer abort the enumeration when a property getter throws + - The exception from the getter is caught and the callback is still called for that key with an `undefined` value, so the remaining keys are still visited + - Exceptions thrown by the callback itself are not caught and still propagate + - `polyObjKeys` no longer invokes property getters (so it no longer throws for an object with a throwing getter), matching the native `Object.keys` + - `polyObjEntries` and `polyObjValues` still invoke each getter once and propagate any exception it throws, matching the native `Object.entries` and `Object.values` + # v0.16.0 July 18th, 2026 ## Changelog diff --git a/lib/src/object/forEachOwnKey.ts b/lib/src/object/forEachOwnKey.ts index e5b0c558..47c4880e 100644 --- a/lib/src/object/forEachOwnKey.ts +++ b/lib/src/object/forEachOwnKey.ts @@ -8,9 +8,9 @@ import { arrForEach } from "../array/forEach"; import { isStrictNullOrUndefined } from "../helpers/base"; -import { CALL, NULL_VALUE, UNDEF_VALUE } from "../internal/constants"; +import { CALL, UNDEF_VALUE } from "../internal/constants"; +import { _objForEachKey } from "./for_each_key"; import { objGetOwnPropertySymbols } from "./get_own_property"; -import { objHasOwn } from "./has_own"; import { isUnsafePropKey } from "./isUnsafePropKey"; import { _objPropertyIsEnumerable } from "./property_is_enumerable"; @@ -24,6 +24,10 @@ import { _objPropertyIsEnumerable } from "./property_is_enumerable"; * **Note:** Unlike {@link objForEachKey}, this helper also iterates enumerable symbol keys via * `Object.getOwnPropertySymbols`. Use {@link forEachOwnKeySafe} when iterating untrusted input * to filter out unsafe keys like `__proto__`, `constructor`, and `prototype`. + * + * If reading a property value throws (for example a getter that throws an exception), the exception + * is caught and the `callbackFn` is still called for that key with an `undefined` value, so a single + * throwing getter does not stop the iteration of the remaining keys. * @since 0.14.0 * @group Object * @typeParam T - The source type. @@ -65,22 +69,24 @@ import { _objPropertyIsEnumerable } from "./property_is_enumerable"; * ``` */ export function forEachOwnKey(theObject: T, callbackfn: (key: PropertyKey, value: T[keyof T]) => void | number, thisArg?: any): void { - if (theObject !== NULL_VALUE && theObject !== UNDEF_VALUE) { - for (const prop in theObject) { - if (objHasOwn(theObject, prop)) { - if (callbackfn[CALL](isStrictNullOrUndefined(thisArg) ? theObject : thisArg, prop, theObject[prop as keyof T]) === -1) { - return; - } - } - } + if (!isStrictNullOrUndefined(theObject)) { + // Resolve the `this` value once, reusing the thisArg variable to keep the minified output small + thisArg = isStrictNullOrUndefined(thisArg) ? theObject : thisArg; + if (_objForEachKey(theObject, callbackfn, thisArg) !== -1) { + arrForEach(objGetOwnPropertySymbols(theObject), (key) => { + if (_objPropertyIsEnumerable(theObject, key)) { + let value: any = UNDEF_VALUE; + try { + value = theObject[key as keyof T]; + } catch (e) { + // A throwing getter, so just pass undefined as the value + } - arrForEach(objGetOwnPropertySymbols(theObject), (key) => { - if (_objPropertyIsEnumerable(theObject, key)) { - if (callbackfn[CALL](isStrictNullOrUndefined(thisArg) ? theObject : thisArg, key, theObject[key as keyof T]) === -1) { - return -1; + // arrForEach stops when -1 is returned + return callbackfn[CALL](thisArg, key, value); } - } - }); + }); + } } } @@ -92,6 +98,9 @@ export function forEachOwnKey(theObject: T, callbackfn: (key: PropertyKey, va * This helper wraps {@link forEachOwnKey} with extra key filtering for safer assignment flows. * Use this instead of {@link forEachOwnKey} whenever keys come from untrusted input (e.g. * user-supplied objects, parsed JSON). + * + * As with {@link forEachOwnKey}, if reading a property value throws (for example a throwing getter) + * the `callbackFn` is called for that key with an `undefined` value and the iteration continues. * @since 0.14.0 * @group Object * @typeParam T - The source type. @@ -145,9 +154,12 @@ export function forEachOwnKey(theObject: T, callbackfn: (key: PropertyKey, va * ``` */ export function forEachOwnKeySafe(theObject: T, callbackfn: (key: PropertyKey, value: T[keyof T]) => void | number, thisArg?: any): void { + // Resolve the `this` value once (not per key), passing it directly rather than via the wrapper's own `this` + // as that would box primitive values (e.g. 0, "", false) when running in non-strict mode + thisArg = isStrictNullOrUndefined(thisArg) ? theObject : thisArg; forEachOwnKey(theObject, (key, value) => { if (!isUnsafePropKey(key)) { - return callbackfn[CALL](isStrictNullOrUndefined(thisArg) ? theObject : thisArg, key, value); + return callbackfn[CALL](thisArg, key, value); } - }, thisArg); + }); } diff --git a/lib/src/object/for_each_key.ts b/lib/src/object/for_each_key.ts index c015b37f..bfcedaab 100644 --- a/lib/src/object/for_each_key.ts +++ b/lib/src/object/for_each_key.ts @@ -7,10 +7,39 @@ */ import { isFunction, isObject, isStrictNullOrUndefined } from "../helpers/base"; -import { CALL } from "../internal/constants"; +import { CALL, UNDEF_VALUE } from "../internal/constants"; import { objHasOwn } from "./has_own"; import { isUnsafePropKey } from "./isUnsafePropKey"; +/** + * @internal + * @ignore + * Shared implementation for {@link objForEachKey} and {@link forEachOwnKey}, iterates the own enumerable + * string keys of the object via a for...in loop. Each value is read inline within a try/catch (rather than + * via a helper function call per key) so that a throwing getter passes `undefined` as the value instead of + * aborting the iteration, while exceptions thrown by the callback still propagate. + * @param theObject - The object to iterate over + * @param callbackfn - The callback to call for each key, returning -1 stops the iteration + * @param ctx - The already resolved `this` value to use when calling the callback + * @returns -1 if the callback requested the iteration to stop, otherwise undefined + */ +export function _objForEachKey(theObject: T, callbackfn: (key: string, value: T[keyof T]) => void | number, ctx: any): number | void { + for (const prop in theObject) { + if (objHasOwn(theObject, prop)) { + let value: any = UNDEF_VALUE; + try { + value = theObject[prop as keyof T]; + } catch (e) { + // A throwing getter, so just pass undefined as the value + } + + if (callbackfn[CALL](ctx, prop, value) === -1) { + return -1; + } + } + } +} + /** * Calls the provided `callbackFn` function once for each key in an object. This is equivalent to `arrForEach(Object.keys(theObject), callbackFn)` or * if not using the array helper `Object.keys(theObject).forEach(callbackFn)` except that this helper avoids creating a temporary array of the object @@ -19,6 +48,10 @@ import { isUnsafePropKey } from "./isUnsafePropKey"; * **Note:** This helper only iterates string (and numeric) keys (via for...in loop) and does NOT include enumerable symbol keys. * If you need to iterate all keys including symbol, use {@link forEachOwnKey} or {@link forEachOwnKeySafe} instead. * + * If reading a property value throws (for example a getter that throws an exception), the exception is caught and the + * `callbackFn` is still called for that key with an `undefined` value, so a single throwing getter does not stop the + * iteration of the remaining keys. + * * Caution: this helper does not filter unsafe keys like `__proto__`, `constructor`, or `prototype`. * If your callback uses the returned key to assign directly to another object (for example * `target[key] = value`), validate keys first (for example with {@link isUnsafePropKey}) or @@ -54,19 +87,21 @@ import { isUnsafePropKey } from "./isUnsafePropKey"; * console.log(key, value); // name Bob (unsafe keys filtered) * }); * ``` + * @example + * ```ts + * // A throwing getter does not stop the iteration + * const obj = { a: 1, get b(): number { throw new Error("fail"); }, c: 3 }; + * objForEachKey(obj, (key, value) => { + * console.log(key, value); // a 1, b undefined, c 3 + * }); + * ``` * @see {@link objForEachKeySafe} for safe iteration that filters unsafe keys * @see {@link forEachOwnKey} for iteration that includes both string and symbol keys * @see {@link forEachOwnKeySafe} for safe iteration with both string and symbol keys */ export function objForEachKey(theObject: T, callbackfn: (key: string, value: T[keyof T]) => void | number, thisArg?: any): void { if (theObject && (isObject(theObject) || isFunction(theObject))) { - for (const prop in theObject) { - if (objHasOwn(theObject, prop)) { - if (callbackfn[CALL](isStrictNullOrUndefined(thisArg) ? theObject : thisArg, prop, theObject[prop as keyof T]) === -1) { - break; - } - } - } + _objForEachKey(theObject, callbackfn, isStrictNullOrUndefined(thisArg) ? theObject : thisArg); } } @@ -76,6 +111,9 @@ export function objForEachKey(theObject: T, callbackfn: (key: string, value: * * Like {@link objForEachKey}, this helper iterates only string keys (via for...in loop) and does NOT include enumerable symbol keys. * If you need to iterate both string and symbol keys, use {@link forEachOwnKeySafe} instead. + * + * As with {@link objForEachKey}, if reading a property value throws (for example a throwing getter) the `callbackFn` + * is called for that key with an `undefined` value and the iteration continues. * @group Object * @since 0.14.0 * @typeParam T - The object type @@ -114,9 +152,12 @@ export function objForEachKey(theObject: T, callbackfn: (key: string, value: * ``` */ export function objForEachKeySafe(theObject: T, callbackfn: (key: string, value: T[keyof T]) => void | number, thisArg?: any): void { + // Resolve the `this` value once (not per key), passing it directly rather than via the wrapper's own `this` + // as that would box primitive values (e.g. 0, "", false) when running in non-strict mode + thisArg = isStrictNullOrUndefined(thisArg) ? theObject : thisArg; objForEachKey(theObject, (key: string, value: T[keyof T]) => { if (!isUnsafePropKey(key)) { - return callbackfn[CALL](isStrictNullOrUndefined(thisArg) ? theObject : thisArg, key, value); + return callbackfn[CALL](thisArg, key, value); } - }, thisArg); + }); } diff --git a/lib/src/polyfills/object/objKeys.ts b/lib/src/polyfills/object/objKeys.ts index 266529db..7671738e 100644 --- a/lib/src/polyfills/object/objKeys.ts +++ b/lib/src/polyfills/object/objKeys.ts @@ -6,17 +6,28 @@ * Licensed under the MIT license. */ -import { isObject } from "../../helpers/base"; +import { isFunction, isObject } from "../../helpers/base"; import { dumpObj } from "../../helpers/diagnostics"; import { throwTypeError } from "../../helpers/throw"; import { _throwIfNullOrUndefined } from "../../internal/throwIf"; -import { objForEachKey } from "../../object/for_each_key"; +import { objHasOwn } from "../../object/has_own"; -function _objIterateEntries(obj: any, mapper: (key: string, value: T) => R): R[] { +/** + * Iterates the own enumerable string keys of the object, passing only the key to the mapper. This intentionally + * does not use objForEachKey (which reads every value and swallows getter exceptions) so that, like the native + * implementations, polyObjKeys never invokes getters and polyObjEntries / polyObjValues invoke each getter once + * and propagate any exception it throws. + */ +function _objIterateEntries(obj: any, mapper: (key: string) => R): R[] { const result: R[] = []; - objForEachKey(obj, (key, val) => { - result.push(mapper(key, val)); - }); + if (obj && (isObject(obj) || isFunction(obj))) { + for (const key in obj) { + if (objHasOwn(obj, key)) { + result.push(mapper(key)); + } + } + } + return result; } @@ -35,7 +46,7 @@ export function polyObjKeys(obj: any): string[] { throwTypeError("non-object " + dumpObj(obj)); } - return _objIterateEntries(obj, (k, _) => k); + return _objIterateEntries(obj, (k) => k); } /** @@ -61,7 +72,7 @@ export function polyObjKeys(obj: any): string[] { */ /*#__NO_SIDE_EFFECTS__*/ export function polyObjEntries(value: {} | { [s: string]: T } | ArrayLike): [string, T][] { - return _objIterateEntries(value, (k, v) => [k, v]); + return _objIterateEntries<[string, T]>(value, (k) => [k, (value as any)[k]]); } /** @@ -87,5 +98,5 @@ export function polyObjEntries(value: {} | { [s: string]: T } | ArrayLi */ /*#__NO_SIDE_EFFECTS__*/ export function polyObjValues(value: {} | { [s: string]: T } | ArrayLike): T[] { - return _objIterateEntries(value, (_, v) => v); + return _objIterateEntries(value, (k) => (value as any)[k]); } diff --git a/lib/test/bundle-size-check.js b/lib/test/bundle-size-check.js index f479393e..0d47b09f 100644 --- a/lib/test/bundle-size-check.js +++ b/lib/test/bundle-size-check.js @@ -25,13 +25,13 @@ const configs = [ { name: "es6-min-zip", path: "../bundle/es6/umd/ts-utils.min.js", - limit: 15.5 * 1024, // 15.5 kb in bytes + limit: 15.75 * 1024, // 15.75 kb in bytes compress: true }, { name: "es5-min-poly", path: "../bundle/es5/ts-polyfills-utils.min.js", - limit: 10.5 * 1024, // 10.5 kb in bytes + limit: 10.75 * 1024, // 10.75 kb in bytes compress: false } ]; diff --git a/lib/test/src/common/object/forEachOwnKeySafe.test.ts b/lib/test/src/common/object/forEachOwnKeySafe.test.ts index 0cc92cf7..aa890b1e 100644 --- a/lib/test/src/common/object/forEachOwnKeySafe.test.ts +++ b/lib/test/src/common/object/forEachOwnKeySafe.test.ts @@ -9,6 +9,9 @@ import { assert } from "@nevware21/tripwire-chai"; import { forEachOwnKey, forEachOwnKeySafe } from "../../../../src/object/forEachOwnKey"; import { hasSymbol } from "../../../../src/symbol/symbol"; +import { arrForEach } from "../../../../src/array/forEach"; +import { dumpObj } from "../../../../src/helpers/diagnostics"; +import { UNDEF_VALUE } from "../../../../src/internal/constants"; describe("object forEachOwnKeySafe tests", () => { describe("forEachOwnKey", () => { @@ -47,6 +50,107 @@ describe("object forEachOwnKeySafe tests", () => { assert.include(visited, "constructor"); assert.include(visited, "prototype"); }); + + it("should continue iterating when a string key getter throws", () => { + const obj: any = { a: 1 }; + Object.defineProperty(obj, "b", { + get: function() { + throw new Error("getter failed"); + }, + enumerable: true, + configurable: true + }); + obj.c = 3; + + const visited: { [key: string]: any } = {}; + assert.doesNotThrow(() => { + forEachOwnKey(obj, (key, value) => { + visited[key as string] = value; + }); + }, "Should not throw when a getter throws"); + + assert.deepEqual(Object.keys(visited).sort(), ["a", "b", "c"], "Should visit all keys including the throwing getter"); + assert.isUndefined(visited["b"], "Should pass undefined for the throwing getter"); + assert.equal(visited["c"], 3, "Should pass the value for c"); + }); + + it("should continue iterating when a symbol key getter throws", () => { + if (!hasSymbol()) { + return; + } + + const badSym = Symbol("bad"); + const goodSym = Symbol("good"); + const obj: any = { a: 1 }; + Object.defineProperty(obj, badSym, { + get: function() { + throw new Error("getter failed"); + }, + enumerable: true, + configurable: true + }); + obj[goodSym] = 2; + + const keys: PropertyKey[] = []; + const values: any[] = []; + assert.doesNotThrow(() => { + forEachOwnKey(obj, (key, value) => { + keys.push(key); + values.push(value); + }); + }, "Should not throw when a symbol getter throws"); + + assert.deepEqual(keys, ["a", badSym, goodSym], "Should visit all keys including the throwing symbol getter"); + assert.deepEqual(values, [1, undefined, 2], "Should pass undefined for the throwing symbol getter"); + }); + + it("should use the provided thisArg for both string and symbol keys", () => { + const obj: any = { a: 1 }; + if (hasSymbol()) { + obj[Symbol("sym")] = 2; + } + + const ctx = { n: "ctx" }; + const thisValues: any[] = []; + forEachOwnKey(obj, function(this: any) { + thisValues.push(this); + }, ctx); + + assert.equal(thisValues.length, hasSymbol() ? 2 : 1, "Should visit every key"); + arrForEach(thisValues, (value) => { + assert.strictEqual(value, ctx, "Should use the provided thisArg for every key"); + }); + }); + + it("should use the object as this for null / undefined thisArg and falsy values as-is", () => { + const obj: any = { a: 1 }; + if (hasSymbol()) { + obj[Symbol("sym")] = 2; + } + + const expectedCalls = hasSymbol() ? 2 : 1; + arrForEach([UNDEF_VALUE, null], (nullValue) => { + const seen: any[] = []; + forEachOwnKey(obj, function(this: any) { + seen.push(this); + }, nullValue); + assert.equal(seen.length, expectedCalls, "Should visit every key"); + arrForEach(seen, (value) => { + assert.strictEqual(value, obj, dumpObj(nullValue) + " thisArg should use the object as this"); + }); + }); + + arrForEach([0, "", false], (falsyValue) => { + const seen: any[] = []; + forEachOwnKey(obj, function(this: any) { + seen.push(this); + }, falsyValue); + assert.equal(seen.length, expectedCalls, "Should visit every key"); + arrForEach(seen, (value) => { + assert.strictEqual(value, falsyValue, "thisArg of " + dumpObj(falsyValue) + " should be used as-is"); + }); + }); + }); }); describe("forEachOwnKeySafe", () => { @@ -124,6 +228,36 @@ describe("object forEachOwnKeySafe tests", () => { assert.equal(actualThis, thisArg, "Should bind the callback to provided thisArg"); }); + it("should use the object as this for null / undefined thisArg and falsy values as-is", () => { + const obj: any = { a: 1 }; + if (hasSymbol()) { + obj[Symbol("sym")] = 2; + } + + const expectedCalls = hasSymbol() ? 2 : 1; + arrForEach([UNDEF_VALUE, null], (nullValue) => { + const seen: any[] = []; + forEachOwnKeySafe(obj, function(this: any) { + seen.push(this); + }, nullValue); + assert.equal(seen.length, expectedCalls, "Should visit every key"); + arrForEach(seen, (value) => { + assert.strictEqual(value, obj, dumpObj(nullValue) + " thisArg should use the object as this"); + }); + }); + + arrForEach([0, "", false], (falsyValue) => { + const seen: any[] = []; + forEachOwnKeySafe(obj, function(this: any) { + seen.push(this); + }, falsyValue); + assert.equal(seen.length, expectedCalls, "Should visit every key"); + arrForEach(seen, (value) => { + assert.strictEqual(value, falsyValue, "thisArg of " + dumpObj(falsyValue) + " should be used as-is"); + }); + }); + }); + it("should handle null and undefined values", () => { let callCount = 0; @@ -137,5 +271,27 @@ describe("object forEachOwnKeySafe tests", () => { assert.equal(callCount, 0, "Should not call callback for null or undefined"); }); + + it("should continue iterating when a property getter throws", () => { + const obj: any = { a: 1 }; + Object.defineProperty(obj, "b", { + get: function() { + throw new Error("getter failed"); + }, + enumerable: true, + configurable: true + }); + obj.c = 3; + + const visited: { [key: string]: any } = {}; + assert.doesNotThrow(() => { + forEachOwnKeySafe(obj, (key, value) => { + visited[key as string] = value; + }); + }, "Should not throw when a getter throws"); + + assert.deepEqual(Object.keys(visited).sort(), ["a", "b", "c"], "Should visit all keys including the throwing getter"); + assert.isUndefined(visited["b"], "Should pass undefined for the throwing getter"); + }); }); }); diff --git a/lib/test/src/common/object/for_each_key.test.ts b/lib/test/src/common/object/for_each_key.test.ts index 43eb19c0..33aa2a00 100644 --- a/lib/test/src/common/object/for_each_key.test.ts +++ b/lib/test/src/common/object/for_each_key.test.ts @@ -8,6 +8,8 @@ import { assert } from "@nevware21/tripwire-chai"; import { objForEachKey, objForEachKeySafe } from "../../../../src/object/for_each_key"; +import { arrForEach } from "../../../../src/array/forEach"; +import { dumpObj } from "../../../../src/helpers/diagnostics"; describe("object for_each_key tests", () => { describe("objForEachKey", () => { @@ -194,6 +196,60 @@ describe("object for_each_key tests", () => { assert.equal(callCount, 0, "Should not call callback for empty objects"); }); + it("should continue iterating when a property getter throws", () => { + const obj: any = { a: 1 }; + Object.defineProperty(obj, "b", { + get: function() { + throw new Error("getter failed"); + }, + enumerable: true, + configurable: true + }); + obj.c = 3; + + const visited: { [key: string]: any } = {}; + assert.doesNotThrow(() => { + objForEachKey(obj, (key, value) => { + visited[key] = value; + }); + }, "Should not throw when a getter throws"); + + assert.deepEqual(Object.keys(visited).sort(), ["a", "b", "c"], "Should visit all keys including the throwing getter"); + assert.equal(visited["a"], 1, "Should pass the value for a"); + assert.isUndefined(visited["b"], "Should pass undefined for the throwing getter"); + assert.equal(visited["c"], 3, "Should pass the value for c"); + }); + + it("should still allow early exit with -1 when a property getter throws", () => { + const obj: any = { a: 1 }; + Object.defineProperty(obj, "b", { + get: function() { + throw new Error("getter failed"); + }, + enumerable: true, + configurable: true + }); + obj.c = 3; + + const visited: string[] = []; + objForEachKey(obj, (key) => { + visited.push(key); + if (key === "b") { + return -1; + } + }); + + assert.deepEqual(visited, ["a", "b"], "Should stop after the throwing getter key"); + }); + + it("should not swallow exceptions thrown by the callback", () => { + const obj = { a: 1, b: 2 }; + assert.throws(() => { + objForEachKey(obj, () => { + throw new Error("callback failed"); + }); + }, "callback failed"); + }); }); describe("objForEachKeySafe", () => { @@ -299,6 +355,29 @@ describe("object for_each_key tests", () => { assert.strictEqual(actualThis, obj, "Should use the object as thisArg when not provided"); }); + it("should use the object as this for null / undefined thisArg and falsy values as-is", () => { + const obj = { a: 1, b: 2 }; + const thisValues: any[] = []; + + objForEachKeySafe(obj, function(this: any) { + thisValues.push(this); + }, null); + objForEachKeySafe(obj, function(this: any) { + thisValues.push(this); + }, undefined); + assert.deepEqual(thisValues, [obj, obj, obj, obj], "null and undefined thisArg should use the object for every key"); + + arrForEach([0, "", false], (falsyValue) => { + const seen: any[] = []; + objForEachKeySafe(obj, function(this: any) { + seen.push(this); + }, falsyValue); + assert.equal(seen.length, 2, "Should call the callback for each key"); + assert.strictEqual(seen[0], falsyValue, "thisArg of " + dumpObj(falsyValue) + " should be used as-is"); + assert.strictEqual(seen[1], falsyValue, "thisArg of " + dumpObj(falsyValue) + " should be used as-is for every key"); + }); + }); + it("should break iteration when callback returns -1", () => { const obj = { a: 1, b: 2, c: 3, d: 4 }; const visitedKeys: string[] = []; @@ -383,5 +462,27 @@ describe("object for_each_key tests", () => { assert.equal(callCount, 0, "Should not call callback for empty objects"); }); + it("should continue iterating when a property getter throws", () => { + const obj: any = { a: 1 }; + Object.defineProperty(obj, "b", { + get: function() { + throw new Error("getter failed"); + }, + enumerable: true, + configurable: true + }); + obj.c = 3; + + const visited: { [key: string]: any } = {}; + assert.doesNotThrow(() => { + objForEachKeySafe(obj, (key, value) => { + visited[key] = value; + }); + }, "Should not throw when a getter throws"); + + assert.deepEqual(Object.keys(visited).sort(), ["a", "b", "c"], "Should visit all keys including the throwing getter"); + assert.isUndefined(visited["b"], "Should pass undefined for the throwing getter"); + assert.equal(visited["c"], 3, "Should pass the value for c"); + }); }); }); diff --git a/lib/test/src/common/polyfills/object.test.ts b/lib/test/src/common/polyfills/object.test.ts index e4827588..1fdb7d8e 100644 --- a/lib/test/src/common/polyfills/object.test.ts +++ b/lib/test/src/common/polyfills/object.test.ts @@ -68,6 +68,15 @@ describe("object polyfills", () => { }); }); + it("polyObjKeys with a throwing getter", () => { + const getterObj = _createGetterObj(true); + + // Object.keys() never invokes getters, so the polyfill must not either (and therefore cannot fail) + assert.deepEqual(polyObjKeys(getterObj.obj), Object.keys(getterObj.obj)); + assert.deepEqual(polyObjKeys(getterObj.obj), ["a", "b", "c"]); + assert.equal(getterObj.calls(), 0, "polyObjKeys should not invoke any getters"); + }); + describe("polyObjEntries", () => { it("examples", () => { assert.deepEqual(polyObjEntries({ Hello: "Darkness", my: "old", friend: "." }), [ [ "Hello", "Darkness" ], [ "my", "old"], [ "friend", "." ] ]); @@ -78,6 +87,34 @@ describe("object polyfills", () => { // Array-like object with random key ordering assert.deepEqual(polyObjEntries({ 100: "a", 2: "b", 7: "c" }), [ ["2", "b"], ["7", "c"], ["100", "a"] ]); }); + + it("should invoke each getter once and return its value", () => { + const getterObj = _createGetterObj(false); + const result = polyObjEntries(getterObj.obj); + assert.deepEqual(result, [ ["a", 1], ["b", 2], ["c", 3] ]); + assert.equal(getterObj.calls(), 1, "Should invoke the getter exactly once"); + + const nativeEntries = (Object as any).entries; + if (nativeEntries) { + const nativeGetterObj = _createGetterObj(false); + assert.deepEqual(result, nativeEntries(nativeGetterObj.obj), "Should match the native result"); + assert.equal(nativeGetterObj.calls(), 1, "Native should also invoke the getter exactly once"); + } + }); + + it("should propagate an exception thrown by a getter like the native implementation", () => { + const getterObj = _createGetterObj(true); + assert.throws(() => { + polyObjEntries(getterObj.obj); + }, "getter failed"); + + const nativeEntries = (Object as any).entries; + if (nativeEntries) { + assert.throws(() => { + nativeEntries(getterObj.obj); + }, "getter failed"); + } + }); }); describe("polyObjValues", () => { @@ -90,8 +127,62 @@ describe("object polyfills", () => { // Array-like object with random key ordering assert.deepEqual(polyObjValues({ 100: "a", 2: "b", 7: "c" }), [ "b", "c", "a"]); }); + + it("should invoke each getter once and return its value", () => { + const getterObj = _createGetterObj(false); + const result = polyObjValues(getterObj.obj); + assert.deepEqual(result, [ 1, 2, 3 ]); + assert.equal(getterObj.calls(), 1, "Should invoke the getter exactly once"); + + const nativeValues = (Object as any).values; + if (nativeValues) { + const nativeGetterObj = _createGetterObj(false); + assert.deepEqual(result, nativeValues(nativeGetterObj.obj), "Should match the native result"); + assert.equal(nativeGetterObj.calls(), 1, "Native should also invoke the getter exactly once"); + } + }); + + it("should propagate an exception thrown by a getter like the native implementation", () => { + const getterObj = _createGetterObj(true); + assert.throws(() => { + polyObjValues(getterObj.obj); + }, "getter failed"); + + const nativeValues = (Object as any).values; + if (nativeValues) { + assert.throws(() => { + nativeValues(getterObj.obj); + }, "getter failed"); + } + }); }); + /** + * Creates `{ a: 1, b: , c: 3 }` where the "b" getter either returns 2 or throws, and counts its invocations + */ + function _createGetterObj(throws: boolean) { + let calls = 0; + const obj: any = { a: 1 }; + Object.defineProperty(obj, "b", { + get: function() { + calls++; + if (throws) { + throw new Error("getter failed"); + } + + return 2; + }, + enumerable: true, + configurable: true + }); + obj.c = 3; + + return { + obj: obj, + calls: () => calls + }; + } + describe("polyObjIs", () => { it("should handle primitive values correctly", () => { // Undefined diff --git a/lib/tsconfig.base.json b/lib/tsconfig.base.json index 9783b840..2ed7d5a6 100644 --- a/lib/tsconfig.base.json +++ b/lib/tsconfig.base.json @@ -7,6 +7,7 @@ "moduleResolution": "node", "target": "es5", "lib": ["ES2015", "DOM"], + "types": ["node"], "skipLibCheck": true, "forceConsistentCasingInFileNames": true, "importHelpers": true,