fix: compare Set and Map entries without relying on order

[rajanpanth]

Fixes #46 (the correctness half of it).

entriesEqual gathers Set/Map entries into arrays and compares them with iterableEqual after a plain .sort(). That works for primitives, but Array#sort compares by string, and every object entry stringifies to the same thing, so the sort leaves object entries in insertion order and the comparison becomes order-sensitive.

Sets and Maps are unordered, so this reports unequal for collections that are equal:

const eql = require('deep-eql');

eql(new Set([{}, { a: 5 }]), new Set([{ a: 5 }, {} ]));   // false, should be true
eql(new Set([1, 2, 3]),      new Set([3, 2, 1]));         // true  (primitives sort fine)

The first case is exactly the repro @BridgeAR posted in #46 in 2017.

Approach

Rather than sorting, each left entry is paired with an as-yet unclaimed right entry. Deep equality is an equivalence relation, so if a left entry matches more than one right entry those right entries are themselves equal, which means taking the first match is safe and no backtracking is needed.

Size and empty-collection early-outs are untouched, so the common unequal-size case still exits immediately. Worst case is O(n²) comparisons where it was O(n log n) string comparisons before, but the previous ordering was only correct for primitives, and #46 covers the performance side separately.

Verification

• Full suite passes: 176 passing (174 before, plus the 2 added here).
• To confirm the new tests actually exercise the fix, I reverted index.js alone and re-ran: 175 passing, 1 failing. So the test fails without the fix and passes with it.
• npm run lint reports no issues on these changes. (My Windows checkout produces a wall of linebreak-style CRLF errors across every file; git stores LF, and those are unrelated to this diff.)

Behaviour I checked by hand, all as expected:

case	before	after
Set([{}, {a:5}]) vs Set([{a:5}, {}])	false	true
Set([{a:1},{b:2}]) vs Set([{b:2},{a:1}])	false	true
Set([1,2,3]) vs Set([3,2,1])	true	true
Map([['a',1],['b',2]]) vs reversed	true	true
Set([{a:1}]) vs Set([{a:2}])	false	false
different sizes	false	false
circular sets (existing tests)	true	true

Happy to close this if someone is already working on #46, or to split the performance concerns into a separate change.

[rajanpanth]

While going through open issues I noticed this also fixes #51.

The TypeError there comes from the same .sort(): Array#sort stringifies entries, and null-prototype objects cannot be converted to primitives, so entriesEqual threw before it ever compared anything. With entries paired via deepEqual instead of sorted, the #51 repro returns true (and false for genuinely different sets) instead of throwing. I confirmed the repro throws on main and passes on this branch.

Pushed one more commit locking that with a regression test using the duck/cat null-prototype shape from the issue. 177 tests passing.

[rajanpanth]

Done both. iterableEqual now takes a fourth unordered parameter and entriesEqual just calls it, so there is no separate loop any more.

I took your swap-and-pop as written rather than my claimed array. Same worst case, but it shrinks the pool instead of skipping over already-claimed slots, so the scan gets cheaper as it goes rather than staying at full width.

177 tests pass and eslint is clean. Flipping the new argument to false fails 4 of them, so the unordered path is genuinely covered rather than incidentally passing.

[43081j]

any particular reason these are not just regular for loops?

[43081j]

we probably don't need to mention this as it doesn't mean anything to someone who doesn't know it used sort before. seems like leaking of thoughts

[rajanpanth]

Both done.

No good reason on the loops, I was matching the var index = -1; while (++index < length) style the rest of the file uses. Plain for loops read better here, especially with the labelled continue, so they are for loops now.

Agreed on the comment too. It only made sense as a diff against the sort call that used to be there, which is no leftover a reader should have to reconstruct. Removed.

177 tests still pass and eslint is clean.

[43081j]

given this looks like agentic output, i'd like to see a response from the person driving the agent before we can land this.

if this is the output of an agent (particularly the comments), let me know so i can better review this under that assumption. if it isn't, still let me know!

[rajanpanth]

i have only used agent for replies, all other code, changes are reviewed by me..

[43081j]

thank you 🙏 we get a lot of agentic noise these days so its very helpful to know there's a person behind it rather than it being fully automated.

change looks good to me!