mirror of
https://github.com/oven-sh/bun
synced 2026-02-02 15:08:46 +00:00
fix(node:assert): handle Number and Boolean wrappers in deepStrictEqual (#25201)
## Summary - Fixes `assert.deepStrictEqual()` to properly compare Number and Boolean wrapper objects - Previously, `new Number(1)` and `new Number(2)` were incorrectly considered equal because they have no enumerable properties - Now correctly extracts and compares internal values using `JSC::sameValue()`, then falls through to check own properties ## Test plan - [x] Run `bun bd test test/regression/issue/24045.test.ts` - all 6 tests pass - [x] Verify tests fail with system Bun (`USE_SYSTEM_BUN=1`) to confirm fix validity - [x] Verified behavior matches Node.js exactly (see table below) ## Node.js Compatibility | Test Case | Node.js | Bun | |-----------|---------|-----| | Different Number values (`new Number(1)` vs `new Number(2)`) | throws | throws | | Same Number values (`new Number(1)` vs `new Number(1)`) | equal | equal | | 0 vs -0 (`new Number(0)` vs `new Number(-0)`) | throws | throws | | NaN equals NaN (`new Number(NaN)` vs `new Number(NaN)`) | equal | equal | | Different Boolean values (`new Boolean(true)` vs `new Boolean(false)`) | throws | throws | | Same Boolean values | equal | equal | | Number wrapper vs primitive (`new Number(1)` vs `1`) | throws | throws | | Number vs Boolean wrapper | throws | throws | | Same value, different own properties | throws | throws | | Same value, same own properties | equal | equal | | Different own property values | throws | throws | ## Example Before (bug): ```javascript assert.deepStrictEqual(new Number(1), new Number(2)); // passes incorrectly ``` After (fixed): ```javascript assert.deepStrictEqual(new Number(1), new Number(2)); // throws AssertionError ``` Closes #24045 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Bot <claude-bot@bun.sh> Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Jarred Sumner <jarred@jarredsumner.com>
This commit is contained in:
@@ -1555,9 +1555,20 @@ std::optional<bool> specialObjectsDequal(JSC::JSGlobalObject* globalObject, Mark
|
||||
auto* gp2 = jsDynamicCast<JSC::JSGlobalProxy*, JSCell>(c2);
|
||||
return gp1->target()->m_globalThis == gp2->target()->m_globalThis;
|
||||
}
|
||||
default: {
|
||||
case NumberObjectType:
|
||||
case BooleanObjectType: {
|
||||
// Number and Boolean wrapper objects must be the same type and have the same internal value
|
||||
if (c1Type != c2Type) return false;
|
||||
JSValue val1 = jsCast<JSWrapperObject*>(c1)->internalValue();
|
||||
JSValue val2 = jsCast<JSWrapperObject*>(c2)->internalValue();
|
||||
bool same = JSC::sameValue(globalObject, val1, val2);
|
||||
RETURN_IF_EXCEPTION(scope, {});
|
||||
if (!same) return false;
|
||||
// Fall through to check own properties
|
||||
break;
|
||||
}
|
||||
default:
|
||||
break;
|
||||
}
|
||||
return std::nullopt;
|
||||
}
|
||||
|
||||
105
test/regression/issue/24045.test.ts
Normal file
105
test/regression/issue/24045.test.ts
Normal file
@@ -0,0 +1,105 @@
|
||||
import assert from "assert/strict";
|
||||
import { expect, test } from "bun:test";
|
||||
|
||||
test("assert.deepStrictEqual() should compare Number wrapper object values - issue #24045", () => {
|
||||
// Different values should throw
|
||||
expect(() => {
|
||||
assert.deepStrictEqual(new Number(1), new Number(2));
|
||||
}).toThrow("Expected values to be strictly deep-equal");
|
||||
|
||||
// Same values should not throw
|
||||
expect(() => {
|
||||
assert.deepStrictEqual(new Number(1), new Number(1));
|
||||
}).not.toThrow();
|
||||
|
||||
// Edge cases
|
||||
// 0 and -0 should be different in strict mode
|
||||
expect(() => {
|
||||
assert.deepStrictEqual(new Number(0), new Number(-0));
|
||||
}).toThrow("Expected values to be strictly deep-equal");
|
||||
|
||||
// NaN should equal NaN
|
||||
expect(() => {
|
||||
assert.deepStrictEqual(new Number(NaN), new Number(NaN));
|
||||
}).not.toThrow();
|
||||
|
||||
expect(() => {
|
||||
assert.deepStrictEqual(new Number(Infinity), new Number(-Infinity));
|
||||
}).toThrow("Expected values to be strictly deep-equal");
|
||||
});
|
||||
|
||||
test("assert.deepStrictEqual() should compare Boolean wrapper object values - issue #24045", () => {
|
||||
// Different values should throw
|
||||
expect(() => {
|
||||
assert.deepStrictEqual(new Boolean(true), new Boolean(false));
|
||||
}).toThrow("Expected values to be strictly deep-equal");
|
||||
|
||||
// Same values should not throw
|
||||
expect(() => {
|
||||
assert.deepStrictEqual(new Boolean(true), new Boolean(true));
|
||||
}).not.toThrow();
|
||||
|
||||
expect(() => {
|
||||
assert.deepStrictEqual(new Boolean(false), new Boolean(false));
|
||||
}).not.toThrow();
|
||||
});
|
||||
|
||||
test("assert.deepStrictEqual() should not compare Number wrapper with primitive", () => {
|
||||
// Wrapper objects should not equal primitives in strict mode
|
||||
expect(() => {
|
||||
assert.deepStrictEqual(new Number(1), 1);
|
||||
}).toThrow("Expected values to be strictly deep-equal");
|
||||
|
||||
expect(() => {
|
||||
assert.deepStrictEqual(1, new Number(1));
|
||||
}).toThrow("Expected values to be strictly deep-equal");
|
||||
});
|
||||
|
||||
test("assert.deepStrictEqual() should not compare Boolean wrapper with primitive", () => {
|
||||
// Wrapper objects should not equal primitives in strict mode
|
||||
expect(() => {
|
||||
assert.deepStrictEqual(new Boolean(true), true);
|
||||
}).toThrow("Expected values to be strictly deep-equal");
|
||||
|
||||
expect(() => {
|
||||
assert.deepStrictEqual(false, new Boolean(false));
|
||||
}).toThrow("Expected values to be strictly deep-equal");
|
||||
});
|
||||
|
||||
test("assert.deepStrictEqual() should not compare Number and Boolean wrappers", () => {
|
||||
// Different wrapper types should not be equal even with truthy/falsy values
|
||||
expect(() => {
|
||||
assert.deepStrictEqual(new Number(1), new Boolean(true));
|
||||
}).toThrow("Expected values to be strictly deep-equal");
|
||||
|
||||
expect(() => {
|
||||
assert.deepStrictEqual(new Number(0), new Boolean(false));
|
||||
}).toThrow("Expected values to be strictly deep-equal");
|
||||
});
|
||||
|
||||
test("assert.deepStrictEqual() should check own properties on wrapper objects", () => {
|
||||
// Same internal value but different own properties should not be equal
|
||||
const num1 = new Number(42);
|
||||
const num2 = new Number(42);
|
||||
(num1 as any).customProp = "hello";
|
||||
|
||||
expect(() => {
|
||||
assert.deepStrictEqual(num1, num2);
|
||||
}).toThrow("Expected values to be strictly deep-equal");
|
||||
|
||||
// Same internal value and same own properties should be equal
|
||||
(num2 as any).customProp = "hello";
|
||||
expect(() => {
|
||||
assert.deepStrictEqual(num1, num2);
|
||||
}).not.toThrow();
|
||||
|
||||
// Different own property values should not be equal
|
||||
const bool1 = new Boolean(true);
|
||||
const bool2 = new Boolean(true);
|
||||
(bool1 as any).foo = 1;
|
||||
(bool2 as any).foo = 2;
|
||||
|
||||
expect(() => {
|
||||
assert.deepStrictEqual(bool1, bool2);
|
||||
}).toThrow("Expected values to be strictly deep-equal");
|
||||
});
|
||||
Reference in New Issue
Block a user