`sv check` should not warn about non-reactive code when runes are disabled

ptrxyz commented 1 month ago

Describe the bug

I am using sv check with a code base that uses Svelte 5, but non-runes mode. I disabled runes in my svelte.config.js using the compilerOptions: { runes : false } setting. However I still get the warnings "Warn: Properties of objects and arrays are not reactive unless in runes mode...."

Reproduction

Create a new project and disable the runes using the using the compilerOptions: { runes : false } setting in svelte.config.js.
Create a file with a $: statement.
run sv check

Logs

tmp/: sv check
/tmp/src/routes/(uishell)/(helpers)/tasks/+page.svelte:33:80
Warn: Properties of objects and arrays are not reactive unless in runes mode. Changes to this property will not cause the reactive statement to update (svelte)

    $: AC = new APIClient(Schematics, data.selectedUnit ? data.selectedUnit.key : ALL_UNIT.key)
    $: RS = new RecordStore(


### System Info

```shell
System:
    OS: Linux 6.11 Arch Linux (btw)
    CPU: (24) x64 AMD Ryzen 9 5900X 12-Core Processor
    Memory: 55.72 GB / 62.71 GB
    Container: Yes
    Shell: 5.9 - /usr/bin/zsh
  Binaries:
    Node: 23.1.0 - /usr/bin/node
    Yarn: 1.22.22 - /usr/bin/yarn
    npm: 10.9.0 - /usr/bin/npm
    pnpm: 9.12.2 - /usr/bin/pnpm
    bun: 1.1.30 - ~/bin/bun
  Browsers:
    Brave Browser: 130.1.71.118
    Chromium: 130.0.6723.69

Severity

annoyance

paoloricciuti commented 4 weeks ago

This is actually a warning for non runes mode because if you import something and access it and use it in a labeled statement that thing it's not reactive

repl

dummdidumm commented 4 weeks ago

I'm wondering how much use this warning is / how many false positives it yields.

paoloricciuti commented 4 weeks ago

I'm wondering how much use this warning is / how many false positives it yields.

tbf i don't think people are importing stuff expecting it to be reactive but overall i'm more inclined to mute a warning than have false negatives...i wonder if there's some github search we can do to check 🤔

benmccann commented 3 weeks ago

https://github.com/sveltejs/svelte/issues/13811 is sort of related

I think it's a little bit weird to add new warnings for non-runes code because this code was typically working in Svelte 4 and so now you have to go back and add ignores for code that you already know was working. It makes more sense in the context where you're writing new code, but it's a little less clear is valuable when it's on legacy code that you're going to migrate anyway and don't want to spend time cleaning up other than migrating it

dummdidumm commented 3 weeks ago

It sounds like the vast majority of people are either confused by this warning or annoyed and silence it. We should remove it

sveltejs / svelte