Skip to content

Svelte 5: the $sync rune #11380

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

Closed
fcrozatier opened this issue Apr 29, 2024 · 17 comments
Closed

Svelte 5: the $sync rune #11380

fcrozatier opened this issue Apr 29, 2024 · 17 comments

Comments

@fcrozatier
Copy link
Contributor

fcrozatier commented Apr 29, 2024

Describe the problem

Feature request: a native Svelte way to keep two $states in sync.

Motivation

This naturally arises in contexts where a value can be updated in multiple ways, eg. a $bindable prop synced with a class field.

The logic is: if A is dirty, update B, if B is dirty update A.

At the moment to do this properly we would have to make a custom wrapper around $stateful values to track which one is dirty, when the signals already have this information

Example

And without a wrapper the design has several gottchas REPL:

  • either the class knows too much (is coupled with the component props)
  • or the $effect fires too many times
  • plus we reinstantiate the whole class each time for a single prop change
<script>
	let {checked = $bindable(false)} = $props()

	class Checker {
		checked = $state(false);

		constructor(initial){
			this.checked = initial
		}

		onclick = () => {
			const newState = !this.checked;
			this.checked = newState;
			//checked = newState; // Option 1: but class must know about props
		}
	}

	// Seems like an anti-pattern to reinstantiate the whole thing for a prop change
	const checker = $derived(new Checker(checked)); 

	$effect(()=>{
             console.log("child state: ", checker.checked);
	    // checked = checker.checked // Option 2: but updates too many times because of $derived
	})
	
</script>

<input checked={checker.checked} onclick={checker.onclick} type="checkbox">

Describe the proposed solution

The $sync rune

$sync(A,B) does what it feels like: when A is dirty it updates B, when B is dirty it updates A.

This way:

  • there is no need for a custom wrapper
  • the intent of the code (sync data) is clear, not spread between $derived+$effect or $effect+$effect blocks
  • no $effect over-firing
  • no need to reinstatiate with $derived on every single prop change
  • no coupling of the class and the component props

Corollary:

  • better designs
  • better performance
  • better code readability

With this rune the example can be written:

<script>
	let {checked = $bindable(false)} = $props()
        
        // win 1 : class only cares about itself (no coupling)
	class Checker {
		checked = $state(false);

		constructor(initial){
			this.checked = initial
		}

		onclick = () => {
			const newState = !this.checked;
			this.checked = newState;
		}
	}

	// win 2: no reinstantiation on every prop change
	const checker = new Checker(checked); 
        
        // win 3: highly perfomant + no unneeded execution / overfiring
        $sync(checked, checker.checked);
</script>

<input checked={checker.checked} onclick={checker.onclick} type="checkbox">

Importance

would make my life easier

@Thiagolino8
Copy link

Thiagolino8 commented Apr 29, 2024

If you want to abstract the modification logic it is not necessary to synchronize two states, just use some kind of getter and setter

@7nik
Copy link
Contributor

7nik commented Apr 29, 2024

Having two competitive sources of truth isn't a good design to begin with. But it can be unavoidable when using libs.
$sync won't be highly performant - it still is two $effects, no other way to do it:

function $sync(a, b) {
  $effect(() => { a = b; });
  $effect(() => { b = a; });
}

REPL.

@fcrozatier
Copy link
Contributor Author

fcrozatier commented Apr 30, 2024

@Thiagolino8 The REPL doesn't work as intended parent and child are not synced. When you click the checkbox it errors (see the console).

@7nik well if $sync is part of Svelte it can avoid overfiring since the signals know which piece of state is dirty, and then perform surgical updates. And your example is overfiring when you click the checkbox REPL

The "Checker" example while simple contains the concepts and gotchas of a more general situation, which will be common in the Signals era:

A few related elements, whose state+behavior are driven by a class encapsulating the logic (which can also come from a library). Now the class is instanciated from the component props, a few of which must be kept in sync since they are $bindable. (Reasonnable situation right?)

The conceptual problem here can be formulated as reactivity crossing the boundary of the class.

There are several ways to go about this but we currently always have to pick among:

  • coupling the class with the props,
  • reinstantiating on every prop change because of $derived
  • $effect overfiring

And on a stylistic point the synchronization intent is spread on several disconnected blocks.
Also the $effect + $effect technique doesn't feel very idiomatic, in addition to being verbose and over-firing:

$effect(()=>{a = b});
$effect(()=>{b = a});

Now with $sync the problem is viewed as a synchronization thing, somehow dual to the boundary crossing:

  • There is no need to reinstantiate the whole class everytime, as the class is initialized from the props, and then the sync happens with the accessors in $sync(prop, instance.attribute)
  • There is no overfiring as $sync performs surgical updates as it knows what's dirty
  • The code is clearer, more idiomatic and doesn't use $effect

@7nik
Copy link
Contributor

7nik commented Apr 30, 2024

@Thiagolino8 The REPL doesn't work as intended parent and child are not synced. When you click the checkbox it errors (see the console).

Typical this error. Fixed REPL. There is nothing to sync because the class uses the passed state instead of creating own one.

it can avoid overfiring since the signals know which piece of state is dirty, and then perform surgical updates.

I strongly feel it will shoot in your leg when you try to sync nested props ($sync(a.b.c.d, e.f.g.h)) with special logic in setters/getters. Example.

@fcrozatier
Copy link
Contributor Author

fcrozatier commented Apr 30, 2024

The fixed REPL is interesting as it works and doesn't have the mentionned shortcomings.

But one could argue that:

  • the code in the class is slightly more opaque (it adds one layer)
  • the class is designed around the fact that it's going to be used in a certain way. (Otherwise we wouldn't do state.value for this but just value). Maybe it's not a problem and we'll declare this the conventionnal way to do things. It could be fine for code you own. But it could also be a code smell
  • For code you don't own you're back to square 1

For the logic $sync would run the accessors so it basically would be the same (but more idiomatic and readable) than the $effect+$effect pattern.

Also notice that the $sync rune would allow your code to be future proof in the sense that you do not have to design your class around expecting 'accessor value', and one day ref values could be a thing, which I think is part of the bet Svelte 5 makes in its design. (correct me if I'm wrong @trueadm)

@dm-de
Copy link

dm-de commented May 1, 2024

I'm not sure, why this is needed.

Here is an example, of multiple states:

LINK

@Rich-Harris
Copy link
Member

I'm probably missing something but what's wrong with this?

@7nik
Copy link
Contributor

7nik commented May 2, 2024

It's supposed the class is third-party or a complex/shared logic you moved out to its own file.
However, it also raises the question of the best API shape of this logic. It's probably a bunch of stateless utils, but using them may require writing semi-repeating code in the component.

@GauBen
Copy link
Contributor

GauBen commented May 2, 2024

Hey there, I've been thinking (and tinkering) a lot about two-way bindings of things that require a transformation:

There is no obvious way to do this without event listeners, and having a mean to $sync states together would be nice.

edit

I managed to create a cross function that does roughly what I want here:

const cross = ([a, toB], [b, toA]) => $effect.root(() => {
	let skipA = true
	let skipB = true
	$effect(() => {
		a()
		if (skipA) skipA = false
	 	else {
			toB()
			skipB = true
		}
	})
	$effect(() => {
		b()
		if (skipB) skipB = false
		else {
			toA()
			skipA = true
		}
	})
})

Here are the previous examples with this cross function doing the sync:

edit 2

Here is the generalized cross sync functions!

const cross = (...pairs) => {
	let skips = pairs.map(_ => true)
	for (const [i, [dependency, action]] of pairs.entries()) {
		$effect(() => {
			dependency()
			if (skips[i]) skips[i] = false
		  else {
				action()
				skips = skips.map((skip, j) => i === j ? skip : true)
			}
		})
	}
}

Example usage: 3 $states kept in sync

@fcrozatier
Copy link
Contributor Author

I'm probably missing something but what's wrong with this?

Yes something is missing as the parent state is not updating when just clicking on the checkbox :)
They're not in sync here

@Rich-Harris
Copy link
Member

Oh, I assumed that was explicitly what you were trying to prevent, so that the child state could temporarily diverge from the parent state. Otherwise why not just use a binding?

@purepani
Copy link

purepani commented May 6, 2024

The idea seems to be $bindable functionality but for class properties(and variables in general) instead of just component props.

@fcrozatier
Copy link
Contributor Author

fcrozatier commented May 7, 2024

Yes in the case of a simple checkbox we would indeed just use a binding (this is where the example is too simple).

But as we move towards using classes for more complex and refactorable internal state+behavior management then the situation is more complex than the props <-> element direct two-way binding. In general it looks like props <-> internal state <-> element. And this is where a $sync could help to keep things in sync.

So the challenge is to make the "Checker" work but keep the internal state in the class to have a taste of something more generalizable to complex situations.

Here is a less contrived example from Discord (thanks @ottomated) where an element has an internal state and the value is bound.

But the ideas to go about this always revolve around using multiple $effects, or $derived + $effect, or passing around some kind of wrapped value. None of which are really satisfying.

@Azarattum
Copy link
Contributor

Azarattum commented May 9, 2024

I want to bring out that $sync probably isn't the great name for it. The first thing it associates with is synchronous (as in synchronous code), not synchronize (as in synchronize variables). I think something like $bind would be more appropriate here. But since we already have $bindable why not extend it instead of making an entirely new rune?

For example:

// inline
let {checked = $bindable(false).sync(checker.checked)} = $props()
// or
let {checked = $bindable(false)} = $props()
$bindable(checked).sync(checker.checked)

Where the argument to bindable is required to be either a bindable rune or its default value during init.

Also maybe $bindable().attach()?

@Azarattum
Copy link
Contributor

Azarattum commented May 9, 2024

Btw, are there any use cases where $sync would be used outside of the $bindable prop? In that case my suggestion above isn't great

@purepani
Copy link

purepani commented May 20, 2024

I think yes, since you could use it for any case where there's just a class with properties you want to sync to. You instantiate the class, and then sync some other variables all within a single component.

@Rich-Harris
Copy link
Member

Closing this as we've investigated it at length and repeatedly concluded that passing thunks and callbacks around is the right approach

@Rich-Harris Rich-Harris closed this as not planned Won't fix, can't repro, duplicate, stale Aug 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants