sveltejs / svelte

web development for the rest of us
https://svelte.dev
MIT License
79.97k stars 4.25k forks source link

Svelte 5: Missing warning for non-state use in `$effect`? #12142

Open brunnerh opened 4 months ago

brunnerh commented 4 months ago

Describe the bug

When a non-state value that is assigned is used in the template, you get a warning:

value is updated, but is not declared with $state(...). Changing its value will not correctly trigger updates (non_reactive_update)

Could/should such a warning be shown if the value is only used in an $effect?

(Related/found via: #12141)

Reproduction

<script>
  import { onMount } from 'svelte';

  let value;

  $effect(() => {
    console.log(value);
  });

  onMount(() => {
    value = 1;
  });
</script>

REPL

Logs

No response

System Info

REPL

Severity

annoyance

FoHoOV commented 4 months ago

If it is decided to have warning, it shoudn't be for usages of none-reactive values in effects/deriveds. IMHO a warning could be there, only if there are 0 reactive values inside an effect/derived. For example an effect could depend on only 1 reactive value, but use 10 other variables that are not reactive which is fine. If nothing in the effect, triggers a rerun, then there could be a warning. Also one might use $effect just for onMount and then cleanup logic. Thoughts?

MotionlessTrain commented 4 months ago

If it is decided to have warning, it shoudn't be for usages of none-reactive values in effects/deriveds

The warning is to make sure you don't forget assigning them with $state, if they were meant to be reactive. That is also why it is a warning, and not an error, as it is fine to use those if you don't need the reactive dependency

brunnerh commented 4 months ago

The question is whether this would lead to many false positives or not. To evaluate that, testing the warning on real world code would probably be helpful.

paoloricciuti commented 4 months ago

The question is whether this would lead to many false positives or not. To evaluate that, testing the warning on real world code would probably be helpful.

I think i'm with @FoHoOV on this one...there are reasons to use non stateful variables in an effect. The error should really just be thrown if that's the only dependency but this also means that the first function call should mute this error. Maybe even property access.

brunnerh commented 4 months ago

I suspect you both overestimate how often the warning would trigger - it is very specific.

The variable has to be...

E.g. constants and imports would not be affected.

How often do you really want to change a local value, use it in an $effect and not have the effect be triggered by that change?