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

[bug] deepEqual breaking change v5.0.0 #508

Closed
robertsLando opened this issue Apr 29, 2020 · 2 comments
Closed

[bug] deepEqual breaking change v5.0.0 #508

robertsLando opened this issue Apr 29, 2020 · 2 comments

Comments

@robertsLando
Copy link

With version 5.0.0 all my deepEqual fails. The reason is: d26c6a0#diff-dd2e881af3311f834199c7b515cff0c7R501

Here you set strict:true but that's wrong because with that test like this will fail:

expected: |-
        [ { cmd: 'publish', brokerId: 'broker-42', brokerCounter: 42, topic: 'hello', payload: <Buffer 77 6f 72 6c 64>, qos: 1, retain: false, dup: false } ]
      actual: |-
        [ { cmd: 'publish', brokerId: 'broker-42', brokerCounter: 42, topic: 'hello', payload: <Buffer 77 6f 72 6c 64>, qos: 1, retain: false, dup: false } ]
@robertsLando robertsLando changed the title [bug] Deepequal breaking chande [bug] Deepequal breaking change Apr 29, 2020
@robertsLando robertsLando changed the title [bug] Deepequal breaking change [bug] deepEqual breaking change v5.0.0 Apr 29, 2020
@ljharb
Copy link
Collaborator

ljharb commented Apr 29, 2020

v5.0.0 is a semver-major, breaking changes are expected.

Setting strict is not "wrong", it matches what node has done since like v4. If you want loose equality, use https://github.com/substack/tape#tdeeplooseequalactual-expected-msg; if you're curious to know why those two objects are considered different, i'd try require('is-equal/why') from https://npmjs.com/is-equal, and that will return a string describing the difference.

@robertsLando
Copy link
Author

I didn't see deepLooseEqual, I always just used deepEqual and I saw everything was broken so I thought it was a mistake :) Thanks @ljharb

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants