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

Signal class lacks implementation of setting new value using previous value #92

Open
titoBouzout opened this issue Apr 1, 2024 · 3 comments

Comments

@titoBouzout
Copy link

titoBouzout commented Apr 1, 2024

Say you have the following:

signal.set(signal.get() +1)

This is reading and writing to the signal at the same time Very problematic in tracked scopes as it will loop forever.

This is solved in Solid via the following form, which uses the previous value of the signal, giving it to the callback without causing tracking on the current scope.

signal.set(prev => prev +1)

While this form works well, it has three problems:

  1. The setter of the signal has to do a typeof to see if something is a Function to call it with the previous value.
  2. Whenever you want to save to a signal a function, you have to wrap it, else, it will run the function you want to save.
signal.set(()=> fn)

It could get even more ugly if you are creating the body of the function to save at the same time, it will look like

signal.set(() => () => a.exec(b).groups)
  1. Whenever you are saving something that you dont know if it is a function (say the values of an array that user/lib gave you), you have to check for function first
signal.set(typeof value === 'function' ? () => value : value)

So I propose a third method, to support this functionality, to avoid the two/three problems mentioned above.

@NullVoxPopuli
Copy link
Collaborator

I think, explained in another Issue (I forget where), but you can achieve a modified value via Signal.Computed, and modification to a signal should happen as the result of an event (user interaction, event, effect, etc).

Example:

const doubled = Signal.Computed(() => signal.get() * 2);

and for the event / user interaction use case, you're not in a tracking frame, so there is no risk of infinite render anywhere:

const userClicked = () => {
  signal.set(signal.get() + 1)
}

<button onClick={userClicked}>Increment</button>

Additionally, I'm testing a thin wrapper around the polyfill here: https://github.com/NullVoxPopuli/signal-utils/pull/1/files#diff-a97765e1344fe9e023876e2b4c82e072ef8899c89d0f79038876a8809cbb7374R14 which is using native JS's ++ operator, e.g.:

theSignal++

(which is, in spirit, the same as what you describe (set + get)).

it's still very WIP -- I hope to have something usable for folks in a couple days, but in my tests I need to have some effect implementation to pull on the reactive layer for testing / protecting for/against accidental infinite loops.

@milomg
Copy link
Collaborator

milomg commented Apr 1, 2024

On signal.get() + 1:
If you read a signal from outside of a reactive function, it is untracked by default, and as a general rule of thumb, you should avoid writing to signals from effects/computeds. If you absolutely need to do that, you either have a more advanced use case, or you should think about how to restructure your code.

However, if you must have this behaviour (as a framework author), you can use composition, and write a wrapper signal that includes this method.

As for looping forever:
I think it's important to remember that this proposal intentionally does not specify createEffect, and the computeds we are discussing here are closer to memoized functions than Solid's createComputed. When a signal changes, the downstream computeds will not run at all until someone asks for their value. It is up to the framework scheduler to determine whether effects will loop for eternity or not.

Adding on to the explanation above:
I generally like to consider a redux like model around signals, where you have events which modify the signals, then values propagate through the graph. As a side effect, a framework may schedule effects that modify the dom etc (or schedule a new event to pass through on the next cycle).

Screenshot 2023-11-20 at 11 27 48 AM

@titoBouzout
Copy link
Author

Thanks Milo,

So in the case presented, it won't loop on declaration(lazy evaluation), nor even when the signal updates(lazy update), only when it's called to retrieve the value.

In a possible effect implementation, which is most likely the primary use case for a primitive like this, will loop.

const c = computed(()=> signal.write(signal.read())
effect(()=>{ c() })

It's worrying to see reads and writes on the same sentence.

From an educational point of view, read and writes shouldn't happen that way, and an api to do read+write at the same time without causing tracking could help people understand reactivity better, as they will ask/think why one has to do prev => prev + 1 instead of seeing their program/browser freeze.

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

No branches or pull requests

4 participants