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
5 changes: 3 additions & 2 deletions src/jsc/bindings/bindings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1793,8 +1793,9 @@ static std::optional<bool> specialObjectsDequalSlow(const DeepEqualsMode& mode,
return false;
}
if (mode.checkPrototypes) {
// node also compares `lastIndex` and own enumerable properties.
bool sameLastIndex = JSC::sameValue(globalObject, left->getLastIndex(), right->getLastIndex());
// node compares `lastIndex` with ===, not Object.is, then own enumerable properties:
// https://github.com/nodejs/node/blob/v26.12.1/lib/internal/util/comparisons.js#L148-L152
bool sameLastIndex = JSC::JSValue::strictEqual(globalObject, left->getLastIndex(), right->getLastIndex());
RETURN_IF_EXCEPTION(scope, {});
if (!sameLastIndex) {
return false;
Comment thread
robobun marked this conversation as resolved.
Resolved
Expand Down
5 changes: 3 additions & 2 deletions src/jsc/modules/NodeUtilTypesModule.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -808,8 +808,9 @@ static bool compareBranch(JSC::JSGlobalObject* globalObject, JSC::MarkedArgument
if (actualRegExp->regExp()->pattern() != expectedRegExp->regExp()->pattern()
|| actualRegExp->regExp()->flags() != expectedRegExp->regExp()->flags())
return false;
// node also compares lastIndex strictly (comparisons.js isRegExp arm).
bool lastIndexEqual = JSC::sameValue(globalObject, actualRegExp->getLastIndex(), expectedRegExp->getLastIndex());
// node compares lastIndex with ===, not Object.is:
// https://github.com/nodejs/node/blob/v26.12.1/lib/internal/util/comparisons.js#L148-L152
bool lastIndexEqual = JSC::JSValue::strictEqual(globalObject, actualRegExp->getLastIndex(), expectedRegExp->getLastIndex());
RETURN_IF_EXCEPTION(scope, false);
if (!lastIndexEqual)
return false;
Expand Down
39 changes: 39 additions & 0 deletions test/js/node/assert/assert.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,45 @@ describe("assert.partialDeepStrictEqual", () => {
assert.partialDeepStrictEqual({ arr: [shared, shared] }, { arr: [{ x: 1 }] });
});

// Node's areSimilarRegExps() compares lastIndex with ===: -0 equals 0, NaN differs from NaN, "0" differs from 0.
// https://github.com/nodejs/node/blob/v26.12.1/lib/internal/util/comparisons.js#L148-L152
test.each([
["at the top level", re => re],
["in an array", re => [re]],
["in an object property", re => ({ re })],
["in a Map value", re => new Map([["k", re]])],
["in a Map key", re => new Map([[re, 1]])],
["in a Set", re => new Set([re])],
["in an Error cause", re => new Error("e", { cause: re })],
])("RegExp lastIndex is compared with === %s", (_name, wrap) => {
const withLastIndex = lastIndex => Object.assign(/a/g, { lastIndex });
const partial = (actual, expected) => {
try {
assert.partialDeepStrictEqual(wrap(actual), wrap(expected));
return "equal";
} catch (e) {
return e.code;
}
};
const sameObject = withLastIndex(NaN);
expect({
negativeZeroAndZero: partial(withLastIndex(-0), withLastIndex(0)),
zeroAndNegativeZero: partial(withLastIndex(0), withLastIndex(-0)),
nanAndNaN: partial(withLastIndex(NaN), withLastIndex(NaN)),
stringZeroAndZero: partial(withLastIndex("0"), withLastIndex(0)),
threeAndZero: partial(withLastIndex(3), withLastIndex(0)),
// The identity shortcut answers before lastIndex is read.
sameObjectWithNaN: partial(sameObject, sameObject),
}).toEqual({
negativeZeroAndZero: "equal",
zeroAndNegativeZero: "equal",
nanAndNaN: "ERR_ASSERTION",
stringZeroAndZero: "ERR_ASSERTION",
threeAndZero: "ERR_ASSERTION",
sameObjectWithNaN: "equal",
});
});

test("a descendant actual identical to an ancestor expected is not mistaken for a cycle", () => {
const e = { k: {} };
assert.partialDeepStrictEqual({ k: e }, e);
Expand Down
99 changes: 99 additions & 0 deletions test/js/node/assert/deep-equal.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -658,6 +658,62 @@ const cases: Case[] = [
// RegExp.
{ name: "two equal regexps", a: () => /a/g, b: () => /a/g, strict: true, loose: true },
{ name: "regexps with different flags", a: () => /a/g, b: () => /a/i, strict: false, loose: false },
// lastIndex is compared with ===, not Object.is, in every mode.
{
name: "regexps with lastIndex -0 and 0",
a: () => Object.assign(/a/g, { lastIndex: -0 }),
b: () => /a/g,
strict: true,
loose: true,
},
{
name: "regexps with lastIndex NaN",
a: () => Object.assign(/a/g, { lastIndex: NaN }),
b: () => Object.assign(/a/g, { lastIndex: NaN }),
strict: false,
loose: false,
looseBug: "reports equal",
},
// A lastIndex of 0 and of -0 make two deep-equal members. The Set and Map walks take the
// first deep-equal member and do not pair members one to one.
// https://github.com/oven-sh/bun/issues/28760
{
name: "sets of three regexps with lastIndex 0, -0, 1 and 0, 1, 1",
a: () =>
new Set([
Object.assign(/a/g, { lastIndex: 0 }),
Object.assign(/a/g, { lastIndex: -0 }),
Object.assign(/a/g, { lastIndex: 1 }),
]),
b: () =>
new Set([
Object.assign(/a/g, { lastIndex: 0 }),
Object.assign(/a/g, { lastIndex: 1 }),
Object.assign(/a/g, { lastIndex: 1 }),
]),
strict: false,
loose: false,
strictBug: "reports equal",
looseBug: "reports equal",
},
// https://github.com/oven-sh/bun/issues/34830
{
name: "maps with two regexp keys whose lastIndex is 0 and -0",
a: () =>
new Map([
[Object.assign(/a/g, { lastIndex: 0 }), 1],
[Object.assign(/a/g, { lastIndex: -0 }), 2],
]),
b: () =>
new Map([
[Object.assign(/a/g, { lastIndex: 0 }), 1],
[Object.assign(/a/g, { lastIndex: -0 }), 2],
]),
strict: true,
loose: true,
strictBug: "reports not equal",
looseBug: "reports not equal",
Comment thread
robobun marked this conversation as resolved.
Resolved
},
{
name: "regexps with different lastIndex",
a: () => Object.assign(/a/g, { lastIndex: 3 }),
Expand Down Expand Up @@ -1162,6 +1218,49 @@ describe("util.isDeepStrictEqual", () => {
expect(calls).toBe(2);
});

// Node's areSimilarRegExps() compares lastIndex with ===: -0 equals 0, NaN differs from NaN, "0" differs from 0.
// https://github.com/nodejs/node/blob/v26.12.1/lib/internal/util/comparisons.js#L148-L152
describe("RegExp lastIndex", () => {
const withLastIndex = (lastIndex: unknown) => Object.assign(/a/g, { lastIndex });
const positions: Array<[string, (re: RegExp) => unknown]> = [
["at the top level", re => re],
["in an array", re => [re]],
["in an object property", re => ({ re })],
["in a Map value", re => new Map([["k", re]])],
["in a Map key", re => new Map([[re, 1]])],
["in a Set", re => new Set([re])],
["in an Error cause", re => new Error("e", { cause: re })],
];

for (const [name, wrap] of positions) {
test(`is compared with === ${name}`, () => {
const sameObject = withLastIndex(NaN);
for (const skipPrototype of [false, true]) {
const equal = (a: RegExp, b: RegExp) => util.isDeepStrictEqual(wrap(a), wrap(b), skipPrototype as any);
assert.deepStrictEqual(
{
skipPrototype,
negativeZeroAndZero: equal(withLastIndex(-0), withLastIndex(0)),
zeroAndNegativeZero: equal(withLastIndex(0), withLastIndex(-0)),
nanAndNaN: equal(withLastIndex(NaN), withLastIndex(NaN)),
stringZeroAndZero: equal(withLastIndex("0"), withLastIndex(0)),
// The identity shortcut answers before lastIndex is read.
sameObjectWithNaN: equal(sameObject, sameObject),
},
{
skipPrototype,
negativeZeroAndZero: true,
zeroAndNegativeZero: true,
nanAndNaN: false,
stringZeroAndZero: false,
sameObjectWithNaN: true,
},
);
}
});
}
});

// The third argument was added in Node v26.
describe("skipPrototype", () => {
class Foo {
Expand Down
Loading