Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix: expect.toMatchObject ignores symbol key properties #13639

Merged
merged 5 commits into from
Dec 31, 2022

Conversation

lpizzinidev
Copy link
Contributor

Fix #13638

Summary

Using Reflect.ownKeys() for listing an object keys allows the comparison of Symbols as well.

Copy link
Member

@SimenB SimenB left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks!

@SimenB SimenB merged commit fb37bd1 into jestjs:main Dec 31, 2022
@lpizzinidev lpizzinidev deleted the fix-13638 branch December 31, 2022 10:50
joshkel added a commit to joshkel/jest that referenced this pull request Jan 24, 2023
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
@github-actions
Copy link

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.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 31, 2023
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: expect.toMatchObject ignores symbol key properties
3 participants