Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Feb 11, 2026

Fix deepStrictEqual to perform deep value comparison

Plan:

  • Explore repository and understand current implementation
  • Fix the _deepEquals function to support deep comparison in strict mode
  • Update existing tests to expect deep value equality instead of reference equality
  • Add new test cases for the specific examples in the problem statement
  • Run tests to verify all changes work correctly
  • Request code review and run CodeQL security check
  • Add comprehensive tests for deepEqual with loose equality
  • Update documentation for v0.1.7 changes

Changes:

Original Implementation Fix

  • Fixed deepStrictEqual to perform deep value comparison instead of reference equality
  • Updated tests to verify deep value equality for strict comparison

Comprehensive Tests

  • Added test suite for deepStrictEqual with strict type checking
  • Added test suite for deepEqual with loose equality

Documentation Updates (v0.1.7)

Updated docs/migration/migrating-from-node-assert.md:

  • Section 3.1: Clarified behavior change in v0.1.7 (now matches Node.js)
  • Section 3.2: Updated comparison table to show identical behavior
  • Section 4.1: Marked deepStrictEqual as identical to Node.js
  • Sections 5.2-5.4: Updated examples to show no migration needed for v0.1.7+
  • Section 6.2: Updated migration strategy (import changes only)
  • Section 7: Updated GitHub Copilot prompts
  • Section 8: Updated common patterns (no changes needed)
  • Section 11: Updated troubleshooting for v0.1.7
  • Section 12: Updated best practices
  • Section 13: Updated migration checklist

Updated README.md:

  • Added "What's New in v0.1.7" section
  • Documented deepStrictEqual behavior change
  • Updated recommended version to >= 0.1.7
  • Added examples showing strict vs loose equality
Original prompt

We need to update the deepStrictEqual implementation to support these passing examples
* assert.deepStrictEqual({ a: 1 }, { a: 1 }); // Passes
* assert.deepStrictEqual({ a: 1 }, { a: "1" }); // Throws AssertionFailure
* assert.deepStrictEqual([1, 2], [1, 2]); // Passes
* assert.deepStrictEqual([1, 2], [2, 1]); // Throws AssertionFailure
* assert.deepStrictEqual({ a: { b: 1 } }, { a: { b: 1 } }); // Passes
* assert.deepStrictEqual({ a: { b: 1 } }, { a: { b: "1" } }); // Throws AssertionFailure
* assert.deepStrictEqual([{ a: 1 }], [{ a: 1 }]); // Passes
* assert.deepStrictEqual([{ a: 1 }], [{ a: "1" }]); // Throws AssertionFailure
otherwise we are just performing an object equality. Add tests for these cases and then update the implementation

The user has attached the following file paths as relevant context:

  • .github\copilot-instructions.md

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.

Copilot AI and others added 2 commits February 11, 2026 17:18
Co-authored-by: nev21 <82737406+nev21@users.noreply.github.com>
Co-authored-by: nev21 <82737406+nev21@users.noreply.github.com>
Copilot AI changed the title [WIP] Update deepStrictEqual implementation to support passing examples Fix deepStrictEqual to perform deep value comparison Feb 11, 2026
Copilot AI requested a review from nev21 February 11, 2026 17:32
@nev21
Copy link
Contributor

nev21 commented Feb 12, 2026

@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>
Copy link
Contributor Author

Copilot AI commented Feb 12, 2026

@copilot Add tests for the examples in deepEqual as well make sure that comparing a: 1 with a: "1" passes

Added comprehensive tests for deepEqual with loose equality in commit d2d8dea. The new test suite validates that:

  • { a: 1 } == { a: "1" } passes (loose type coercion)
  • { a: { b: 1 } } == { a: { b: "1" } } passes (nested loose equality)
  • [{ a: 1 }] == [{ a: "1" }] passes (arrays with objects, loose equality)

All 2,117 tests passing ✓

Co-authored-by: nev21 <82737406+nev21@users.noreply.github.com>
@nev21 nev21 force-pushed the copilot/update-deepstrictequal-implementation branch from 0eb3961 to 47eb64d Compare February 12, 2026 03:17
@nev21
Copy link
Contributor

nev21 commented Feb 12, 2026

@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

@nev21 nev21 marked this pull request as ready for review February 12, 2026 03:22
@nev21 nev21 requested review from a team as code owners February 12, 2026 03:22
Copilot AI review requested due to automatic review settings February 12, 2026 03:22
Copy link
Contributor Author

Copilot AI commented Feb 12, 2026

@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:

migrating-from-node-assert.md:

  • Section 3.1: Documented v0.1.7 behavior change (now matches Node.js)
  • Updated all migration examples to show no changes needed for v0.1.7+
  • Updated migration strategy, patterns, and best practices

README.md:

  • Added "What's New in v0.1.7" section
  • Documented deepStrictEqual behavior change with examples
  • Updated recommended version to >= 0.1.7
  • Added migration note for users upgrading from older versions

Copy link
Contributor

Copilot AI left a 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 when strict: true.
  • Updated deepStrictEqual tests 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

Comment on lines 494 to 498
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)) {
Copy link

Copilot AI Feb 12, 2026

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)).

Copilot uses AI. Check for mistakes.
Copy link
Contributor

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>
@nev21 nev21 force-pushed the copilot/update-deepstrictequal-implementation branch from da9b07d to 291cb95 Compare February 12, 2026 06:24
@nev21 nev21 enabled auto-merge (squash) February 12, 2026 06:25
Copy link
Contributor

@nev21 nev21 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@codecov
Copy link

codecov bot commented Feb 12, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.43%. Comparing base (808339a) to head (291cb95).
⚠️ Report is 1 commits behind head on main.

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              
Files with missing lines Coverage Δ
core/src/assert/funcs/equal.ts 82.01% <ø> (-0.16%) ⬇️
🚀 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.

Copy link
Contributor

@nevware21-bot nevware21-bot left a 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

@nev21 nev21 merged commit 119dd67 into main Feb 12, 2026
11 checks passed
@nev21 nev21 deleted the copilot/update-deepstrictequal-implementation branch February 12, 2026 06:43
Copilot AI mentioned this pull request Feb 12, 2026
9 tasks
nev21 added a commit that referenced this pull request Feb 12, 2026
…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
@nev21 nev21 added this to the 0.1.7 milestone Feb 12, 2026
nev21 added a commit that referenced this pull request Feb 12, 2026
…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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants