FormidableLabs / react-fast-compare

fastest deep equal comparison for React
MIT License
1.59k stars 54 forks source link

Handle anonymous function #45

Closed Cwanyo closed 5 years ago

Cwanyo commented 5 years ago

I use this package in React to detect changes in props. And, I came across a problem that it could not detect the changes of the anonymous function that passed in the props.

Test Case:

{
  description: "same anonymous function is equal",
  value1: () => {
    console.log("func3");
  },
  value2: () => {
    console.log("func3");
  },
  equal: true
},
{
  description: "different anonymous functions are not equal",
  value1: () => {
    console.log("func3");
  },
  value2: () => {
    console.log("func4");
  },
  equal: false
}

To solve this problem, I added the below code before the end of return

  // check for the function
  var funcA = a instanceof Function,
    funcB = b instanceof Function;
  if (funcA != funcB) return false;
  if (funcA && funcB) return a.toString() == b.toString();

  return a !== a && b !== b;

It seem to work fine. But I would like to know if there is a better way to handle this case?

ryan-roemer commented 5 years ago

Unfortunately, toString() is going to run into limitations where the scope of the variables in the function can be literally the same yet very different.

This is a contrived example using classes, but could well translate to more real-world situations where a literal matches of fn.toString() can be identical but should be treated differently.

class Foo {
  getName() {
    return this.constructor.name;
  }
}

class Bar {
  getName() {
    return this.constructor.name;
  }
}

const isEqual = (a, b) => {
  var funcA = a instanceof Function,
      funcB = b instanceof Function;

  if (funcA != funcB) return false;
  if (funcA && funcB) return a.toString() == b.toString();

  return a !== a && b !== b;
};

const foo = new Foo();
console.log("foo", foo.getName());
const bar = new Bar();
console.log("bar", bar.getName());

// Is `true`. Should be `false.
console.log("isEqual", isEqual(foo.getName, bar.getName));
console.log("isEqual", isEqual(Foo.prototype.getName, Bar.prototype.getName));
chrisbolin commented 5 years ago

@Cwanyo thanks for helping to make the library better.

In this case the current behavior is intended. There is no way to "deep compare" functions, as is possible with objects.

As @ryan-roemer pointed out, functions can have different contexts, and therefore must be compared by reference, not "by value" (e.g. by their stringified representations).

var equal = require("react-fast-compare")

const f0 = () => null;
const f1 = () => null;
const f0copy = f0;

console.log("f0 =? f0copy", equal(f0, f0copy)); // TRUE, these are the same function instance
console.log("f0 =? f0copy", equal(f0, f1)); // FALSE, these are two different functions

it could not detect the changes of the anonymous function that passed in the props

I'm guessing you mean this library over-estimates changes, meaning it shows functions as unequal when you conceive of them as being equal; is that right? This might be because you are creating anonymous functions on every render, which you probably want to avoid: https://medium.com/@oleg008/arrow-functions-in-react-f782d11460b4