Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 71 additions & 5 deletions src/jsc/bindings/bindings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -863,8 +863,34 @@ bool Bun__deepEquals(JSC::JSGlobalObject* globalObject, JSValue v1, JSValue v2,
return true;
}

if constexpr (isStrict) {
// Proxies skip the array fast path above, which is the only place array length is compared.
if (v1Array && v2Array) {
JSValue lengthValue = o1->get(globalObject, vm.propertyNames->length);
RETURN_IF_EXCEPTION(scope, false);
uint64_t length1 = lengthValue.toLength(globalObject);
RETURN_IF_EXCEPTION(scope, false);
lengthValue = o2->get(globalObject, vm.propertyNames->length);
RETURN_IF_EXCEPTION(scope, false);
uint64_t length2 = lengthValue.toLength(globalObject);
RETURN_IF_EXCEPTION(scope, false);
if (length1 != length2) {
return false;
}
}
}

if constexpr (isStrict && !skipPrototype) {
if (!equal(JSObject::calculatedClassName(o1), JSObject::calculatedClassName(o2))) {
if (c1->type() == ProxyObjectType || c2->type() == ProxyObjectType) {
// calculatedClassName() is always "ProxyObject" for a Proxy; compare observable prototypes instead.
JSValue p1 = o1->getPrototype(globalObject);
RETURN_IF_EXCEPTION(scope, false);
JSValue p2 = o2->getPrototype(globalObject);
RETURN_IF_EXCEPTION(scope, false);
if (p1 != p2) {
return false;
}
} else if (!equal(JSObject::calculatedClassName(o1), JSObject::calculatedClassName(o2))) {
Comment thread
claude[bot] marked this conversation as resolved.
Comment thread
robobun marked this conversation as resolved.
return false;
}
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Expand Down Expand Up @@ -1010,6 +1036,20 @@ bool Bun__deepEquals(JSC::JSGlobalObject* globalObject, JSValue v1, JSValue v2,
if (propertyArrayLength1 != propertyArrayLength2) {
return false;
}
if (c1->type() == ProxyObjectType || c2->type() == ProxyObjectType) {
// A Proxy `has` trap can make the getIfPropertyExists probe below vacuous; compare the own-key sets directly.
for (size_t j = 0; j < propertyArrayLength1; j++) {
UniquedStringImpl* name1 = a1[j].impl();
bool found = false;
for (size_t k = 0; k < propertyArrayLength2; k++) {
if (a2[k].impl() == name1) {
found = true;
break;
}
}
if (!found) return false;
}
}
}

// take a property name from one, try to get it from both
Expand Down Expand Up @@ -1736,7 +1776,7 @@ bool Bun__deepMatch(
JSObject* obj = objValue.getObject();
JSObject* subsetObj = subsetValue.getObject();

PropertyNameArrayBuilder subsetProps(vm, PropertyNameMode::StringsAndSymbols, PrivateSymbolMode::Include);
PropertyNameArrayBuilder subsetProps(vm, PropertyNameMode::StringsAndSymbols, PrivateSymbolMode::Exclude);
subsetObj->getPropertyNames(globalObject, subsetProps, DontEnumPropertiesMode::Exclude);
RETURN_IF_EXCEPTION(throwScope, false);

Expand All @@ -1745,12 +1785,38 @@ bool Bun__deepMatch(
// - two "simple" arrays
// similar to what is done in deepEquals (canPerformFastPropertyEnumerationForIterationBun)

bool objIsArray = isArray(globalObject, objValue);
RETURN_IF_EXCEPTION(throwScope, false);
bool subsetIsArray = isArray(globalObject, subsetValue);
RETURN_IF_EXCEPTION(throwScope, false);

// An array expectation only matches an array; the reverse is allowed (object expectation is a key subset).
if (subsetIsArray && !objIsArray) {
return false;
}

// arrays should match exactly
if (isArray(globalObject, objValue) && isArray(globalObject, subsetValue)) {
if (obj->getArrayLength() != subsetObj->getArrayLength()) {
if (objIsArray && subsetIsArray) {
uint64_t objLength = 0;
uint64_t subsetLength = 0;
if (!obj->isProxy() && !subsetObj->isProxy()) {
objLength = obj->getArrayLength();
subsetLength = subsetObj->getArrayLength();
} else {
// getArrayLength() reads the indexed butterfly, which a Proxy does not have.
JSValue lengthValue = obj->get(globalObject, vm.propertyNames->length);
RETURN_IF_EXCEPTION(throwScope, false);
objLength = lengthValue.toLength(globalObject);
RETURN_IF_EXCEPTION(throwScope, false);
lengthValue = subsetObj->get(globalObject, vm.propertyNames->length);
RETURN_IF_EXCEPTION(throwScope, false);
subsetLength = lengthValue.toLength(globalObject);
RETURN_IF_EXCEPTION(throwScope, false);
}
if (objLength != subsetLength) {
return false;
}
PropertyNameArrayBuilder objProps(vm, PropertyNameMode::StringsAndSymbols, PrivateSymbolMode::Include);
PropertyNameArrayBuilder objProps(vm, PropertyNameMode::StringsAndSymbols, PrivateSymbolMode::Exclude);
obj->getPropertyNames(globalObject, objProps, DontEnumPropertiesMode::Exclude);
RETURN_IF_EXCEPTION(throwScope, false);
if (objProps.size() != subsetProps.size()) {
Expand Down
123 changes: 115 additions & 8 deletions test/js/bun/test/expect.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
* `NODE_OPTIONS=--experimental-vm-modules npx jest test/js/bun/test/expect.test.js`
*/

import assert from "node:assert";
import { isDeepStrictEqual } from "node:util";
// import these functions typed with the bun:test types,
// so this test can also be used to detect issues with the "bun:test" type definitions
import test_interop from "./test-interop.js";
Expand Down Expand Up @@ -677,8 +679,111 @@ describe("expect()", () => {
expect(p1).toStrictEqual(p2);
}
});

test("toStrictEqual treats Proxy as transparent", () => {
// https://github.com/oven-sh/bun/issues/9103
{
const t = { a: 1 };
expect(new Proxy(t, {})).toStrictEqual(t);
expect(t).toStrictEqual(new Proxy(t, {}));
expect(new Proxy(t, {})).toStrictEqual({ a: 1 });
expect({ a: 1 }).toStrictEqual(new Proxy(t, {}));
expect(Bun.deepEquals(new Proxy(t, {}), t, true)).toBe(true);
expect(Bun.deepEquals(t, new Proxy(t, {}), true)).toBe(true);

expect(new Proxy({ a: 1 }, {})).not.toStrictEqual({ a: 2 });
expect(new Proxy({ a: 1 }, {})).not.toStrictEqual({ a: 1, b: 2 });
expect(new Proxy({ a: 1, b: 2 }, {})).not.toStrictEqual({ a: 1 });
}
{
// arrays
const arr = [1, 2, 3];
expect(new Proxy(arr, {})).toStrictEqual([1, 2, 3]);
expect([1, 2, 3]).toStrictEqual(new Proxy(arr, {}));
expect(new Proxy(arr, {})).not.toStrictEqual([1, 2, 4]);
expect(new Proxy(arr, {})).not.toStrictEqual([1, 2]);
expect(new Proxy(arr, {})).not.toStrictEqual({ 0: 1, 1: 2, 2: 3 });

// A trailing hole changes length but not the own-enumerable property
// set, and a Proxy skips the array fast path that compares lengths.
const sparse = [1, 2, 3];
sparse.length = 4;
expect(new Proxy([1, 2, 3], {})).not.toStrictEqual(sparse);
expect(new Proxy(sparse, {})).not.toStrictEqual([1, 2, 3]);
expect(new Proxy([1, 2, 3], {})).not.toStrictEqual(new Proxy(sparse, {}));
expect(new Proxy(sparse, {})).toStrictEqual(sparse);
}
{
// class instances: proxy over instance equals another instance,
// but not a plain object with the same shape
class Foo {
constructor() {
this.a = 1;
}
}
const f = new Foo();
expect(new Proxy(f, {})).toStrictEqual(new Foo());
expect(new Foo()).toStrictEqual(new Proxy(f, {}));
expect(new Proxy(f, {})).not.toStrictEqual({ a: 1 });
expect({ a: 1 }).not.toStrictEqual(new Proxy(f, {}));

// two proxies over different-class targets must not pass the strict type gate
expect(new Proxy({ a: 1 }, {})).not.toStrictEqual(new Proxy(new Foo(), {}));
expect(new Proxy(new Foo(), {})).not.toStrictEqual(new Proxy({ a: 1 }, {}));
expect(Bun.deepEquals(new Proxy({ a: 1 }, {}), new Proxy(f, {}), true)).toBe(false);
}
{
// nested
expect({ x: new Proxy({ a: 1 }, {}) }).toStrictEqual({ x: { a: 1 } });
expect({ x: { a: 1 } }).toStrictEqual({ x: new Proxy({ a: 1 }, {}) });
}
{
// proxy of proxy
const t = { a: 1 };
expect(new Proxy(new Proxy(t, {}), {})).toStrictEqual({ a: 1 });
}
{
// node:util and node:assert route through the same path
const t = { a: 1 };
expect(isDeepStrictEqual(new Proxy(t, {}), t)).toBe(true);
expect(isDeepStrictEqual(new Proxy(t, {}), { a: 2 })).toBe(false);
assert.deepStrictEqual(new Proxy(t, {}), { a: 1 });
}
{
// get trap is honored for values
const p = new Proxy({ a: 1 }, { get: () => 99 });
expect(p).not.toStrictEqual({ a: 1 });
expect(p).toStrictEqual(new Proxy({ a: 0 }, { get: () => 99 }));

// a get trap alone cannot mask different own-key sets (existence is probed via `has`)
expect({ b: 1 }).not.toStrictEqual(new Proxy({ a: 1 }, { get: () => 1 }));
expect(new Proxy({ a: 1 }, { get: () => 1 })).not.toStrictEqual({ b: 1 });

// even a lying `has` trap cannot mask different own-key sets
const lie = { get: () => 1, has: () => true };
expect({ b: 1 }).not.toStrictEqual(new Proxy({ a: 1 }, lie));
expect(new Proxy({ a: 1 }, lie)).not.toStrictEqual({ b: 1 });
expect(new Proxy({ b: 1 }, lie)).not.toStrictEqual(new Proxy({ a: 1 }, lie));
}
{
// revoked proxy comparison should throw, not crash
const { proxy, revoke } = Proxy.revocable({ a: 1 }, {});
revoke();
expect(() => Bun.deepEquals(proxy, { a: 1 }, true)).toThrow(TypeError);
}
});
}

test("toMatchObject treats a Proxy over an array as an array", () => {
// https://github.com/oven-sh/bun/issues/9103
expect(new Proxy([{ a: 1 }], {})).toMatchObject([{ a: 1 }]);
expect([{ a: 1 }]).toMatchObject(new Proxy([{ a: 1 }], {}));
expect(new Proxy([1, 2, 3], {})).toMatchObject([1, 2, 3]);
expect(new Proxy([1, 2], {})).not.toMatchObject([1, 2, 3]);
expect(new Proxy([1, 2, 3], {})).not.toMatchObject([1, 2]);
expect({ x: new Proxy([{ a: 1 }], {}) }).toMatchObject({ x: [{ a: 1 }] });
});

test("deepEquals works with sets/maps/dates/strings", () => {
const f = Symbol.for("foo");

Expand Down Expand Up @@ -1169,33 +1274,35 @@ describe("expect()", () => {
expect(w).toEqual(w);
});

// Allocation-heavy by design (GC stress for #14256); measured at ~4 minutes
// under a debug+ASAN build, far past the 5s default per-test timeout.
test("deepEquals Set/Map stress test", () => {
// https://github.com/oven-sh/bun/issues/14250. Distinct array keys take the
Comment thread
robobun marked this conversation as resolved.
// JSSetIterator/JSMapIterator linear fallback (quadratic in N); 20 x 500
// exercises that fallback thousands of times inside the debug+ASAN budget.
const N = 20;
const arr1 = [];
const arr2 = [];
const arr3 = [];
const arr4 = [];

for (let i = 0; i < 150; i++) {
for (let i = 0; i < N; i++) {
arr1[i] = [i];
arr2[i] = [i];
arr3[i] = [i, [i]];
arr4[i] = [i, [i]];
}

for (let i = 0; i < 2000; i++) {
for (let i = 0; i < 500; i++) {
let outerSet = new Set(arr1);
let innerSet = new Set(arr2);
Bun.deepEquals(outerSet, innerSet);
expect(Bun.deepEquals(outerSet, innerSet)).toBe(true);
}

for (let i = 0; i < 1000; i++) {
for (let i = 0; i < 250; i++) {
let outerMap = new Map(arr3);
let innerMap = new Map(arr4);
Bun.deepEquals(outerMap, innerMap);
expect(Bun.deepEquals(outerMap, innerMap)).toBe(true);
}
}, 480_000);
});

test("deepEquals - Date", () => {
let d = new Date();
Expand Down
Loading