Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reference comparison #84

Open
Bassel-Bakr opened this issue Aug 27, 2023 · 1 comment
Open

Reference comparison #84

Bassel-Bakr opened this issue Aug 27, 2023 · 1 comment

Comments

@Bassel-Bakr
Copy link

Bassel-Bakr commented Aug 27, 2023

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
@Tao-VanJS
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants