sveltejs / svelte

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

Reactivity with runes should work in old syntax #9287

Closed enyo closed 3 months ago

enyo commented 1 year ago

Describe the problem

It's great that mixing Svelte 4 + 5 syntax will be possible! But this is only helpful in large code bases if reactivity can be maintained in both cases.

It is possible to use stores in runes mode, but the reactivity fails the other way around:

See this REPL

Specifically this line:

$: z = counter.value + 2

(Where counter.value is a $state)

Describe the proposed solution

I don't think there is an actual reason for this not to work, and it would make incrementally changing to runes a lot easier.

Alternatives considered

The alternative is to rewrite all components in one go that need to import runes.

Importance

would make my life easier

enyo commented 1 year ago

The main use case for this, is that I have a few stores that I expose to many components with setContext.

The first thing I want to do, when switching to Svelte 5, is to change these stores to runes. But currently that means that I have to rewrite all components that use this context.

If it's possible to maintain reactivity in Svelte 4 syntax while using runes, then I can simply remove the $ sign from the invocations and everything should continue working as expected.

dummdidumm commented 8 months ago

We shouldn't change this, because that means subtly changing the rules around $: statements. Today it's "if this is state, have a dependency on it, else don't". If we loosen that to "everything can potentially be state" and track everything just in case, then other things could break. Giving this the documentation label because this is something we should note in the docs somewhere.

elron commented 3 months ago

I hope there will be auto-migration from svelte-4 to svelte-5 for this kind of stuff.

7nik commented 3 months ago

The MIGRATE button in the REPL already works nicely, and it will obviously be available as a command tool.

Rich-Harris commented 3 months ago

I'm not sure there's a good place in the documentation to put this (where 'good place' means 'a place that people will be able to search for it, or are likely to have read it and understood it before encountering this scenario').

I suspect it's better to just improve the compiler warnings around this stuff. We currently generate a warning in a case like this...

image

...though it's misleading — it says 'declared in a module script', because imports are hoisted (i.e. they're equivalent to declarations inside a <script context="module">. And it doesn't catch the OP's case, because counter is a local declaration.

Perhaps if instead of warning on 'module script' declarations, we warned if a $: statement doesn't depend on anything reactive, it would help diagnose these (somewhat rare, I suspect) cases. It would fail in the case where a statement had a mix and match of reactive and unexpectedly-non-reactive bindings, but we can't win 'em all.

Rich-Harris commented 3 months ago

Or better still, we could identify the non-reactive member expressions:

image

Working on a PR

elron commented 3 months ago

If we take it a step further, a button that converts the line automatically to runes would be really awesome.

Rich-Harris commented 3 months ago

As @7nik pointed out, that already exists.