assert: node v26 deep-equality fidelity — loose mode, errors, set/map pairing (+0 tests, correctness only) - #35393
Conversation
assert.deepEqual and assert.notDeepEqual were routed to Bun.deepEquals(a, b, false), the comparison expect().toEqual() uses. That is a different relation from node's loose mode, and it produced wrong 'equal' verdicts for a long list of inputs: 4 vs '4' compared as unequal while a Date subclass carrying extra own properties compared as equal to a plain Date. They now go through a node-loose instantiation of the same native comparator as deepStrictEqual. Relative to node's strict mode, node's loose mode compares non-objects with == (counting callables as non-objects), does not compare prototypes, compares Object.prototype.toString tags instead, and ignores symbol-keyed properties. Bun.deepEquals and expect() do not set checkPrototypes and are unaffected by every new branch. Also fixes two error-comparison gaps that applied to both modes: AggregateError's non-enumerable .errors was never compared, and objects with Error.prototype in their chain that are not real Error instances were compared as plain objects rather than by message/name/cause/errors.
deepStrictEqual(new Set([{a:1},{a:1}]), new Set([{a:1},{a:2}])) reported
equal: every element of the first set found *some* deep-equal partner in
the second, and nothing recorded that {a:1} had already been spent. Maps
had the same hole for deep-equal-but-distinct object keys.
The node entry point now consumes each match, the way node's setEquiv
and mapEquiv do, so multiplicity is significant. Bun.deepEquals and
expect() keep the previous rule. The pairing comparisons memoise through
the existing cycle stack, without which a self-referential set recurses
until the stack overflows.
|
Updated 10:52 PM PT - Jul 23rd, 2026
⏳ @cirospaciari, your commit d1e9ec5 is still building in
Add |
||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Found 3 issues this PR may fix:
🤖 Generated with Claude Code |
cirospaciari
changed the title
|
This PR may be a duplicate of:
🤖 Generated with Claude Code |
|
Superseded by #35391, which now combines eight branches stacked on The branch is not deleted. |
|
🤖 From the combined PR #35391: your change makes 28 tests in That is your fix working — Confirmed member-caused, not a merge interaction: on your own branch's build ( Please strip the now-obsolete |
2 participants
Stacked on #35382 — review that first.
What this does
Brings
assert.deepEqual/assert.deepStrictEqualup to node v26.3.0 fidelity. No testfiles are added here: this is a correctness change to the comparator that hundreds of
already-vendored tests assert through, where a wrong "equal" verdict silently makes
assertions pass that should fail.
Measured against upstream
test/parallel/test-assert-deep.js, which is not vendored yet:24 of its 54 subtests failed before this branch, 4 do now. The four that remain are
listed at the bottom; none of them is a deep-equality bug.
Why now
Two other branches in the v26 sweep are blocked behind this comparator, and every
deepStrictEqualin the vendored suite inherits its verdicts.What changed — all in
Bun__deepEquals(src/jsc/bindings/bindings.cpp)1.
assert.deepEqualnow uses node's loose comparison, not Bun's. It was wired toBun.deepEquals(a, b, false)— the relationexpect().toEqual()uses, which is adifferent relation from node's loose mode. It produced wrong verdicts in both directions:
assert.deepEqual(4, '4')threw (node passes), while aDatesubclass carrying extra ownproperties compared equal to a plain
Date(node fails it). It now goes through anode-loose instantiation of the same native comparator as
deepStrictEqual.Relative to node's strict mode, node's loose mode: compares non-objects with
==(countingcallables as non-objects, per
typeof val !== 'object'), does not compare prototypes,compares
Object.prototype.toStringtags instead — this is what separates anargumentsobject from a plain object with the same keys — and ignores symbol-keyed properties.
Bun.deepEqualsandexpect()never setcheckPrototypes, so every new branch iscompiled out for them.
2. Errors are compared by node's rules even when they are not
Errorinstances.AggregateError'serrorsis own-but-non-enumerable and was never compared, sonew AggregateError([e1], 'x')equallednew AggregateError([e1, e2], 'x'). And an objectwith
Error.prototypein its chain that is not a realErrorInstance— whatObject.create(Object.getPrototypeOf(err), descriptors)produces — was compared as a plainobject, so two of them with different non-enumerable
messagevalues compared equal. Bothnow follow node:
message,name,causeanderrorsare compared unless the right-handside owns them enumerably, in which case the ordinary key walk already covers them.
3. Sets and maps are paired off instead of matched repeatedly.
deepStrictEqual(new Set([{a:1},{a:1}]), new Set([{a:1},{a:2}]))reported equal: everyelement of the first set found some deep-equal partner in the second, and nothing
recorded that
{a:1}had already been spent. Maps had the same hole for deep-equal butdistinct object keys. The node entry point now consumes each match the way node's
setEquiv/mapEquivdo. The pairing comparisons memoise through the existing cyclestack — without that a self-referential set recurses until the stack overflows, which is
how the first version of this change was caught.
How this was verified
test-assert-deep.jsrun the way CI runs it: 30 → 50 of 54 subtests passing.test-assert-partial-deep-equal.js: 139 of 140, unchanged by this branch.test-assert-*,test-util-*andtest-dns-*upstream filesstill pass, including
test-util-isDeepStrictEqual.js.bun test test/js/bun/test/expect.test.js: 415 pass, 0 fail — the jest-facing relationis untouched.
nodev26.3.0 binary rather than inferred,including the asymmetric promise rule and the sparse-array cases.
Still failing in test-assert-deep.js (4), none of them deep-equality
KeyObject/CryptoKeymaterial comparison being added onanother branch. It lands inside
specialObjectsDequal, under the samecheckPrototypesgate, so it will cover loose mode too once merged.
util.inspect(url, { customInspect: false })printsURL {}in Bun andhttp://foo/in node, so the assertion message does not contain the href the testmatches on. The comparison verdict itself is already correct.
util.inspect'sshowProxyrendering inside the assertiondiff.
partialDeepStrictEqual([1,,,3], [1,undefined,,3])passes inBun and throws in node: a hole on the left must not satisfy an explicit
undefinedonthe right. That is in the separate partial comparator (
Bun__deepMatch), not inBun__deepEquals.test-assert-partial-deep-equal.js's one remaining failure is unrelated to comparison too:Bun sets
File.prototypetoBlob.prototyperather than makingFilea real subclass, soObject.getPrototypeOf(File.prototype) !== Blob.prototype. There is a standing TODO forthat in
src/jsc/bindings/JSDOMFile.cpp.