-
-
Notifications
You must be signed in to change notification settings - Fork 0
Fix deepStrictEqual to perform deep value comparison #273
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Co-authored-by: nev21 <82737406+nev21@users.noreply.github.com>
Co-authored-by: nev21 <82737406+nev21@users.noreply.github.com>
|
@copilot Add tests for the examples in deepEqual as well make sure that comparing a: 1 with a: "1" passes |
Co-authored-by: nev21 <82737406+nev21@users.noreply.github.com>
Added comprehensive tests for
All 2,117 tests passing ✓ |
Co-authored-by: nev21 <82737406+nev21@users.noreply.github.com>
0eb3961 to
47eb64d
Compare
|
@copilot update the #file:migrating-from-node-assert.md and readme,md files with the changes to deepStrictEqual introduced in this change that will be published as v0.1.7 |
Updated both documentation files in commit da9b07d:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes assert.deepStrictEqual() so it performs deep value comparison for objects/arrays (with strict type checking on nested values), instead of incorrectly requiring reference equality.
Changes:
- Removed an early-return in
_deepEquals()that prevented deep comparison whenstrict: true. - Updated
deepStrictEqualtests to assert deep value equality (not same-reference equality). - Added additional
deepEqual(loose) tests demonstrating nested coercive comparisons.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
core/test/src/assert/assert.equals.test.ts |
Expands deepEqual/deepStrictEqual test cases to validate value-based deep comparison behavior. |
core/src/assert/funcs/equal.ts |
Removes strict-mode early return so strict mode continues into deep comparison logic. |
common/config/rush/npm-shrinkwrap.json |
Updates shrinkwrap due to dependency patch bumps. |
Files not reviewed (1)
- common/config/rush/npm-shrinkwrap.json: Language not supported
| if (isEquals !== null) { | ||
| return isEquals; | ||
| } | ||
|
|
||
| if (options.strict === true) { | ||
| // If strict is set and we get here, then the values are not strictly equal | ||
| return false; | ||
| } | ||
|
|
||
| if (isPlainObject(value) && isPlainObject(expected)) { |
Copilot
AI
Feb 12, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With the strict early-return removed, _deepEquals() can now reach the typeFn comparison path for types like Promise/function/WeakMap/etc. Several of those type comparers are currently _strictEquals, which returns null for non-primitive non-reference-equal values. Returning null from _deepEquals() breaks nested comparisons because callers check === false (e.g. in _iterEquals / _matchKeys), so null is treated as “equal” and deepStrictEqual can incorrectly pass for objects/arrays containing different Promises/functions. Ensure _deepEquals() never returns null (coerce null -> false), or wrap the _typeEquals entries that use _strictEquals to return a boolean (e.g. (_strictEquals(...) === true)).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot apply changes based on this feedback
Co-authored-by: nev21 <82737406+nev21@users.noreply.github.com>
da9b07d to
291cb95
Compare
nev21
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #273 +/- ##
==========================================
- Coverage 91.43% 91.43% -0.01%
==========================================
Files 78 78
Lines 3445 3443 -2
Branches 887 886 -1
==========================================
- Hits 3150 3148 -2
Misses 295 295
🚀 New features to boost your workflow:
|
nevware21-bot
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved by nevware21-bot
…comparisons Fixes issue from PR #273 where _deepEquals() returning null from _strictEquals for non-reference-equal Promises/functions/WeakMaps caused nested comparisons to fail incorrectly. Callers check (=== false), treating null as "equal". Two bugs fixed: 1. Added _strictEqualsBool wrapper to convert null → false for types that should only be equal if reference-equal (Promise, function, symbol, WeakMap, WeakSet) 2. Fixed _getTypeComparer() objType lookup bug where undefined objType accessed _typeEquals["undefined"] returning wrong comparer instead of falling back to theType Changes: - core/src/assert/funcs/equal.ts: * Added _strictEqualsBool wrapper function * Updated _typeEquals map for promise/function/symbol/weakmap/weakset * Fixed _getTypeComparer to check (objType && _typeEquals[objType]) - core/test/src/assert/assert.equals.test.ts: * Added regression test for Promise/function in nested objects/arrays
…comparisons (#275) Fixes issue from PR #273 where _deepEquals() returning null from _strictEquals for non-reference-equal Promises/functions/WeakMaps caused nested comparisons to fail incorrectly. Callers check (=== false), treating null as "equal". Two bugs fixed: 1. Added _strictEqualsBool wrapper to convert null → false for types that should only be equal if reference-equal (Promise, function, symbol, WeakMap, WeakSet) 2. Fixed _getTypeComparer() objType lookup bug where undefined objType accessed _typeEquals["undefined"] returning wrong comparer instead of falling back to theType Changes: - core/src/assert/funcs/equal.ts: * Added _strictEqualsBool wrapper function * Updated _typeEquals map for promise/function/symbol/weakmap/weakset * Fixed _getTypeComparer to check (objType && _typeEquals[objType]) - core/test/src/assert/assert.equals.test.ts: * Added regression test for Promise/function in nested objects/arrays
Fix deepStrictEqual to perform deep value comparison
Plan:
_deepEqualsfunction to support deep comparison in strict modeChanges:
Original Implementation Fix
deepStrictEqualto perform deep value comparison instead of reference equalityComprehensive Tests
deepStrictEqualwith strict type checkingdeepEqualwith loose equalityDocumentation Updates (v0.1.7)
Updated
docs/migration/migrating-from-node-assert.md:Updated
README.md:Original prompt
Created from VS Code.
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.