Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reactivity with runes should work in old syntax #9287

Closed
enyo opened this issue Oct 2, 2023 · 8 comments · Fixed by #12824
Closed

Reactivity with runes should work in old syntax #9287

enyo opened this issue Oct 2, 2023 · 8 comments · Fixed by #12824
Milestone

Comments

@enyo
Copy link

enyo commented Oct 2, 2023

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
Copy link
Author

enyo commented Oct 2, 2023

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.

@dominikg dominikg added the runes label Oct 2, 2023
@dummdidumm
Copy link
Member

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
Copy link

elron commented Aug 12, 2024

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

@7nik
Copy link

7nik commented Aug 13, 2024

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

@Rich-Harris
Copy link
Member

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
Copy link
Member

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

image

Working on a PR

@elron
Copy link

elron commented Aug 13, 2024

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

@Rich-Harris
Copy link
Member

Rich-Harris commented Aug 13, 2024

As @7nik pointed out, that already exists.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants