node v26 compat: util.diff(), dns perf entries, proxy-aware deepStrictEqual - #35382
Closed
cirospaciari wants to merge 8 commits into
Closed
node v26 compat: util.diff(), dns perf entries, proxy-aware deepStrictEqual#35382cirospaciari wants to merge 8 commits into
cirospaciari wants to merge 8 commits into
Conversation
The strict deep-equality path rejected any pair where the two objects
report different class names. JSC reports a Proxy's own class name, not
its target's, so assert.deepStrictEqual(new Proxy({}, {}), {}) failed
even though every trap forwards to an identical target. Node compares
prototypes here, never class names.
Skip the class-name check when either side is a proxy; the prototype
check and the own-property walk above and below it already go through
the traps.
Adds the v26 util.diff(actual, expected) API, which reports the Myers diff of two strings or two string arrays as [operation, value] pairs (-1 delete, 0 unchanged, 1 insert). The existing native myersDiff comparator only accepted strings; it now also takes arrays of strings, diffing one element per line, so the public API and node:assert share a single implementation.
node records a PerformanceEntry of type 'dns' for every successful
lookup, lookupService and resolver query, with the query entries named
after the c-ares binding (queryAny, queryA, ...). Bun recorded none, so
a PerformanceObserver watching { type: 'dns' } never fired.
Entries are only constructed when such an observer is registered.
test-diff.js and test-dns-perf_hooks.js are unblocked by the two preceding commits. test-compile-cache-typescript-esm.js already passed (it skips itself on a build that reports no Amaro, which is what Bun reports).
deepStrictEqual bailed out when either side was a WeakMap, WeakSet or Promise. node tests only the left operand, which makes the comparison asymmetric on purpose: a Proxy wrapping a promise equals that promise with the proxy on the left, but not the other way round. Verified against the v26.3.0 binary.
Exercises deepStrictEqual against the immutable-view proxies the upstream harness hands to tests.
Collaborator
|
Updated 10:00 PM PT - Jul 23rd, 2026
⏳ @cirospaciari, your commit 0c71dce is still building in
Add |
||||||||||||||||||||||||||||||||||||||||||||||||||||||
It only ever skips: it gates on process.config.variables.node_use_amaro, which Bun reports as falsy and cannot report as true. Vendoring it adds a file that asserts nothing.
Contributor
|
Found 1 issue this PR may fix:
🤖 Generated with Claude Code |
Contributor
|
This PR may be a duplicate of:
🤖 Generated with Claude Code |
Member
Author
|
Superseded by #35391, which combines this branch with the other five stacked on The branch |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #34660 — review that first.
What this does
Closes a slice of the Node v26.3.0
test/parallelcompatibility gap: three runtime gapsare fixed and the three upstream tests they unblock are vendored verbatim.
Why now
The v26 test-suite sweep needs these files present in
test/js/node/test/parallel, andeach was blocked on a real behavioural difference rather than on test scaffolding.
What changed
assert.deepStrictEqualsees through a Proxy (src/jsc/bindings/bindings.cpp).Two changes to the strict comparison path:
It bailed out whenever the two objects reported different class names. JSC reports a
Proxy's own class name, not its target's, so
assert.deepStrictEqual(new Proxy({}, {}), {})returned false — as did
util.isDeepStrictEqualfor any proxy. Node comparesprototypes here, never class names, so the class-name check is now skipped when either
side is a proxy. The prototype check immediately above it and the own-property walk
below it already go through the proxy traps.
WeakMaps, WeakSets and Promises were rejected when either operand was one. Node tests
the left operand only, which makes the comparison deliberately asymmetric: a Proxy
wrapping a promise equals that promise with the proxy on the left, but not the other
way round. Confirmed against the v26.3.0 binary:
The check moved out of
specialObjectsDequal(which is called twice with the operandsswapped) into
Bun__deepEquals, where operand order is stable. Both changes are insideif constexpr (checkPrototypes), i.e. thenode:assert/node:utilentry point only —Bun.deepEqualsandexpect()are untouched.util.diff()is implemented (src/js/node/util.ts,src/runtime/node/node_assert*.rs).New in Node v26: it reports the Myers diff of two strings or two string arrays as
[operation, value]pairs (-1delete,0unchanged,1insert). Bun already had theMyers comparator natively for
node:assertdiffs, but it only accepted strings; it now alsoaccepts arrays of strings, one element per line, so the public API and
node:assertshareone implementation rather than growing a second copy of the algorithm.
node:dnsemits'dns'performance entries (src/js/node/dns.ts). Node records aPerformanceEntryof type'dns'for every successfullookup,lookupServiceandresolver query, naming resolver entries after the c-ares binding (
queryAny,queryA, …).Bun recorded none, so a
PerformanceObserverwatching{ type: 'dns' }never fired. Thisreuses the JS-side node-entry registry that
'net'and'http'already use. Entries areonly constructed when such an observer is registered, so the ordinary path allocates
nothing extra.
Upstream tests now vendored (3)
parallel/test-diff.js—util.diff()parallel/test-dns-perf_hooks.js—'dns'performance entriesparallel/test-common-must-not-mutate-object-deep.mjs—deepStrictEqualagainst theimmutable-view proxies the upstream harness hands to tests
All three are byte-identical to
v26.3.0, and each one fails on the base branch for thereason its commit fixes — none is a test that merely skips itself.
How this was verified
bun run --config=bunfig.node-test.toml <file>(bun testfor the files that importnode:test), withBUN_GARBAGE_COLLECTOR_LEVEL=1 BUN_FEATURE_FLAG_INTERNAL_FOR_TESTING=1 NO_COLOR=1 FORCE_COLOR=0. All three exit 0; all three fail on the base branch.test-assert-*,test-util-*andtest-dns-*upstream files: all still pass.bun test test/js/node/dns test/js/node/util test/js/bun/test/expect.test.js: no newfailures (two pre-existing debug-build timeouts in
util-inspect/parse-argsreproduceidentically on the base binary).
util.diffoutput compared field-by-field againstnode v26.3.0for the string, array,identical-input and three error cases in the upstream test.
Note for reviewers
Another branch in this campaign also edits
Bun__deepEqualsinsrc/jsc/bindings/bindings.cpp: it adds key-material comparison forBun::JSKeyObject/JSCryptoKey, whose own-property walk currently reports unrelated keys as equal. Thatchange and this one are complementary — it lands a new case inside
specialObjectsDequal,this one narrows two guards outside it — but they touch nearby lines, so expect a small
textual conflict.