Redundant Immutable.is() for entire props and state?

[pstoica]

Hey, I wanted to make sure I understood this bit correctly:

  if (objA === objB || is(objA, objB)) {
    return true;
  }

I don't think you can use Immutable.Map as your entire props/state. is would then check for referential equality, just like ===.

[jurassix]

is() is essentially a deep value compare of the Immutable object.

[pstoica]

Sorry, here's what I mean. I could be missing something, let me know:

// ImmutableRenderMixin.js
...
  shouldComponentUpdate: function(nextProps, nextState) {
    return !shallowEqualImmutable(this.props, nextProps) ||
           !shallowEqualImmutable(this.state, nextState);
  }
...

// shallowEqualImmutable.js
...
// this receives props/state as objects
function shallowEqualImmutable(objA, objB) {
  // consider state with: {myMap: Immutable.Map}
  // you might not always reuse references for props and state even if they're shallow equal

  // Immutable.is tries === first, and only does further work if it's an immutable object
  // Immutable.is(objA, objB) would only be true if objA === objB
  // additionally, Immutable.is('a', 'a') works since 'a' === 'a'

  if (objA === objB || is(objA, objB)) {
    return true;
  }
...

// Immutable.js console
var map = Immutable.Map({a: 1})
Immutable.is({myMap: map}, {myMap: map});
-> false
var a = {myMap: map};
Immutable.is(a, a);
-> true

[jurassix]

The problem with your assertions is that your creating two Javascript (not immutable) objects and calling Immuable.is(), which should be false.

var a = {}
var b = {}
Immutable.is(a, b)
-> false

Eventually, when we actually process each key in the javascript object, the values we will in fact satisfy our equality if they are both equal using is().

if (!bHasOwnProperty(keysA[i]) || !is(objA[keysA[i]], objB[keysA[i]])) 

First we compare the top level objects (which could be a mixed bag of Immutable or Javascript objects

var map = Immutable.Map({a: 1});
var objA = {myMap: map};
var objB = {myMap: map};
objA === objB
-> false
Immutable.is(objA, objB);
-> false

Second if we have not found equality yet we pull the keys from each object and compare the values at the keys:

var map = Immutable.Map({a: 1});
var objA = {myMap: map};
var objB = {myMap: map};
Immutable.is(objA.myMap, objB.myMap);
-> true

[jurassix]

Ok so I think I understand the original issue; is the === equal to the is() since props/state objects are essentially javascript objects with some Immutable values. Let me think about possible edge cases here. We may be able to remove the is() call; I'll check and see what the source looks like in Immutable to see how expensive this call is in this case.

[pstoica]

FWIW the file used to just run is() until it was reformatted to look like the original Facebook code and someone added the extra ===. I think they're completely equivalent, but I guess it's worth checking the source.