Circular reference object equality is not symmetrical

[sz-piotr]

🐛 Bug Report

Circular structure comparison is wrong.

The algorithm for determining object equality is asymmetric and causes problems. Pretty sure that the following lines create the problem:
https://github.com/facebook/jest/blob/fe19d6978858d76e1cb7544b39a56250a18b2103/packages/expect/src/jasmineUtils.js#L150-L157

To Reproduce

it('circular structures', () => {
    const a = {}
    a.x = { x: a }
    const b = {}
    b.x = b
    expect(a).toEqual(b) // passes
    expect(b).toEqual(a) // fails
})

Expected behavior

Both tests should pass or both tests should fail - design decision

Link to repl or repo (highly encouraged)

https://repl.it/repls/VagueRashBraces

Run npx envinfo --preset jest

Paste the results here:

  System:
    OS: Linux 4.19 Manjaro Linux undefined
    CPU: (8) x64 Intel(R) Core(TM) i7-2600 CPU @ 3.40GHz
  Binaries:
    Node: 11.4.0 - ~/.nvm/versions/node/v11.4.0/bin/node
    Yarn: 1.12.3 - /usr/bin/yarn
    npm: 6.4.1 - ~/.nvm/versions/node/v11.4.0/bin/npm
  npmPackages:
    jest: ^23.6.0 => 23.6.0

[grosto]

I can take this one :)

thanks for a very clear description

[grosto]

I've been quite busy for past month, so only now I managed to get back to the issue.

Above case breaks in every popular library(underscore, lodash, ramda). Since Jest's equality is based on underscore's implementation, it has the same problem. Only node.js equality is consistent with this case. ramda returns true for this case but if you add one level of nesting, it returns false.

You can check out an example here.

We can solve the issue this way, by adding another check

if (bStack[length] == b) {
   return aStack[length] == a;
}

This is a very primitive solution, but it works. I tried some other options, but this seemed the most reasonable to implement for now. One of them was to copy Node's assert.deepEqual() implementation

@SimenB could you please give your opinion on the issue?

[jeysal]

Wow this is an interesting one :D
Wondering what the reasoning for the choices Ramda / node made might be. My intuition would be that it should pass, since toEqual does not care about reference equality

[SimenB]

I don't really have an opinion on a solution, beyond agreeing it's a bug. I'm not sure if the bug is expect(a).toEqual(b) or expect(b).not.toEqual(a), though :P Probably the second one? They are structurally equals, which is what toEqual should test. I think

[grosto]

I see.
I have been thinking too whether they are structurally equal or not.

a =  {
 x: {
   x: [Circular]
 }
}

b = {
 x: [Circular]
}

if you open circular, the depth of a will increase twice, and depth of b will increase by one. Also they will produce different hashes. I will try to research more.

I will probably open the issues in other libraries and hopefully will get some ideas :)

[github-actions]

This issue 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.