proposal-signals / signal-polyfill

Implementation tracking the current state of https://github.com/tc39/proposal-signals
Apache License 2.0
185 stars 13 forks source link

[throwaway proposal] Add Effect node #22

Open ds300 opened 3 months ago

ds300 commented 3 months ago

A couple of things came up while working on the React bindings in https://github.com/proposal-signals/signal-utils/pull/72, and I believe they've come up in other contexts:

  1. We need a way to force tracked functions to execute even if the sources they track have not updated since the last invocation.
  2. We need a way to detect whether the sources of a tracked function have actually changed since the previous invocation. Simply knowing whether they might have changed is not enough to allow the fine-grained reactivity of signals to be seamlessly syncrhonized with external systems like React.

Rather than bog down the Computed class with special behaviours to allow it to be used for side effects, I think it would be cleaner, both in terms of API design and implementation/maintainability, to add a new subtle.Effect type.

This PR adds the Effect type with a constructor signature of new Effect<T>(run: () => T) and with two public methods:

And otherwise the effect instances behave similarly to computeds in that you pass them to a watcher to make them active, and they can be tracked by other effects.

Why is this proposal marked as 'throwaway'?

I'm simultaneously working on another (larger) API refactor that does away with the Watcher class which I believe is quite unwieldy, replacing it with more user-friendly primitives without sacrificing expressiveness. Not quite ready to share that idea yet.

justinfagnani commented 2 months ago

Can you talk more about these requirements?

Forcing tracked functions to run

We need a way to force tracked functions to execute even if the sources they track have not updated since the last invocation.

I see that the reason seems to be from https://github.com/proposal-signals/signal-utils/pull/72:

This is needed because you cannot memoize the result of a react render and simply return that result on future renders. The render may use hooks and React will throw errors if you don't invoke the same set of hooks on every render.

Wouldn't this imply either:

  1. That a React render just isn't a computed - It's an effect. You can build effects with computed and watchers.signal-utils has utilities for this.

or:

  1. React hooks should be modeled as signals as well so that they can contribute to the dirtiness of the graph. useState() is the most obvious candidate to be a state signal.

And if you do need to force tracked functions to run, you can do that already by running the function passed to the computed, just outside of the computed. This won't update the computed's cached value, obviously, but it sounds like that doesn't matter.

Detect whether a source has changed

We need a way to detect whether the sources of a tracked function have actually changed since the previous invocation. Simply knowing whether they might have changed is not enough to allow the fine-grained reactivity of signals to be seamlessly syncrhonized with external systems like React.

You can do this now by memoizing the value of a function. signal-utils has a reaction() utility as well that does this. There is also a proposal to have watchers include a list of changed signals that triggered the watcher: https://github.com/tc39/proposal-signals/issues/77

ds300 commented 2 months ago

a React render just isn't a computed - It's an effect.

exactly yes!

You can build effects with computed and watchers.signal-utils has utilities for this.

Sadly no, because as things stand effects must be implemented as Computed values. That's the problem. Computed values are cached and there's no way to reevaluate them on demand if none of their source values changed.

React hooks should be modeled as signals as well so that they can contribute to the dirtiness of the graph. useState() is the most obvious candidate to be a state signal.

Sure one day perhaps. In the meantime we should still be striving to design a reactivity system that can be integrated seamlessly with other state management systems.

And if you do need to force tracked functions to run, you can do that already by running the function passed to the computed, just outside of the computed. This won't update the computed's cached value, obviously, but it sounds like that doesn't matter.

Sadly no. In the case of React, a prop change or a useState update might indeed cause different signals to be dereferenced than on the previous render, so we must always execute the tracked function in a tracked context.

You can do this now by memoizing the value of a function. signal-utils has a reaction() utility as well that does this.

Sadly no. We can not evaluate the Computed to check whether it has returned a new value for a couple of reasons:

  1. As you correctly point out, react renders are effectful, we can't use their return value to make any meaningful decisions about whether they should run or not.

    function SetTitle() {
      const title = useTile()
      useEffect(() => {
        document.title = title
      }, [title])
      return null
    }
  2. hooks must run in the context of a react render cycle. We can't call computed.get to check whether to tell react to re-render, because we're only allowed to call computed.get during a re-render! It's a catch-22.

There is also a proposal to have watchers include a list of changed signals that triggered the watcher

This wouldn't help alas. We need to know whether the source values have actually changed, not whether they might have changed, which is all that proposal provides. Otherwise we'd end up triggering too many react renders.

Thanks for the feedback ❤️