useSignalEffect should execute after changes have been applied to the DOM

[luisherranz]

The callback passed to useSignalEffect should be executed after the changes (if any) have been applied to the DOM. Otherwise, it won't be a direct replacement for useEffect.

For example, people expect this to work, even if ref appears when open switches from false to true, because useEffect waits until changes have been applied to the DOM:

useEffect(() => {
  if (open) ref.current.focus();
}, [open]);

But with the current implementation of useSignalEffect, the callback runs before the DOM changes, and therefore ref.current may not exist yet:

useSignalEffect(() => {
  if (open.value) ref.current.focus();
});

Full example in StackBlitz.

Other reactive libraries introduce a tick function (Vue, Svelte, Alpine), but I'd prefer if useSignalEffect could mimic useEffect because the familiarity and ergonomics are better. I.e., access to signals after the tick() call are not tracked.

[marvinhagemeister]

That's a very good point. I hadn't thought about the interplay with ref's.

[JoviDeCroock]

Hmm, that makes sense. I guess the sync escape hatch is possible but leveraging the commit options hook would allow us to immitate the timing of a regular useEffect

[luisherranz]

If you give me some instructions, I can work on the PR.

[souporserious]

Ran into this as well, I ended up creating a reactive ref that seems to do the trick and essentially would work like a tick mechanism:

const node = useSignal<HTMLElement | null>(null)
const ref = useCallback((refNode) => (node.value = refNode), [])

useSignalEffect(() => {
  if (open.value) node.value?.focus()
})

If this could somehow work like useEffect out of the box though that'd be awesome!

[luisherranz]

@JoviDeCroock, @marvinhagemeister: any idea how to implement this? I can work on it.

[JoviDeCroock]

I would say that it would work like the following, we introduce a second variable an object containing mode/timing as a key with three values

• sync (default): current behaviour
• layout: behaves like useLayoutEffect - you can probably get that from preact/hooks/src/index.ts but tldr, array that gets processed after RaF
• commit: behaves like useEffect gets pushed on _component._commitCallbacks and processes as part of options._commit, can also look at preact/hooks

Would this be sufficient to work on it? Imho we could omit layout as an option for the time being until folks find a use for it.

[luisherranz]

Thanks @JoviDeCroock.

commit: behaves like useEffect gets pushed on _component._commitCallbacks and processes as part of options._commit, can also look at preact/hooks

I can't find _commitCallbacks in the code. Do you mean _renderCallbacks?

Would this be sufficient to work on it?

Hmmm... I guess the problem is how to delay the execution of the effect callback once it has run for the first time. We can't do it with normal async execution because if we do so, the second time the computation won't pick the dependencies. For example, imagine a setTimeout of 1 second was the right timing:

function useSignalEffect(cb: () => void | (() => void)) {
  const callback = useRef(cb);
  const firstTime = useRef(true);
  callback.current = cb;

  useEffect(() => {
    return effect(() => {
      if (firstTime.current) {
        // First time
        firstTime.current = false;
        callback.current();
      } else {
        // Subsequent times.
        setTimeout(() => callback.current(), 1000);
        // It doesn't work because the callback will run once the computation
        // (`effect`) has finished.
        }
    });
  }, []);
}

So, is there an API to delay the execution of effect to the right moment? Or does this need a change in @preact/signals-core?

we introduce a second variable an object containing mode/timing as a key with three values

About that: so, right now this is the code of useSignalEffect:

function useSignalEffect(cb: () => void | (() => void)) {
  const callback = useRef(cb);
  callback.current = cb;

  useEffect(() => {
    return effect(() => callback.current());
  }, []);
}

And it would accept a second argument, like this:

function useSignalEffect(
  cb: () => void | (() => void),
  mode: "sync" | "layout" | "commit" = "sync"
) {
  // ...
}

Then, the implementation for each mode would be:

• "sync"

  You say it should be the current behavior, but the current behavior acts as "commit" in the first run and as "sync" later. Should we change it as well for the first run? I guess something like this:

  function useSignalEffect(
    cb: () => void | (() => void),
    mode: "sync" | "layout" | "commit" = "sync"
  ) {
    if (mode === "sync") {
      const dispose = effect(cb);
      useEffect(() => dispose, []);
    }
  }

• "layout" and "commit"

  Again, any direction here would be greatly appreciated as I'm not familiar with the signal's internals yet 🙂

  function useSignalEffect(
    cb: () => void | (() => void),
    mode: "sync" | "layout" | "commit" = "sync"
  ) {
    if (mode === "commit") {
      const callback = useRef(cb);
      callback.current = cb;
      useEffect(() => {
        return effect(() => {
          // The first time it needs to run syncronously.
          callback.current();
          // After that, it needs to wait until `useEffect`/`useLayoutEffect` runs.
          ??
        });
      }, []);
    }
  }

[inakiabt]

Is this still an issue?

[luisherranz]

We have our own implementation of useSignalEffect in WordPress that has been working well so far.