False positives comparing equality of Maps/Sets with core-js

[mjackson]

Using v1.4.0, I'm seeing a few false positives in Safari 7-8 and IE 9-11. In the tmp branch of expect I pushed a commit that demonstrates the problem. You can see the failed build here.

Basically, in these environments, I'm seeing the following:

isEqual({ a: 1 }, { a: 2 }) // true
isEqual({ c: 2 }, { a: 2 }) // true

However, both of these values are (correctly) false in Chrome 39 and Firefox 32.

[mjackson]

I'm not 100% sure, but it seems like this is a dup of #3.

[ljharb]

I'm using those test cases, and it's working for me in Safari 7 and 8, and IE 9 - 11 :-/

[ljharb]

Is there any possibility that in your tests, you've got something that modifies globals?

[mjackson]

Is there any possibility that in your tests, you've got something that modifies globals?

Yes, there is. I'm using Babel's polyfill to get Map/Set support in those browsers. That's probably the culprit.

Either:

a) I should just skip Map/Set tests in environments that don't support them (probably easiest) or
b) we should make sure is-equal is ok with the Babel polyfill

I'm open to either.

[mjackson]

ok, I just pushed a commit that only runs tests for Map and Set in environments that support them natively (no polyfill). However, it looks like I've still got a problem in IE11 where this test is failing with Sets.

From what I can tell, it looks like

isEqual(new Set('a'), new Set('b')) === true

in IE11. Can you please confirm?

[ljharb]

OK, so, core-js (babel's polyfill) definitely does slightly different things than es6-shim's (which is what is-equal is tested with) - they're not fully spec-compliant. I can definitely try to add tests though to make is-equal work with those.

Regarding IE 11, this is actually a browser bug - new Set('a').size === 0 is true in IE 11, so both of your sets are empty. There's enough browser bugs with passing an iterable in both Map and Set that you simply can not rely on it working, unless of course you're using the es6-shim. If you change your tests to var a = new Set(); a.add('a'); var b = new Set(); b.add('b'); isEqual(a, b) === true will pass as expected.

[mjackson]

Thanks for the tip. I've updated my Set tests accordingly and everything's green in IE 11. Looks like is-equal just needs compat with core-js.

[ljharb]

@mjackson I've narrowed down the issue to a bug in core-js, and filed the above issue to attempt to resolve it.

[Poplava]

I found a similar problem in [email protected]

Problem is that Phantom is not throwing an error while calling Map.prototype.forEach with object as first arg.

https://github.com/ljharb/is-equal/blob/master/index.js#L68

And the tryMapSetEntries returns an empty array instead of false.

It looks like PhantomJS issue, but may be it will help you.

[ljharb]

@Poplava phantomjs 1.9 has Map and Set? I thought it didn't even have "bind". Are you using core-js with it?

[Poplava]

@ljharb I didn't use core-js. I've just realized, Map and Set may come from babel-polyfill.

[ljharb]

@Poplava babel-polyfill is core-js.

[Poplava]

@ljharb oh, really! I'm sorry... I confused.

[ljharb]

@mjackson v1.4.1 is released - this will ensure both that core-js doesn't cause false positives due to Map/Set support, and also that when zloirock/core-js#144 is fixed, core-js Maps and Sets should just automatically start working with is-equal.

[mjackson]

👍 Thanks for the quick turnaround on this, @ljharb