preactjs / signals

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

Bug?: useComputed doesn't update when given a new signal #366

Open jaskp opened 1 year ago

jaskp commented 1 year ago

I'm wondering if this is intended behavior or not.

Steps to preroduce:

So it seems that useComputed can only start tracking a new signal when the previous one changes. If this is intended, it should be very well documented that you need to pass the same instance of the signal to get proper behavior.

JoviDeCroock commented 1 year ago

Hey, this feels similar to https://github.com/preactjs/signals/issues/361#issuecomment-1552229236, we should def document that! Thank you for calling this out. Basically this

jaskp commented 1 year ago

@JoviDeCroock I suppose that if Preact's signals are to be widely-used, some conventions should emerge. Such as, if a library exposes a component that takes signals as props, whose responsibility is to wrap them in useLiveSignal()?

It would also probably be useful to add the currently buggy uses of signals to tests so it can be verified that the problem is solved.

jaskp commented 1 year ago

And it also gets me thinking, I haven't experimented as much with useSignalEffect() so let's put that aside for a while and assume signals work perfectly until you use useComputed(). Wouldn't it be more robust ifuseComputed() was changed so that it could react to changes of the signal itself? The signal reference can only change if the component renders, so on every render, the closure could be evaluated with some no-op flag to find out if the signals are the same as before and if not, it would create a new computed() or something like that.

(Just thinking out loud)

noopurphalak commented 9 months ago

@JoviDeCroock does this also work if we have a global state and we are using the useContext hook to get the signal?

XantreDev commented 9 months ago

main feature of useComputed is fact that it is not reexecuted on every rerender. The real way to check it - on babel plugin level, generate code which compares signals in scope with current ones

XantreDev commented 9 months ago

But I don't think it's a good idea. I think it's better to have convention that you should provide stable signal refference into child component. Or if you can't - change child component key

XantreDev commented 9 months ago

Btw, here is my hook that solve the issue

Here is fixed example: https://stackblitz.com/edit/vitejs-vite-xtukpt?file=src%2Fapp.jsx scrnli_12_28_2023_5-15-47 PM.webm

noopurphalak commented 9 months ago

@XantreGodlike Will this hook work in react?

XantreDev commented 9 months ago

@noopurphalak All library work both in react and preact. This hook too

noopurphalak commented 9 months ago

@XantreGodlike I am asking specifically for state derived from useContext

XantreDev commented 9 months ago

If you will change reference of signal in context provider - it will rerender bottom components, but signal in useComputed, useSignalEffect closures will not update

noopurphalak commented 9 months ago

@XantreGodlike Would you recommend defining signals and effects in a new JS file and exporting the signals to be used by multiple files instead of passing it to the Context API?

XantreDev commented 9 months ago

If you're using Context for dependency injection it will work just fine. If you will do like this.

const Ctx = createContext(null)
const Child = () => {
  const sig = useContext(Ctx)
  // will not update, will be 2
  return useComputed(() => sig.value)
}

const A = () => {
  const sig1 = useSignal(1)
  const sig2 = useSignal(2)
  const useFirst = useSignal(false)

  return (
    <Ctx.Provider value={useFirst.value ? sig1 : sig2}>
      <Child />
    </Ctx.Provider>
  )
}
noopurphalak commented 9 months ago

So how do I get the updated state from useContext for useComputed or useSignalEffect to work? Is there any way to subscribe to signal changes in those hooks?

XantreDev commented 9 months ago
import { useLinkedSignal } from '@preact-signals/utils/hooks'

const Child = () => {
  const sig = useLinkedSignal(useContext(Ctx))

  return useComputed(() => sig.value)
}
noopurphalak commented 9 months ago
import { useLinkedSignal } from '@preact-signals/utils/hooks'

const Child = () => {
  const sig = useLinkedSignal(useContext(Ctx))
  // will not update, will be 2
  return useComputed(() => sig.value)
}

I still don't understand the significance of useLinkedSignal if we can't use useComputed or useSignalEffect with its derived state. Maybe I am wrong, could you please clarify this for me. I will tell you an example of situation I am stuck in a REACT project:

I have a global state defined in authState.js as follows:

import { signal, effect, computed } from "@preact/signals-react";

export function createAuthState() {
  const authState = signal({
    userUUID: ""
  });

  const isLoggedIn = computed(() => !!authState.value.userUUID);

  return { authState, isLoggedIn };
}

Now I am passing the above state in the Context API as shown below:

<AuthContext.Provider value={createAuthState()}>
  <ConfigProvider theme={theme}>
    <App />
  </ConfigProvider>
</AuthContext.Provider>

In my Login.jsx I am updating my state like this:

state = useContext(AuthContext)

const updateState = (uuid) => {
  state.authState.value = {
    userUUID: uuid
  }
}

I also have a useSignalEffect in the Login.jsx which will redirect the user somewhere else if the user is already logged in like this:

useSignalEffect(() => {
  if (state.isLoggedIn.value) navigate("/dashboard")
})

This useSignalEffect does not seem to work when the state is updated in Login.jsx.

Please suggest a solution to this if possible.

XantreDev commented 9 months ago

Don't create state at every render

<AuthContext.Provider value={useMemo(() => createAuthState(), [])}>
  <ConfigProvider theme={theme}>
    <App />
  </ConfigProvider>
</AuthContext.Provider>
noopurphalak commented 9 months ago

I tried that, still doesn't work

allandcarv commented 9 months ago

I'm facing the same problem as @noopurphalak.

I have a signal and a computed signal globally shared via context, both created using useSignal and useComputed, but updating signals does not trigger the computed one

"@preact/signals-react": "^2.0.0",