-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
fix: not addressing to Sets and Maps as object without keys #14873
Conversation
✅ Deploy Preview for jestjs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
1557ff2
to
769fa58
Compare
@@ -336,7 +336,8 @@ const isObjectWithKeys = (a: any) => | |||
isObject(a) && | |||
!(a instanceof Error) && | |||
!Array.isArray(a) && | |||
!(a instanceof Date); | |||
!(a instanceof Date) && | |||
!(a instanceof Set); | |||
|
|||
export const subsetEquality = ( |
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.
In case of subset
as a Set
the getObjectKeys(subset)
at line 358 returns empty array ([]
), so the .every(...)
in the same line is vacuous truth, resulting the comparison not to raise an exception
769fa58
to
bd31657
Compare
bd31657
to
e35b950
Compare
e35b950
to
05ccc77
Compare
05ccc77
to
7d6586d
Compare
7d6586d
to
f2346a7
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.
Thanks!
@SimenB Hey, I see one of the tests failed after the squash - is there an option to rerun it only? 🙌 |
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Summary
Fixes #14599
The problem is that comparing list of nulls with Set or list of Sets:
In both cases the comparison does not throw an exception.
Update:
Same issue is happening with Map instead of set, I have fixed it too
Test plan
Unit tests added
Both cases described in the Summery have been tested locally with the fix and reproduced without the fix