Skip to content

Commit

Permalink
Include symbol keys when diffing objects
Browse files Browse the repository at this point in the history
As a result of jestjs#13639, `expect.toMatchObject` can now compare symbol keys.  However, the diffs that it generates if a symbol key doesn't match are misleading.  For example, the following assertion:

```
const Foo = Symbol("foo");
test("mismatched symbols", () => {
  expect({ a: 1, [Foo]: {id: 1} }).toMatchObject({ a: 1, [Foo]: {id: 2} });
});
```

fails as desired, but it displays the following diff:

```
    - Expected  - 3
    + Received  + 0

      Object {
        "a": 1,
    -   Symbol(foo): Object {
    -     "id": 2,
    -   },
      }
```

*Note*: In inspecting the code, I wonder if `getObjectSubset` should use the same logic as `subsetEquality` - i.e., if `subsetEquality` does not evaluate an object's inherited string keys when determining equality, then `getObjectSubset` should not evaluate an object's inherited string keys for reporting on inequality? To put it another way - jestjs#13639 appears to change `subsetEquality` from considering an object's inherited string keys to not considering inherited string keys, and I'm not sure if that was intentional in a SemVer-minor change.  For now, I've preserved the previous behavior, but let me know if this should change.

Fixes jestjs#13809
  • Loading branch information
joshkel committed Jan 24, 2023
1 parent 3d7a096 commit 740a8d3
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 4 deletions.
15 changes: 13 additions & 2 deletions packages/expect-utils/src/utils.ts
Expand Up @@ -43,6 +43,17 @@ const hasPropertyInObject = (object: object, key: string | symbol): boolean => {
);
};

// Retrieves an object's keys for evaluation by getObjectSubset. This evaluates
// the prototype chain for string keys but not for symbols. (Otherwise, it
// could find values such as a Set or Map's Symbol.toStringTag, with unexpected
// results.)
//
// Compare with subsetEquality's use of Reflect.ownKeys.
const getObjectKeys = (object: object) => [
...Object.keys(object),
...Object.getOwnPropertySymbols(object),
];

export const getPath = (
object: Record<string, any>,
propertyPath: string | Array<string>,
Expand Down Expand Up @@ -131,7 +142,7 @@ export const getObjectSubset = (
const trimmed: any = {};
seenReferences.set(object, trimmed);

Object.keys(object)
getObjectKeys(object)
.filter(key => hasPropertyInObject(subset, key))
.forEach(key => {
trimmed[key] = seenReferences.has(object[key])
Expand All @@ -144,7 +155,7 @@ export const getObjectSubset = (
);
});

if (Object.keys(trimmed).length > 0) {
if (getObjectKeys(trimmed).length > 0) {
return trimmed;
}
}
Expand Down
Expand Up @@ -4140,13 +4140,13 @@ exports[`toMatchObject() {pass: false} expect({"a": "a", "c": "d"}).toMatchObjec
exports[`toMatchObject() {pass: false} expect({"a": "b", "c": "d", Symbol(jest): "jest"}).toMatchObject({"a": "c", Symbol(jest): Any<String>}) 1`] = `
<d>expect(</><r>received</><d>).</>toMatchObject<d>(</><g>expected</><d>)</>

<g>- Expected - 2</>
<g>- Expected - 1</>
<r>+ Received + 1</>

<d> Object {</>
<g>- "a": "c",</>
<g>- Symbol(jest): Any<String>,</>
<r>+ "a": "b",</>
<d> Symbol(jest): Any<String>,</>
<d> }</>
`;

Expand Down

0 comments on commit 740a8d3

Please sign in to comment.