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

[v5 Runes] Re-assigning NaN to an already NaN variable triggers update #10061

Open
Malix-Labs opened this issue Jan 2, 2024 · 5 comments
Open

Comments

@Malix-Labs
Copy link

Malix-Labs commented Jan 2, 2024

Describe the bug

  1. In v5 Runes, re-assigning NaN to an already NaN variable triggers update
  2. Inconsistency between v5 Runes and v5 Classic:
    In v5 Classic (and v4), re-assigning NaN to an already NaN variable doesn't trigger update

Reproduction

https://svelte-5-preview.vercel.app/#H4sIAAAAAAAACsWQTWvDMAyG_4owhSYQkkNvbhIYu_ew6zJo6iqrmWMbWxmMkP8-4nw07NCx046SXj3iUc8aqdAz_tozXbfIOHuyliWMvuxY-E9UhCxh3nROjJ3cCyctlZWuSLbWOIJnVXsvBTTOtLBPs7lOp-X9JvnSafRrLlT3VJ7d0Tq_HcqZk2e3Q1npfDmTrYEAWMYTexyyhLXmKhuJV8bJdTgkq90MeWwYWtxYkkZ7cCO46JtaeRyW65svKCTQXXtBdxxHFe04CKO9UZgq8x6dA4DDrp9Swzk-_rS9dERGg9FCSfFR9FEMRTlToYBTfRrCrW1nZEx7vziHz_zZ-KEpFLDzVBNG8SIttbco6L9034Zvw2X5Z8wCAAA=

Logs

No response

System Info

not needed

Severity

annoyance

@Malix-Labs Malix-Labs changed the title [v5 Runes] Re-assigning NaN to an already NaN variable triggers reactivity [v5 Runes] Re-assigning NaN to an already NaN variable triggers update Jan 2, 2024
@Malix-Labs
Copy link
Author

Malix-Labs commented Jan 2, 2024

About the first issue:

  • Comment from @Malix-off:

    • Issue Description from @Rich-Harris:

      I'm pretty sure that's a small enough edge case that we needn't worry about it

      I disagree and think the performance tradeoff is better than the deprecated support for NaN checking before update tradeoff

About the second issue:

I think having a logic inconsistency between v5 Classic and v5 Runes could be misleading, and thus is not desirable.

@Rich-Harris
Copy link
Member

I stand by my assessment — it would be very wasteful to check for NaN every time. Candidly, if you have NaN somewhere in your state, it's very likely an opportunity for improvement.

@dummdidumm
Copy link
Member

Instead of using === for comparison we could use Object.is which handles this case correctly. I'm not what that would mean for performance though

@Rich-Harris
Copy link
Member

Nothing, according to this microbenchmark, though I'm not sure I trust it — it violates my expectations quite deeply.

image

@Malix-Labs
Copy link
Author

Malix-Labs commented Apr 5, 2024

Quite unexpected for me too

if you have NaN somewhere in your state, it's very likely an opportunity for improvement

By the way, I do agree falling back to NaN is bugs and weird behavior prone, but it is unfortunately the default value for some HTML input elements state (valueAsNumber and sometimes value)

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

3 participants