sveltejs / svelte

Cybernetically enhanced web apps
https://svelte.dev
MIT License
78.21k stars 4.09k forks source link

Svelte 5: (non-state-reference) `moduleVariable` is updated, but is not declared with $state(...)... #11269

Closed qupig closed 4 months ago

qupig commented 4 months ago

Describe the bug

moduleVariable is updated, but is not declared with $state(...). Changing its value will not correctly trigger updates.(non-state-reference)eslint(svelte/valid-compile)

moduleVariable is updated, but is not declared with $state(...). Changing its value will not correctly trigger updates.svelte(non-state-reference)

Refer to the reproduction below.

It's worth noting that this is reported as error rather than warn in eslint-svelte.

I don't know why it just shows up as warn in the source and REPL.

In other words, it shows up as a red wavy line in local VSCode.

Reproduction

Svelte5-REPL

Logs

No response

System Info

"svelte": "5.0.0-next.110"

Severity

blocking an upgrade

dummdidumm commented 4 months ago

What is the desired behavior here? Is it about this being flagged as an error instead of a warning? If eslint-svelte flags this as an error, that sounds like something within your eslint config which treats warnings as errors (possibly to be more strict). Or is this about not warning about this in the first place? If yes I'd like to see a non-minimal reproduction because the way it's written is certainly not how it appears in your code, to better understand if there's a false positive we could silence.

qupig commented 4 months ago

First of all, I'm not sure why this would be flagged as an error/warning, since it's just passed to the component as a property and there are no reactive updates that depend on it.

You can see that if you move the variable to a non-module script, the error disappears: Svelte5-REPL

Secondly, I don't know why it's flagged as a different level of error in eslint-plugin-svelte and svelte, I don't have any custom rules.

because the way it's written is certainly not how it appears in your code

I'm not sure what you mean, but I think minimal reproduction is enough to illustrate the problem, I'm not sure what you're looking for.

If you don't think the issues exist there, feel free to close the issue.

dummdidumm commented 4 months ago

I can't help you with the eslint case (it may be that the default is to error on these warnings). As for the warning itself, this is a non-runes file in which this error shouldn't appear.

I'm not sure what you mean, but I think minimal reproduction is enough to illustrate the problem

What I mean is: Your reproduction is too minimal. Surely you didn't write code like this in your real app, so I'd like to see a more realistic, less minimal example to understand whether this is a false positive or not.

qupig commented 4 months ago

Surely you didn't write code like this in your real app, so I'd like to see a more realistic, less minimal example

Sorry, I still don't understand what you're saying, this is a minimal reproduction of the code from my REAL app. I don't have time to make up code that doesn't exist to flood(?!) the repo.

In fact it is a very complex object, so adding any non-relevant code will cause a huge increase in complexity. I don't want to throw the whole app at you, and I can't.

I also don't want to make an extra complicated reproduction for this. I've spent a lot of time investigating and making this minimal reproduction. It's effective and straightforward.

I'm guessing you're looking to see if it's being used as a reactive variable elsewhere, but you shouldn't, in this reproduction it's not, so it shouldn't be a warning, right?

Also yes, it's not a rune file, it's code come from Svelte 4.

I can't help you with the eslint case

Maybe it should be forwarded to the eslint-plugin-svelte project? So this isn't your part? You don't have a transfer mechanism?

If you can't reproduce it locally, I could try replicating the project environment, but are you saying you're not interested in investigating that at all?

dummdidumm commented 4 months ago

Ok it's a non runes files so #11434 will fix this