-
-
Notifications
You must be signed in to change notification settings - Fork 108
Fixes equality checking for Map objects. #52
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
|
Updated description to include more details of the issue this addresses. Updated the Travis config so I could get a nice green checkmark. |
|
I'd also like to see this feature. Can anyone review it? |
20523d1 to
06a483c
Compare
index.js
Outdated
| } | ||
|
|
||
| // Maps | ||
| if (typeof Map && a.constructor === Map && b.constructor === Map) { |
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.
This check is not robust across realms; it will need a brand check.
index.js
Outdated
|
|
||
| // Maps | ||
| if (typeof Map && a.constructor === Map && b.constructor === Map) { | ||
| ka = Array.from(a.keys()); |
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.
Array.from is not necessarily available; we'll need to use a package or manually implement a while loop to call .next() on the iterator. However, not all Map implementations have keys either, so it's actually a pretty hard problem to robustly iterate Maps and Sets.
…ssert Fixes inspect-js#54. Fixes inspect-js#46.
191fb2f to
ecd15ae
Compare
2a4c42d to
47eb288
Compare
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.
updated this with a robust solution
Currently:
While
Maps don't have good browser support at this time, they are useful in Node, and I've found that I want tape'sdeepEqual,deepLooseEqual, and friends to work properly withMaps when testing Node applications.The implementation is a near duplication of the
Objecthandling code, but deduplicating that didn't seem straightforward or all that beneficial to me at the time.I also made a few small changes to make the style more consistent, and added a .gitignore, but tried to not overreach or get too out of scope. I can revert these changes upon request.