preactjs / signals

Manage state with style in every framework
https://preactjs.com/blog/introducing-signals/
MIT License
3.72k stars 91 forks source link

`useSignal` should not use `useMemo` #307

Open reosablo opened 1 year ago

reosablo commented 1 year ago

The current implementation of useSignal uses useMemo.

https://github.com/preactjs/signals/blob/325499cf29d65c23a426751965ea241a27281472/packages/react/src/index.ts#L217-L219

React document says to write the code so that it still works without useMemo. Preact might be the same.

You may rely on useMemo as a performance optimization, not as a semantic guarantee. In the future, React may choose to “forget” some previously memoized values and recalculate them on next render, e.g. to free memory for offscreen components. Write your code so that it still works without useMemo — and then add it to optimize performance.

Without useMemo, useSignal will create a new signal instance every time when re-rendering occurs and lose the previous value.

Also, useComputed uses useMemo. There's no problem I think.

https://github.com/preactjs/signals/blob/325499cf29d65c23a426751965ea241a27281472/packages/react/src/index.ts#L221-L225

reosablo commented 1 year ago

The implementation could be like the bellow.

export function useSignal<T>(value: T) { 
    const $signal = useRef<Signal<T>>();
    return $signal.current ??= signal<T>(value);
} 
billybimbob commented 1 year ago

The implementation could be like the bellow.

export function useSignal<T>(value: T) { 
  const $signal = useRef<Signal<T>>();
  return $signal.current ??= signal<T>(value);
} 

At least for Preact hooks, utlizing the useRef hook vs the useMemo hook makes no difference, because useRef utilizes useMemo under the hood.

The useRef implementation may still be worthwhile for specifically React version of useSignal.

XantreDev commented 1 year ago

In reality useMemo computes once. btw you can use useState useState(() => signal(item))[0]

jamesarosen commented 1 year ago

Related: the current useMemo implementation also makes it awkward to create a Signal with input that varies based on React props or the results of other React hooks.

The best option seems to be to create a Signal with the initial value and then immediately set it again using useSignalEffect:

function useMyHook(someValue: SomeType) {
  // value set during signal creation:
  const signal = useSignal<SomeType>(someValue)

  // update value if `someValue` changes, since `useMemo` hides that change:
  signal.value = someValue

  return computed(() => {
    return doSomethingWith(signal.value)
  })
}

Regardless of whether useSignal uses useMemo, it should understand that its input may vary over time and should update the signal's value accordingly. The naive useMemo implementation would cause the destruction and recreation of the Signal itself, which defeats the purpose:

function useSignal<T>(value: T) {
  return useMemo(() => signal(value), [value])
}

A slightly more sophisticated implementation would bundle a useMemo and a useEffect or useSignalEffect together to achieve the right result.

Or, in other words, why wouldn't we want useSignal(someProp) to behave just like the proposed useWatcher?

Later

Is there perhaps a use-case for creating a Signal from a prop, then manually controlling when it updates? I can envision the code, but I'm not sure why I would want it:

function MyComponent({ foo }) {
  const fooSignal = useSignal(foo); // always "Foo1" unless we update it here

  useEffect(() => {
    if (someCondition(foo, bar)) {
      fooSignal.value = foo
    }
  }, [foo])

  return <div>foo: {foo}, fooSignal: {fooSignal.value}</div>
}

But why wouldn't you model that as a computed based on a fooSignal that follows foo?

Further edit

I retract this. A really simple use-case that requires a different useSignal and useWatcher is tracking the previous value so you can, e.g. unsubscribe to some stream based on the previous value and, simultaneously subscribe to a new stream based on the new value.

skybrian commented 7 months ago

I think there is no problem [^1], but since a search led me here and I just figured this out, maybe my explanation will help others:

This is all about lifetimes. The reason we have hooks is that parts of a component live longer than a render of that component. We want to re-render and get a view that's logically the same. [^2]

All we want for signals is that they have the same lifetime as the component. This can be achieved in different ways, which are all equivalent:

useState seems like the most idiomatic way to do this. A signal is a kind of state with more features and updated in a different way, so we don't want the second return value.

[^1]: If the implementation of useMemo ever changes in Preact, updating useSignal would be easy to do, so someone can do it then.

[^2]: This is similar to how reloading a web page should often show the same content, even though technically, the web page was recreated. (The URL is the same and the server-side persistent data is the same.)

zetaraku commented 3 months ago

In reality useMemo computes once. btw you can use useState useState(() => signal(item))[0]

I like this solution. It's logically correct and concise.