Fix deepStrictEqual to perform deep value comparison#273
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.
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)) { |
There was a problem hiding this comment.
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)).
Co-authored-by: nev21 <82737406+nev21@users.noreply.github.com>
da9b07d to
291cb95
Compare
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.
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.