Svelte 5: false warn (non_reactive_update) var is updated, but is not declared with `$state(...)`

[qupig]

Describe the bug

I believe the issue is related to the incomplete fix for #11269.

When variables are passed as component props, they don't have to be reactive.

Additional issue that still exists: the warn is reported as "warn" in the svelte compiler but "error" in "eslint-plugin-svelte", It prevents CI from passing eslint tests.

Reproduction

Svelte5-REPL

Svelte5-REPL-NEW

Logs

No response

System Info

"svelte": "5.0.0-next.143"

Severity

blocking an upgrade

[paoloricciuti]

Describe the bug

I believe the issue is related to the incomplete fix for #11269.

When variables are passed as component props, they don't have to be reactive.

Additional issue that still exists: the warn is reported as "warn" in the svelte compiler but "error" in "eslint-plugin-svelte", It prevents CI from passing eslint tests.

Reproduction

Svelte5-REPL

Logs

No response

System Info

"svelte": "5.0.0-next.143"

Severity

blocking an upgrade

I don't think this is an issue at all:

you can't really await in the script tag

you could in theory do .then and then assign but then the warning is right: you need state for that or else the component will not be updated.

[qupig]

@paoloricciuti Not relevant at all, the original await function is wrapped in an async function, and the minimal reproduction just simplifies the irrelevant code.

Please note that this warning is also present in your second link!

[paoloricciuti]

The only situation i can see this being a problem is for something like this.

Maybe we can silence the warning if the only changes happens during the script run? At the same time it feels like an unusual enough situation that just disabling the rule could be enough on the user part.

[paoloricciuti]

@paoloricciuti Not relevant at all, the original await function is wrapped in an async function, and the minimal reproduction just simplifies the irrelevant code.

Please note that this warning is also present in your second link!

Yeah but it's a correct warning...if it happens in an async function the value will not be reflected in the prop passed to the component.

[qupig]

@paoloricciuti What about a even simpler example.

And more realistic example of what you want, I'm crazy.

There are a million possibilities out there for how users write code, but you should always only report errors in the right place, not estimate or enumerate every scenario!

If it's passed as component props and not used in the DOM (svelte conditions etc.) in the components, you should never report an warn or error!

And disabling the rule is even not an option at all here!

[paoloricciuti]

If it's passed as component props and not used in the DOM (svelte conditions etc.) in the components, you should never report an warn or error!

I literally showed you a case where the warning is effective. And yeah you can do all sort of sheningan but this doesn't mean that we should silence a useful warning and possibly have false positives. If you pass that down to a component you expect that component to receive an updated prop when the state is mutated and this doesn't happen unless you do this sort of crazy things.

So i'm in favor of muting the warning if we can detect a pattern that will not yield false negatives but this pattern is not what you suggested.

I'm also taking a look to see if it's possible to add svelte-ignore comments to the script tag too (this honestly seems like a bug to me)

[paoloricciuti]

If we want talk about crazy things you can do we can also do this does this mean we should disable the warning because someone could write code like this? Definitely no.

[qupig]

@paoloricciuti I never said you should suppress or remove this whole warning, it can definitely be beneficial in many situations!

I literally showed you a case where the warning is effective.

No, that's not a valid case! That's a JS programming error, not a reactivity error with Svelte variables!

Changing it to a reactive variable won't solve anything! (it is still undefined)

And yeah you can do all sort of sheningan but...

That is NOT a shenanigan, that is one of the normal ways of writing logic, and again I stress, don't anticipate or enumerate all cases, do the right thing (This warning is for reactivity in the DOM, not component props).

You fail to see the real issue and keep nitpicking at the reproduction I provided and some of the wording, which drives me crazy. I don't think my use case is crazy or rare at all.

If you pass that down to a component you expect that component to receive an updated prop

Reactivity is about the DOM, not the variables in the JS, which in JS itself is live.

[paoloricciuti]

@paoloricciuti I never said you should suppress or remove this whole warning, it can definitely be beneficial in many situations!

I literally showed you a case where the warning is effective.

No, that's not a valid case! That's a JS programming error, not a reactivity error with Svelte variables!

Changing it to a reactive variable won't solve anything!

And yeah you can do all sort of sheningan but...

That is NOT a shenanigan, that is one of the normal ways of writing logic, and again I stress, don't anticipate or enumerate all cases, do the right thing (This warning is for reactivity in the DOM, not component props).

You fail to see the real issue and keep nitpicking at the reproduction I provided and some of the wording, which drives me crazy. I don't think my use case is crazy or rare at all.

If you pass that down to a component you expect that component to receive an updated prop

Reactivity is about the DOM, not the variables in the JS, which in JS itself is live.

I'm not nitpicking: i'm trying to understand the real use case to see if we can idntitfy a pattern that can disable the warning without causing false negatives.

Unfortunately just disabling the warning if you pass it to a component is not enough because you don't know what the component is doing with that prop ...so while in your case changing to state doesn't make a difference there are cases where it does.

My previous example was meant to show that if we really want to take every possible way people could write code we should just disable the warning all toghether.

Instead what we should do is identify a pattern where we know that the variable defined as let is really a const in disguise and mute the warning in that case. As i've show the euristich of "If it's being passed to a component and not used in the dom" is not a valid one because the component could use that in the dom or in an effect.

My suggestion is that if we determine that the variable is only updated in the script tag or in a block that execute syncronously in the script tag we can mute the warning. And the other suggestion is that we should have a way to mute warnings in the script tag so that if you know that your component will not use that in the dom you can mute the warning yourself.

[qupig]

because the component could use that in the dom or in an effect.

That's what you need to do, detect its usage in the sub components and if you find it being used for DOM or effect, please report a warn, I'd really appreciate you reporting it.

But don't report false warn/error until you've done it right. Detect real errors, even if you don't detect them (normal course of development), don't report false ones.

Any intelligence should start with what you can do and report the right things.

For example, you can first check whether it is in DOM tags and whether it is in Svelte Logic blocks, which can completely exclude component props.

Then, until you have the ability to detect if it's being used for reactive purposes in a child component, start warning it when using component props as well.

Here is an example that illustrates user confusion very well.

you can mute the warning yourself.

You reported a false warn/error but asked me to mute it manually, do you think this is the right thing to do?

[paoloricciuti]

That's what you need to do, detect its usage in the sub components and if you find it being used for DOM or effect, please report a warn, I'd really appreciate you reporting it.

You can't do that...cross module static analysis is just not feasible.

But don't report false warn/error until you've done it right. Detect real errors, even if you don't detect them (normal course of development), don't report false ones.

It's not an error, it's a warning and it's not blocking (you can disable the error in eslint if you want)...and with warnings a slight overfire is better than an underfire.

You reported a false warn/error but asked me to mute it manually, do you think this is the right thing to do?

Absolutely: if it's not detectably safe is better to overfire the warning and let the user interveene when they know their code is safe than underfire because you cannot tell the compiler with a comment "hey i know that this is not safe" otherwise there would be no need for the warn.

Here is an example that illustrates user confusion very well.

Why warn? Because it is used in the template 😄

[qupig]

You can't do that...cross module static analysis is just not feasible.

I don't know the svelte code and internal mechanisms, so I give feedback on issues and logic. If you can't do it, it means there are issues and flaws in the design.

Absolutely: if it's not detectably safe is better to overfire the warning and let the user...

I agree that in some security-related scenarios this should indeed be done. But this is NOT the one.

I'm not going to argue more with you about whether you should overfire or underfire. Either way you're already overcooked.

So the issue is a bug that does exist and you reported a false warn/error.

Either improve and fix it or ignore and close it, that's your choice.

Why warn? Because it is used in the template 😄

Well, you should at least mention that it has false alarms (e.g. when in component props) instead of asking people to look for bugs that don't exist.

[paoloricciuti]

Yeah let's stop it here. I'm trying to be constructive and find ways to help you with your use case without diminishing the experience of others but we don't seem to be able to reach an agreement.

Let's wait for some maintainers to show up and see what they think of.

[qupig]

(this honestly seems like a bug to me)

At the end, why do you think it's a bug? Do you mean it's a bug in svelte or a bug in my code logic?

I obtain a set of static data from an external async API in the main component and share it with sub-components for further use. Is this a design problem? What do you think is a better approach?

[paoloricciuti]

(this honestly seems like a bug to me)

At the end, why do you think it's a bug? Do you mean it's a bug in svelte or a bug in my code logic?

I obtain a set of static data from an external async API in the main component and share it with sub-components for further use. Is this a design problem? What do you think is a better approach?

The bug is that you can't ignore warnings in the script and imho that should be possible to do (and it's on the svelte side)

[qupig]

@paoloricciuti Ok, anyway, thank you for your participation and quick response. I admit I was a little offended when you first denied this was an issue, after all I spent time checking and reporting on it. But anyway, it's not your or anyone else's obligation, so I won't ask for anything.

[dummdidumm]

Assuming it doesn't take too much additional code, I'm ok with silencing the warning in the case of

<script>
  let foo;
  if (..) {
    foo = x
  }
</script>

{foo}

but if the variable is assigned anywhere within a function we should just warn - it's not worth it to try to do elaborate analysis for such a rare case.

[qupig]

@dummdidumm

I don't think that's a rare case unless there's a better way of sharing static data to child components.

Otherwise making it $state() is an unnecessary waste. Because it never really changes.

And most users who see the warn will do so directly, instead of ignoring the warn, that's why I raised it.

[paoloricciuti]

@dummdidumm

I don't think that's a rare case unless there's a better way of sharing static data to child components.

Otherwise making it $state() is an unnecessary waste. Because it never really changes.

And most users who see the warn will do so directly, instead of ignoring the warn, that's why I raised it.

Again the problem is that if the component is using that prop for something rather than static initialization of another state you NEED state. So in most cases the warning IS correct.

@dummdidumm what about being able to ignore warnings in the script...should that be considered a bug? I can work on it

[brunnerh]

The example given here can be written in a more idiomatic way that sidesteps the warning completely.

  <script>
    import Component2 from "./Component2.svelte"

-   let constVar;

    async function init() {
        try {
-           constVar = await Promise.resolve("some promise returned value");
+           return await Promise.resolve("some promise returned value");
        } catch (error) {
            console.error(error);
+           throw error; // if you need the catch block
        }
    }
  </script>

  {#await init()}
    loading...
- {:then}
+ {:then constVar}
    <Component2 {constVar} />
  {/await}

REPL

[dummdidumm]

@dummdidumm what about being able to ignore warnings in the script...should that be considered a bug? I can work on it

Yeah that's definitely a bug

[qupig]

@brunnerh I've certainly thought of this approach, but it's not feasible outside of minimal reproduction.

The init function could contain multiple try...catch blocks.

This is why using let instead of const is necessary, otherwise I would definitely use it.

[paoloricciuti]

@brunnerh I've certainly thought of this approach, but it's not feasible outside of minimal reproduction.

The init function could contain multiple try...catch blocks.

This is why using let instead of const is necessary, otherwise I would definitely use it.

You can either return in each of those try catch or use a local let inside the function and then return that let

[qupig]

return in each of those try catch

I don't understand you. And their results may be used outside the function.

Anyway, using this ugly const object is an efficient way to get around svelte's ugly bugs (for the sake of others who encounter the same warn):

REPL

[paoloricciuti]

return in each of those try catch

I don't understand you. And their results may be used outside the function.

Anyway, using this ugly const object is an efficient way to get around svelte's ugly bugs (for the sake of others who encounter the same warn):

REPL

Depending on what's your need the actual solution may change but what i meant is

async function init() {
    try {
        return Promise.reject("first try");
    } catch (error) {
        console.error(error);
    }

    // first failed so we want to return next

    try {
        return Promise.reject("second try");
    } catch (error) {
        console.error(error);
    }

    // second failed so we want to return third

    try {
        return Promise.resolve("third try");
    } catch (error) {
        console.error(error);
    }
}

or something like this

async function init() {
    let value;
    try {
        value = Promise.reject("first try");
    } catch (error) {
        console.error(error);
    }

    // first failed so we want to return next

    try {
        value = Promise.reject("second try");
    } catch (error) {
        console.error(error);
    }

    // second failed so we want to return third

    try {
        value = Promise.resolve("third try");
    } catch (error) {
        console.error(error);
    }
    return value;
}

but again it really depends on your use case.

[paoloricciuti]

Anyway, using this ugly const object is an efficient way to get around svelte's ugly bugs (for the sake of others who encounter the same warn):

Anyway i will work on silencing warns in the script so after that you can just mute the bug knowing your code will be safe. 😄

[qupig]

Just add a use case, even if a variable is used in the HTML template it does not necessarily have to be $state. Over-encouraging $state(...) will negate the benefits of fine-grained control brought by $state and make the code no longer svelte.

Svelte5-REPL

If there was a switch, I would most likely disable that specific warning globally.

[paoloricciuti]

Just add a use case, even if a variable is used in the HTML template it does not necessarily have to be $state. Over-encouraging $state(...) will negate the benefits of fine-grained control brought by $state and make the code no longer svelte.

Svelte5-REPL

If there was a switch, I would most likely disable that specific warning globally.

Or you can even remove one variable

REPL

[qupig]

@paoloricciuti Do you think I split it into two variables for no reason? Or do you think others are not as smart as you? If I need to do something with the last opened item when I open a new item, how do you do that with one variable?

Also, does this warning tell people "you can even remove one variable"? Or do you encourage people to simply add unnecessary $state?

[MotionlessTrain]

You actually got lucky in your repl that it worked without $state. When html gets rendered, the current value of the non-$state variable is used. In this particular case, the html gets rendered when opened becomes true (as it is in the {#if} block), and every time item gets edited, opened is set to true. This is very bug prone, and I can think of several ways how this could be slightly different, and be broken

[dummdidumm]

Came here to write this - totally agree. The warning is correct in all the cases I saw except the one where it's eagerly reassigned. All the others are prone to get buggy through changes to the code down the line and/or such an edge case that it doesn't warrant the additional code complexity to detect the false positive.

[paoloricciuti]

@paoloricciuti Do you think I split it into two variables for no reason? Or do you think others are not as smart as you? If I need to do something with the last opened item when I open a new item, how do you do that with one variable?

Also, does this warning tell people "you can even remove one variable"? Or do you encourage people to simply add unnecessary $state?

Store the variable currently in state in a separate variable before reassigning it? I don't know your use case, I go for what I see in the repl you share.

We can make absurd examples all days but the reality is that a bunch of people are telling you that you are either unnecessarily complicate stuff or write code where the warning would be a good thing and yet to try to come up with more convoluted ways in which the warning could maybe make no sense.

And at the end of the day it doesn't matter because it's a warning and if your code really works it will work even with the warning. And if the PR in the other issue is merged you can even silence it.

[dummdidumm]

All the examples in this thread are either prone to be buggy when someone refactors code later on, or can be written in a better way that circumvents the problem. Also that PR to silence this warning with an ignore comment was just merged, therefore closing this issue.

[Crenshinibon]

Hm. Just stumbled upon the same issue. In my case I need to bind a HTML-Element to a variable, so that I can check the dimensions of it after rendering it with dynamic and changing content (to answer the question: does the scrollHeight exceed the offsetHeight). This HTML-Element-Binding happens once and the binding never changes.

let infoBox:HTMLDivElement|undefined;

This raises the warning. Using $state() here seems wrong.

@qupig has a point here and the workarounds seem "unsvelty" ... not displaying the warning, would be fine with me.

[paoloricciuti]

workarounds seem "unsvelty" There's no workarounds: if you know it's fine just mute the warning, you have a tool to do it. not displaying the warning, would be fine with me Yeah but it will not be fine for the other cases where a warning is due.

Also what's your code?

<svelte:options runes />

<script>
    let el;
    console.log(el?.scrollHeight > el?.offsetHeight);
</script>

<div bind:this={el}>
    clicks
</div>

this is not giving me any warning so...are you sure state is not needed? :)

[Crenshinibon]

Hello @paoloricciuti I have just made a simple TestComponent and there the warning is not displayed. I'm not sure why I get this warning in my more complex actually Component. Maybe it's just the LSP that's confused or my dev setup is off.

[paoloricciuti]

Hello @paoloricciuti I have just made a simple TestComponent and there the warning is not displayed. I'm not sure why I get this warning in my more complex actually Component. Maybe it's just the LSP that's confused or my dev setup is off.

Try to send the code for your component...as I've said maybe you really need state and it's working only by chance.

[Crenshinibon]

It's here

[paoloricciuti]

It's here

It's a 404

[Crenshinibon]

Ah yes. Didn't realize it's a private repository. I have invited you, with read access.

[paoloricciuti]

You do need state because the div you are binding to is inside an if...so while you are only binding to one element that element can become null and then populated again.

Here's a minimal repro as you can see this works but also if you remove state you get the warning again but try to move the div outside the if...svelte will recognize that that can't change and mute the warning for you since there's no need for it anymore.

[Crenshinibon]

Thank you very much. This explains it very well.