Skip to content

Commit ccc9aba

Browse files
ish1416Tango992
andauthored
fix(node/assert): deepStrictEqual now correctly handles Number objects (denoland#31233)
Fixes denoland#31172 ## Description The `deepStrictEqual` function was using `asserts.equal()` which doesn't properly handle boxed primitives like Number objects. Changed it to use `isDeepStrictEqual()` from `comparisons.ts` which correctly handles Number, String, Boolean, BigInt, and Symbol objects. ## Changes - Modified `ext/node/polyfills/assert.ts`: - Added import for `isDeepStrictEqual` from `ext:deno_node/internal/util/comparisons.ts` - Updated `deepStrictEqual` function to use `isDeepStrictEqual()` instead of `asserts.equal()` - Added test cases in `tests/unit_node/assert_test.ts`: - Test that `deepStrictEqual` throws `AssertionError` for different Number objects - Test that `deepStrictEqual` passes for equal Number objects ## Testing Added unit tests to verify: - `deepStrictEqual` throws `AssertionError` for different Number objects (e.g., `new Number(1)` vs `new Number(2)`) - `deepStrictEqual` passes for equal Number objects (e.g., `new Number(1)` vs `new Number(1)`) ## Related Issue denoland#31172 - `assert.deepStrictEqual` does not throw exception for Number objects --------- Co-authored-by: Daniel Rahmanto <[email protected]>
1 parent 5421a29 commit ccc9aba

File tree

3 files changed

+39
-11
lines changed

3 files changed

+39
-11
lines changed

ext/node/polyfills/assert.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,10 @@ import {
1616
ERR_INVALID_RETURN_VALUE,
1717
ERR_MISSING_ARGS,
1818
} from "ext:deno_node/internal/errors.ts";
19-
import { isDeepEqual } from "ext:deno_node/internal/util/comparisons.ts";
19+
import {
20+
isDeepEqual,
21+
isDeepStrictEqual,
22+
} from "ext:deno_node/internal/util/comparisons.ts";
2023
import { primordials } from "ext:core/mod.js";
2124
import { CallTracker } from "ext:deno_node/internal/assert/calltracker.js";
2225
import { deprecate } from "node:util";
@@ -394,7 +397,7 @@ function deepStrictEqual(
394397
throw new ERR_MISSING_ARGS("actual", "expected");
395398
}
396399

397-
if (!asserts.equal(actual, expected)) {
400+
if (!isDeepStrictEqual(actual, expected)) {
398401
throw new AssertionError({
399402
message,
400403
actual,

ext/node/polyfills/internal/util/comparisons.ts

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,12 @@ import {
2727
ONLY_ENUMERABLE,
2828
SKIP_SYMBOLS,
2929
} from "ext:deno_node/internal_binding/util.ts";
30+
import { primordials } from "ext:core/mod.js";
31+
32+
const {
33+
ObjectPrototypeHasOwnProperty,
34+
ObjectPrototypePropertyIsEnumerable,
35+
} = primordials;
3036

3137
enum valueType {
3238
noIterator,
@@ -265,7 +271,7 @@ function keyCheck(
265271
// Cheap key test
266272
let i = 0;
267273
for (; i < aKeys.length; i++) {
268-
if (!val2.propertyIsEnumerable(aKeys[i])) {
274+
if (!ObjectPrototypePropertyIsEnumerable(val2, aKeys[i])) {
269275
return false;
270276
}
271277
}
@@ -276,14 +282,14 @@ function keyCheck(
276282
let count = 0;
277283
for (i = 0; i < symbolKeysA.length; i++) {
278284
const key = symbolKeysA[i];
279-
if (val1.propertyIsEnumerable(key)) {
280-
if (!val2.propertyIsEnumerable(key)) {
285+
if (ObjectPrototypePropertyIsEnumerable(val1, key)) {
286+
if (!ObjectPrototypePropertyIsEnumerable(val2, key)) {
281287
return false;
282288
}
283289
// added toString here
284290
aKeys.push(key.toString());
285291
count++;
286-
} else if (val2.propertyIsEnumerable(key)) {
292+
} else if (ObjectPrototypePropertyIsEnumerable(val2, key)) {
287293
return false;
288294
}
289295
}
@@ -436,7 +442,9 @@ function isEqualBoxedPrimitive(a: any, b: any): boolean {
436442
}
437443

438444
function getEnumerables(val: any, keys: any) {
439-
return keys.filter((key: string) => val.propertyIsEnumerable(key));
445+
return keys.filter((key: string) =>
446+
ObjectPrototypePropertyIsEnumerable(val, key)
447+
);
440448
}
441449

442450
function objEquiv(
@@ -459,21 +467,21 @@ function objEquiv(
459467
}
460468
} else if (iterationType === valueType.isArray) {
461469
for (; i < obj1.length; i++) {
462-
if (obj1.hasOwnProperty(i)) {
470+
if (ObjectPrototypeHasOwnProperty(obj1, i)) {
463471
if (
464-
!obj2.hasOwnProperty(i) ||
472+
!ObjectPrototypeHasOwnProperty(obj2, i) ||
465473
!innerDeepEqual(obj1[i], obj2[i], strict, memos)
466474
) {
467475
return false;
468476
}
469-
} else if (obj2.hasOwnProperty(i)) {
477+
} else if (ObjectPrototypeHasOwnProperty(obj2, i)) {
470478
return false;
471479
} else {
472480
const keys1 = Object.keys(obj1);
473481
for (; i < keys1.length; i++) {
474482
const key = keys1[i];
475483
if (
476-
!obj2.hasOwnProperty(key) ||
484+
!ObjectPrototypeHasOwnProperty(obj2, key) ||
477485
!innerDeepEqual(obj1[key], obj2[key], strict, memos)
478486
) {
479487
return false;

tests/unit_node/assert_test.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,3 +45,20 @@ Deno.test("[node/assert] error message from strictEqual should be the same as As
4545
{ message },
4646
);
4747
});
48+
49+
Deno.test("[node/assert] deepStrictEqual throws for different Number objects", () => {
50+
// Test case from issue #31172
51+
assert.throws(
52+
() => {
53+
assert.deepStrictEqual(new Number(1), new Number(2));
54+
},
55+
assert.AssertionError,
56+
);
57+
});
58+
59+
Deno.test("[node/assert] deepStrictEqual passes for equal Number objects", () => {
60+
// Equal Number objects should pass
61+
assert.doesNotThrow(() => {
62+
assert.deepStrictEqual(new Number(1), new Number(1));
63+
});
64+
});

0 commit comments

Comments
 (0)