tc39 / proposal-array-equality

Determining Array Equality
MIT License
70 stars 4 forks source link

Consider `Object.isEqual(a, b)` or similar instead #5

Open dead-claudia opened 3 years ago

dead-claudia commented 3 years ago

I feel very strongly that deep equality should be performed with a static function, not an array method or anything similar. Library and language precedent strongly argues in favor of this model:

dead-claudia commented 3 years ago

BTW, Java's java.util.Arrays.equals delegates to a[i] != null ? a[i].equals(b[i]) : b[i] == null as its comparison, and coincidentally, this is literally what Kotlin desugars a[i] == b[i] to, relegating Java's equality operator's behavior to ===.

ljharb commented 3 years ago

The array method isn’t that useful without a protocol, and a protocol without a generic method (like on Object) is also not that useful. It seems reasonable to me to provide this kind of static method.

L3P3 commented 3 years ago

I think that static method should also have a 3rd argument which is for deepness, true by default. This approach would be similar to that of cloneNode. Would be useful because sometimes one wants to test for any data changes, like in JSON.stringify(a) === JSON.stringify(B) and in most web frameworks, things like props and state objects have to be compared where it would make much more sense to not deeply compare these objects, due to performance reasons.

dead-claudia commented 3 years ago

@L3P3 That could be done with an Object.isShallowEqual, but I could see the utility of it - React uses shallow equality IIRC for its React.memo.

L3P3 commented 3 years ago

I also agree with #6 so Object.isEqual should use that implementation for the used type, early-returning false when their type does not match up. Edit: Might not work like that. Then, I don't see much of a point for this static method when == could also be used. The only upside of this static method approach is that deepness can be specified in my opinion. Or should there be another, new operator for shallow equality?

dead-claudia commented 3 years ago

@L3P3

Then, I don't see much of a point for this static method when == could also be used

