Skip to content

assert: fix TypeError on deepStrictEqual with null Map key or Set member - #64449

Open
semx wants to merge 1 commit into
nodejs:mainfrom
semx:fix-deepstrictequal-map-null-key
Open

assert: fix TypeError on deepStrictEqual with null Map key or Set member#64449
semx wants to merge 1 commit into
nodejs:mainfrom
semx:fix-deepstrictequal-map-null-key

Conversation

@semx

@semxsemx commented Jul 12, 2026

Copy link
Copy Markdown

assert.deepStrictEqual() (and util.isDeepStrictEqual()) throw a TypeError instead of comparing when the first Map has a null (or other primitive) key that lines up against object-only keys in the other map:

constassert=require('node:assert');consta=newMap([[null,1],[{},2]]);constb=newMap([[{},9],[{},9]]);assert.deepStrictEqual(a,b);// TypeError: Cannot read properties of null (reading 'constructor')// expected: an AssertionError (the maps are not deeply equal)

The same happens for an undefined key. Set has the identical problem for a null/undefined member (once the set is large enough to skip the small-set fast path):

assert.deepStrictEqual(newSet([null,{},{}]),newSet([{},{},{}]));// TypeError: Cannot read properties of null (reading 'constructor')

It only triggers in strict mode when the other collection's keys/members are all objects and their count equals the first collection's size.

Cause

In mapObjectEquiv and setObjectEquiv (lib/internal/util/comparisons.js), primitive/null keys and members are resolved directly via b.has() / b.get(), but that handling was gated behind extraChecks (array.length !== a.size). When the counts match, the gate is skipped and the primitive/null key/member falls through to objectComparisonStart, which dereferences .constructor and throws on null/undefined.

Fix

Handle primitive/null keys and members unconditionally — they can only match by identity and can never match through the object comparator — so they are always resolved by direct lookup and never reach objectComparisonStart. The collections above now compare as unequal (throwing an AssertionError, as expected) instead of throwing a TypeError. Object comparison is unchanged.

Added regression cases (null and undefined keys/members, for both Map and Set) to test/parallel/test-assert-deep.js.

@nodejs-github-botnodejs-github-bot added needs-ci PRs that need a full CI run. util Issues and PRs related to the built-in util module. labels Jul 12, 2026
@codecov

codecovBot commented Jul 15, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.24%. Comparing base (8a3b11c) to head (19f16d9).
⚠️ Report is 41 commits behind head on main.

Additional details and impacted files
@@ Coverage Diff @@## main #64449 +/- ##
==========================================
- Coverage 90.24% 90.24% -0.01% 
==========================================
Files 741 741 Lines 241384 241429 +45 Branches 45480 45488 +8 ==========================================
+ Hits 217844 217875 +31 + Misses 15097 15090 -7 - Partials 8443 8464 +21 
Files with missing linesCoverage Δ
lib/internal/util/comparisons.js99.71% <100.00%> (+<0.01%)⬆️

... and 38 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@semx
semxforce-pushed the fix-deepstrictequal-map-null-key branch from 19f16d9 to 98843e4CompareJuly 15, 2026 22:00
@semx

semx commented Jul 30, 2026

Copy link
Copy Markdown
Author

This has two approvals (thanks @ljharb, @jasnell) and is still labelled needs-ci. Could a collaborator kick off a CI run?

Happy to rebase first if that helps — the branch is otherwise unchanged since the reviews.

@semx

semx commented Aug 5, 2026

Copy link
Copy Markdown
Author

Following up: this has two approvals (thanks @ljharb, @jasnell) and is only waiting on CI to run so it can land. Could a collaborator start CI / apply the label so it can move through the commit-queue? Happy to rebase if needed. Thanks!

@ljharb

Copy link
Copy Markdown
Member

First I think it needs a rebase?

deepStrictEqual() and util.isDeepStrictEqual() threw "Cannot read
properties of null (reading 'constructor')" instead of comparing when
a Map key or Set member was null/undefined (or another primitive) and
lined up against object-only keys/members in the other collection with
an equal count. The primitive/null handling was gated behind an
optimization that is skipped when the counts match, letting such keys
reach objectComparisonStart, which dereferences `.constructor`.
Resolve primitive and null keys/members directly in every case.
Signed-off-by: semx <7532921+semx@users.noreply.github.com>
@semx
semxforce-pushed the fix-deepstrictequal-map-null-key branch from 98843e4 to cdd7206CompareAugust 6, 2026 10:03
@semx

semx commented Aug 6, 2026

Copy link
Copy Markdown
Author

Rebased onto current main and force-pushed. There was one conflict, in lib/internal/util/comparisons.js: main has since landed an equivalent fix for the Map path (mapObjectEquiv), so I dropped that now-redundant hunk and kept the still-needed Set-path fix (setObjectEquiv) plus all four null/undefined regression tests. The final diff is just the Set fix + tests; the branch is one commit on top of main and both files pass node --check. Should be good for CI now — thanks!

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-ciPRs that need a full CI run.utilIssues and PRs related to the built-in util module.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@semx@ljharb@jasnell@nodejs-github-bot