-
-
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
Iterable equality within object #8359
Changes from 10 commits
9bc6a08
86312c9
7e13ba4
8ec8ab5
53408bd
87c4e62
1e5d3b7
84f8bda
8068054
3cd2b4e
c4d40b9
2c3a862
416f5b3
e50e95e
5a46db1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -133,6 +133,13 @@ export const getObjectSubset = (object: any, subset: any): any => { | |
return object; | ||
}; | ||
|
||
// NOTE: Should be removed after dropping node@6.x support, | ||
// and we should use (Object.entries) | ||
export const getObjectEntries: typeof Object.entries = ( | ||
object: any, | ||
): Array<[string, any]> => | ||
object == null ? [] : Object.keys(object).map(key => [key, object[key]]); | ||
|
||
const IteratorSymbol = Symbol.iterator; | ||
|
||
const hasIterator = (object: any) => | ||
|
@@ -251,6 +258,12 @@ export const iterableEquality = ( | |
return false; | ||
} | ||
|
||
const aEntries = getObjectEntries(a); | ||
const bEntries = getObjectEntries(b); | ||
if (!equals(aEntries, bEntries)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need to add This illustrates a limitation that we recently noticed: it is not (yet) possible to for a custom tester to know about sibling testers (for example, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @pedrottimark is there an extra test case you're thinking of we should add? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm so sorry for the very late follow up.
IMO |
||
return false; | ||
} | ||
|
||
// Remove the first value from the stack of traversed values. | ||
aStack.pop(); | ||
bStack.pop(); | ||
|
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.
Instead of
Object.keys
do we need to callkeys
injasmineUtils.ts
to include enumerable symbols (and maybe filter outSymbol.iterator
in rare case it might be iterable in user-defined object) so that the comparison of properties is as consistent with ordinaryequals
call? I am asking, not telling.This illustrates a limitation we have recently noticed: it is not (yet) possible for custom tester to know additional arguments for property comparison which distinguish
toEqual
fromtoStrictEqual
matcher. Ignore that problem.