The chances of that operator being modifiable in any way is basically zero. Plus, its semantics are not useful - I don't want Object.isShallowEqual(1, "1") to evaluate to true, and I don't think anyone else here wants that, either (that's the semantics it provides). Plus, {} == {} evaluates to false, which is explicitly not what anyone wants. So let's not go that route. πŸ™‚

dead-claudia commented 3 years ago

As for any operator, I feel it's a little premature for that. The proposal's very, very stage 1, and even my #6 is probably premature. (I wasn't paying close enough attention to how close it was to stage 2, or I would've probably worded that a little differently.)

L3P3 commented 3 years ago

Yes, lets abandon == into legacy. :grin: But what about Object.isEqual and Object.isShallowEqual being just a shim for returning false on type mismatch and then calling type[Symbol.equals](a, b, bool deep)?

dead-claudia commented 3 years ago

Have you...tried using that kind of function in any sort of demo code? I highly recommend it - you might be enlightened. πŸ˜‰

L3P3 commented 3 years ago

Actually no and I have no idea what function you mean. None of these are existing in the spec so far, right? And in case my definition was too short, I think of these:

Object.isEqual = (a, b) => {
  if (typeof a !== typeof b) return false;
  if (a === b) return true;
  if (a === null || b === null) return false;
  if (a.constructor !== b.constructor) return false;
  return Boolean(a.constructor.prototype[Symbol.equals].call(a, b, true));
}
Object.isShallowEqual = (a, b) => {
  if (typeof a !== typeof b) return false;
  if (a === b) return true;
  if (a === null || b === null) return false;
  if (a.constructor !== b.constructor) return false;
  return Boolean(a.constructor.prototype[Symbol.equals].call(a, b, false));
}
L3P3 commented 3 years ago

I might totally miss something huge. I have no experience with JS spec drafts. :wink:

L3P3 commented 3 years ago

In my example, I think there is too much redundancy so I propose to ditch isShallowEqual and instead have it like this:

Object.isEqual = (a, b, deep = true) => {
  if (typeof a !== typeof b) return false;
  if (a === b) return true;
  if (a === null || b === null) return false;
  if (a.constructor !== b.constructor) return false;
  return Boolean(a.constructor.prototype[Symbol.equals].call(a, b, deep));
}
ljharb commented 3 years ago

The problem this proposal is trying to solve requires deep - and only deep - equality. There won't be any way to do "shallow" equality beyond a user object defining a protocol method that has those semantics.

C-Ezra-M commented 2 years ago

It's also recommended that this method should employ detection of circular references, or else it will run into an infinite loop when it finds a self-referential object.

In my example, I think there is too much redundancy so I propose to ditch isShallowEqual and instead have it like this:

Object.isEqual = (a, b, deep = true) => {
  if (typeof a !== typeof b) return false;
  if (a === b) return true;
  if (a === null || b === null) return false;
  if (a.constructor !== b.constructor) return false;
  return Boolean(a.constructor.prototype[Symbol.equals].call(a, b, deep));
}

I thought of replacing the checks if an object is null with loose equality because null == undefined is true (but null === undefined is false), but since undefined and null have a different output from the typeof operator (typeof undefined is "undefined", while typeof null is "object"), this isn't necessary.

C-Ezra-M commented 2 years ago

It's also recommended that this method should employ detection of circular references, or else it will run into an infinite loop when it finds a self-referential object.

I actually doubt if it's necessary because the following example Python program throws a RecursionError in the end:

mydict1 = {'this': 'that'}
mydict2 = {**mydict1}
print(mydict1 == mydict2) # true
print(mydict1 is mydict2) # false, `is` keyword checks memory references

mydict1['self'] = mydict1
mydict2['self'] = mydict2

print(mydict1 == mydict2) # throws a RecursionError due to circular references

Object.isEqual() could then look like this:

Object.isEqual = (a, b, deep = true) => {
  if (Object.is(a, b)) return true;
  if (a === b) return true;
  if (typeof a !== typeof b) return false;
  if (a === null || b === null) return false;
  if (a.constructor !== b.constructor) return false;
  return Boolean(a.constructor.prototype[Symbol.equals].call(a, b, deep));
}

This was supposed to call for Number.isNaN() as a more robust alternative to the global function isNaN(), which I initially considered instead of Object.is(), which does treat NaN as equal to itself and Number.NaN.

The === equality check is still required because Object.is(0, -0) is false, even though Object.is(0n, -0n) is true.

Also, checking typeof a !== typeof b was removed because === already performs the type check. EDIT: Added it back.

cben commented 1 year ago

This is interesting discussion, but I just want to point out the current README completely fails to explain what kind of comparison is to be done on the elements. If it's still fuzzy, I recommend saying so, and listing required properties to clarify the design space being explored...

Properties that I might currently guess from the README:

  1. It should support deep equality on arrays containing Arrays. β€” mentioned in Motivation, plus [1, [2, [3,4]]] example)
  2. It should support deep equality on arrays containing Objects. β€” [{ foo: 'bar' }] example (technically ambiguous β€” is it false because different, or false because doesn't recurse on objects?).
  3. It works by calling a[i].equals(b[i])?? A natural guess given it's available as an Array method, but would not support Objects, Maps etc. unless you give them same method?
    • Do Strings, Numbers, undefined etc. get same method, or do you specify a fallback algorithm when method is missing (e.g. fallback to === would cover strings and numbers, though NaN is a nuisance).

The last point has a critical consequence β€” can this comparison be redefined by providing your own method? (This I guess is what @ljharb meant by "protocol")

@dead-claudia I'm not sure if your opening question here was merely about how end-user accesses this β€”Β f(a1, a2) vs a1.equals(a2) β€” or also about hard-coded algorithm vs. extensible protocol?
It is tempting but not mandatory for these two questions to correlate!

dead-claudia commented 1 year ago

@dead-claudia I'm not sure if your opening question here was merely about how end-user accesses this β€”Β f(a1, a2) vs a1.equals(a2) β€” or also about hard-coded algorithm vs. extensible protocol?

It is tempting but not mandatory for these two questions to correlate!

@cben I think you misread my intent completely. I was saying that 1. any array equality method should be shallow and 2. any deep equality should use a static method. Nothing here is about the precise algorithm of the latter, only about the way end users invoke it.