Skip to content
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 subsetEquality: same referenced object on same level node of tree is regarded as circular reference #9322

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -47,6 +47,7 @@
- `[expect]` Avoid incorrect difference for subset when `toMatchObject` fails ([#9005](https://github.com/facebook/jest/pull/9005))
- `[expect]` Consider all RegExp flags for equality ([#9167](https://github.com/facebook/jest/pull/9167))
- `[expect]` [**BREAKING**] Consider primitives different from wrappers instantiated with `new` ([#9167](https://github.com/facebook/jest/pull/9167))
- `[expect]` Fix subsetEquality false circular reference detection ([#9322](https://github.com/facebook/jest/pull/9322))
- `[jest-config]` Use half of the available cores when `watchAll` mode is enabled ([#9117](https://github.com/facebook/jest/pull/9117))
- `[jest-config]` Fix Jest multi project runner still cannot handle exactly one project ([#8894](https://github.com/facebook/jest/pull/8894))
- `[jest-console]` Add missing `console.group` calls to `NullConsole` ([#9024](https://github.com/facebook/jest/pull/9024))
Expand Down
13 changes: 13 additions & 0 deletions packages/expect/src/__tests__/utils.test.js
Expand Up @@ -333,6 +333,19 @@ describe('subsetEquality()', () => {
expect(subsetEquality(primitiveInsteadOfRef, circularObjA1)).toBe(false);
});

test('referenced object on same level should not regarded as circular reference', () => {
const referencedObj = {abc: 'def'};
const object = {
a: {abc: 'def'},
b: {abc: 'def', zzz: 'zzz'},
};
const thisIsNotCircular = {
a: referencedObj,
b: referencedObj,
};
expect(subsetEquality(object, thisIsNotCircular)).toBeTruthy();
});

test('transitive circular references', () => {
const transitiveCircularObjA1 = {a: 'hello'};
transitiveCircularObjA1.nestedObj = {parentObj: transitiveCircularObjA1};
Expand Down
16 changes: 10 additions & 6 deletions packages/expect/src/utils.ts
Expand Up @@ -166,7 +166,6 @@ export const iterableEquality = (
if (a.constructor !== b.constructor) {
return false;
}

let length = aStack.length;
while (length--) {
// Linear search. Performance is inversely proportional to the number of
Expand Down Expand Up @@ -290,20 +289,25 @@ export const subsetEquality = (

return Object.keys(subset).every(key => {
if (isObjectWithKeys(subset[key])) {
if (seenReferences.get(subset[key])) {
if (seenReferences.has(subset[key])) {
return equals(object[key], subset[key], [iterableEquality]);
}
seenReferences.set(subset[key], true);
}

return (
const result =
object != null &&
hasOwnProperty(object, key) &&
equals(object[key], subset[key], [
iterableEquality,
subsetEqualityWithContext(seenReferences),
])
);
]);
// The main goal of using seenReference is to avoid circular node on tree.
// It will only happen within a parent and its child, not a node and nodes next to it (same level)
// We should keep the reference for a parent and its child only
// Thus we should delete the reference immediately so that it doesn't interfere
// other nodes within the same level on tree.
seenReferences.delete(subset[key]);
return result;
});
};

Expand Down