Jest: Immutable.Map comparison with not the same key order

[davidfou]

Do you want to request a feature or report a bug?
bug

What is the current behavior?
Comparison between to identical Immutable.Map() but declared without the same key order fails.

If the current behavior is a bug, please provide the steps to reproduce and either a repl.it demo through https://repl.it/languages/jest or a minimal repository on GitHub that we can yarn install and yarn test.

What is the expected behavior?
All the specs on this example should pass.

describe('comparison with Immitable.Map', () => {
  const map1 = Immutable.Map({ a: 1, b: 2 });
  const map2 = Immutable.Map({ b: 2, a: 1 });

  it('should ignore key order with Immutable.is', () => {
    expect(Immutable.is(map1, map2)).toBe(true);
  });

  it('should ignore key order with Jest', () => {
    expect(map1).toEqual(map2);
  });
});

[SimenB]

The behavior is consistent with normal JS Map.

describe('comparison with Map', () => {
  const map1 = new Map();
  const map2 = new Map();

  map1.set('a', 1);
  map1.set('b', 2);
  map2.set('b', 2);
  map2.set('a', 1);

  it('should ignore key order with Jest', () => {
    expect(map1).toEqual(map2);
  });
});

(We could probably start rendering "visual differences" here)

It's the same for plain arrays.

// Fails
expect(['foo', 'bar']).toEqual(['bar', 'foo']);

But objects don't care about insertion order (which is not really specced, so makes sense)

// Passes
expect({ a: 1, b: 2 }).toEqual({ b: 2, a: 1 });

I'm not sure what the correct solution is, but it feels like we should be more consistent between Maps and Objects?

This feels weird:

  it('should treat objects, and arrays of their keys, consistently', () => {
    const obj1 = { a: 1, b: 2 };
    const obj2 = { b: 2, a: 1 };

    // Passes
    expect(obj1).toEqual(obj2);
    // Fails
    expect(Object.keys(obj1)).toEqual(Object.keys(obj2));
  });

_{Put the parts about arrays in detail blocks, not really related to the issue at hand}

[leebyron]

Map is not ordered
On Sat, Oct 7, 2017 at 7:45 AM Simen Bekkhus [e-mail]
wrote:

The behavior is consistent with normal JS Map.

describe('comparison with Map', () => {
const map1 = new Map();
const map2 = new Map();

map1.set('a', 1);
map1.set('b', 2);
map2.set('b', 2);
map2.set('a', 1);

it('should ignore key order with Jest', () => {
expect(map1).toEqual(map2);
});
});

[image: image]
https://user-images.githubusercontent.com/1404810/31307491-5a10f014-ab65-11e7-8885-48c3031b011b.png

It's the same for plain arrays.

// Failsexpect(['foo', 'bar']).toEqual(['bar', 'foo']);

[image: image]
https://user-images.githubusercontent.com/1404810/31307425-9dec3614-ab64-11e7-838b-255f446031ce.png

But objects don't care about insertion order

// Passesexpect({ a: 1, b: 2 }).toEqual({ b: 2, a: 1 });

I'm not sure what the correct solution is, but it feels like we should be
more consistent?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/facebook/jest/issues/4618#issuecomment-334929424, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AADD0kT6Mqjyx7zsQU1aFHGyNgrm5m4bks5sp2RigaJpZM4Pw2vL
.

[SimenB]

Indeed, which makes me think that both Maps from Immutable and standard JS Maps should behave like Object when checking equality. That's up to @cpojer though.

(we can safely ignore what I said about arrays, it's not relevant here. I'll put it in details)

[cpojer]

This is tricky because Jest doesn't have knowledge on how to sort two iterables to make them comparable. I think I'd be in favor of making toEqual work though if it isn't too big of a deal in the equality matcher.

[davidfou]

Here my 2 cents: why not first use Immutable.is then fallback to current behaviour only if it's false?

[SimenB]

Feels icky to have a dependency on immutable js in a generic assertion library. You can create your own assertion for that (and plug it in with expect.extend)

[davidfou]

That's right. So toEqual only use the output from pretty-format? Maybe this package should reorder on all unordered types recursively?

const initialMap = Immutable.Map({b: 1, a: 2});
const orderedMap = initialMap.toSeq().sortBy((v, k) => k).toMap();
Immutable.Map({b: 1, a: 2}).toSeq().sortBy((v, k) => k).toMap()

POC

[SimenB]

we do a deep equality (which doesn't know about immutable js), pretty-format is used to try to show a pretty diff afterwards

Again, I'd suggest to do create a custom matcher. See e.g. https://github.com/jest-community/jest-extended for lots of examples on how to create them

[seanli3]

The JS Map test given by @SimenB passes on the new Jest.

describe('comparison with Map', () => {
  const map1 = new Map();
  const map2 = new Map();

  map1.set('a', 1);
  map1.set('b', 2);
  map2.set('b', 2);
  map2.set('a', 1);

  it('should ignore key order with Jest', () => {
    expect(map1).toEqual(map2);
  });
});

Jest: 21.2.1
Node: 8.9.1
Yarn: 1.3.2

Should we make it consistent for Immutable Map?

[cpojer]

Yeah, we should make it consistent if possible.

[enrique-ramirez]

Hi!

I encountered this issue today and wanted to upvote this so it has a bit more priority. So, thanks!

[SimenB]

PR welcome 🙂

[robinpokorny]

I want to implement this.

What is the correct (or sufficient) way to say a structure is ImmutableJS?

1. It has a sentinel - as we do in pretty-format.
2. It has equals and hashCode functions (cf. isValueObject).
3. Both conditions above.

[SimenB]

I'd say that whatever pretty-format does is good enough

[robinpokorny]

Current status: I tried to use method equals which every Immutable structure has. However, it does not perform deep equality :(

[enrique-ramirez]

@robinpokorny Hi Robin!

I believe that the best way to compare two immutable objects is to use is()

It basically compares a hash based on the contents of the object (so if any value is different, the hash will also be different), rather than the actual object, so it's faster than an actual deep comparison.

[robinpokorny]

@enrique-ramirez, hi!

Internally, .equals and Immutable.is are the same. The problem is that isEqual should perform a deep equality.