van icon indicating copy to clipboard operation
van copied to clipboard

Reference comparison

Open Bassel-Bakr opened this issue 2 years ago • 1 comments
trafficstars

The code uses !== to compare old and new values. The problem is, if at least one of the values is NaN the condition will always be true

NaN !== 0; // is true
NaN !== NaN; // is also true!

We can fix the problem by using Object.is:

Object.is('0', 0); // false
Object.is(0, -0); // false
Object.is(NaN, 0); // false
Object.is(NaN, NaN); // true

Bassel-Bakr avatar Aug 27 '23 20:08 Bassel-Bakr

Thanks @Bassel-Bakr for the feedback!

Here are my 2 cents on the issue:

  1. Neither !== or !Object.is is perfect solution. For !==, we have NaN !== NaN being true. For !Object.is, we have !Object.is(0, -0) being true.
  2. This is probably not a significant issue. The worst outcome is we would fail to skip propagating state changes and re-render DOM when we indeed can skip (i.e.: when the new value in the s.val = <new value> call is semantically the same as the existing one). Thus we're just wasting some computation in rare circumstances. This will not cause any semantic issue for apps built with VanJS.
  3. Among 2 options, I personally believe !== is slightly better than !Object.is. We might encounter the comparison between 0 and -0 more often than the comparison between NaN and NaN, as NaN shouldn't usually occur for a semantically correct app.

Tao-VanJS avatar Aug 28 '23 04:08 Tao-VanJS