preactjs / signals

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

Proposal to add a signal version of the "useSyncExternalStore" hook #568

Open ndt080 opened 4 months ago

ndt080 commented 4 months ago

I suggest adding an alternative version of the useSyncExternalStore hook, which will return a signal and not cause the template to be rerender. This hook will simplify the integration of state storages with preact and allow developers to use stores without rerendering components when the value of the selector changes.

Here is an example of an implementation that I wrote for my own project based on the implementation of the useSyncExternalStore hook in preact

type SyncExternalStoreInstance<T> = { _value: T; _getSnapshot: () => T };

export function is(x: any, y: any) {
    return (x === y && (x !== 0 || 1 / x === 1 / y)) || (x !== x && y !== y);
}

function didSnapshotChange<T>(inst: SyncExternalStoreInstance<T>) {
    const latestGetSnapshot = inst._getSnapshot;
    const prevValue = inst._value;
    try {
        const nextValue = latestGetSnapshot();
        return !is(prevValue, nextValue);
    } catch (error) {
        return true;
    }
}

export function useSyncExternalStoreSignal<T>(
    subscribe: (flush: () => void) => () => void,
    getSnapshot: () => T,
): ReadonlySignal<T> {
    const value = getSnapshot();

    const cache = useSignal({
        instance: { _value: value, _getSnapshot: getSnapshot },
    });

    const onDetectChanges = useCallback(
        (value: T) => {
            const { instance } = cache.peek();
            instance._value = value;
            instance._getSnapshot = getSnapshot;

            if (didSnapshotChange(instance)) {
                cache.value = { instance };
            }
        },
        [cache, getSnapshot],
    );

    useSignalEffect(() => {
        cache.value;
        onDetectChanges(getSnapshot());
    });

    useLayoutEffect(() => {
        onDetectChanges(value);
    }, [subscribe, value, getSnapshot, onDetectChanges]);

    useEffect(() => {
        const { instance } = cache.peek();

        if (didSnapshotChange(instance)) {
            cache.value = { instance };
        }

        return subscribe(() => {
            if (didSnapshotChange(instance)) {
                cache.value = { instance };
            }
        });
    }, [cache, subscribe]);

    return useComputed(() => cache.value.instance._getSnapshot());
}
XantreDev commented 4 months ago

What is a use case? You can use useComputed for this purpose, isn't it?

rschristian commented 4 months ago

allow developers to use stores without rerendering components when the value of the selector changes.

What's the value of this? You can use any other state container (including a plain object) to achieve this.

ndt080 commented 4 months ago

Absolutely the same use cases as the useSyncExternalStore hook https://github.com/preactjs/preact/blob/main/compat/src/index.js#L170 BUT

When using state stores (for example, zustand, nanostores, etc.) via useSyncExternalStore, your component, where the value from the store is used, will be constantly updated, as the "useState" value is returned. I suggest adding an alternative version suitable for the concept of signals - useSyncExternalStoreSignal. It returns a signal and does not rerender the component.

The above code covers two cases of status updates: if the component is being updated and if the subscription is triggered. But we can shorten the code if triggering the subscription is enough as a state update trigger.

export function useSyncExternalStoreSignal<T>(
    subscribe: (flush: () => void) => () => void,
    getSnapshot: () => T,
): ReadonlySignal<T> {
         const cache = useSignal<T>(getSnapshot());

    useLayout(() => {
        return subscribe(() => {
                    cache.value = getSnapshot();
        });
    }, [cache, subscribe, getSnapshot]);

    return useComputed(() => cache.value.instance);
}
XantreDev commented 4 months ago

@rschristian I think it should be in place like recipes in docs, because use case is pretty niche. Should we create recipes section somewhere?

rschristian commented 4 months ago

Tbh I'm quite fine w/ leaving this as-is, I don't think we need to specifically document any of this ourselves. If someone else needs it in the future then we can link to this / move it to a discussion.


I don't think this is a good candidate for adding, as it's unlikely to be used by most. We generally have an ideology here of primitives over toolkits, and aren't looking to supply absolutely everything one can need. We focus on making sure the primitives are adapatable enough to allow structures to be build on top, as you've already done here.

For something to be added to the library, it needs to have a clear and wide value to the consumers at large, and I don't think uSESS fits the bill.

ndt080 commented 4 months ago

Based on your logic, by what principle did the useSyncExternalStore hook get into preact/compat? Just because React has this primitive? In my opinion, this primitive falls under all that you have described above. It is a building block for building higher-level structures in state libraries such as useStore, useStoreSelector, etc. I just gave a high-level implementation of the hook, but it can be implemented at a lower level to reduce the delay of translation from one signal to another and intermediate calls.

This hook fits perfectly into the concept of signals. And, as far as I thought earlier, the ideology of preact/signals is to get rid of unnecessary rerenders

rschristian commented 4 months ago

by what principle did the useSyncExternalStore hook get into preact/compat?

preact/compat is the (sub)package that mirrors React's API. That's why it's added and why it sits there, rather than in preact/hooks.

It is a building block for building higher-level structures

It can be, but it's not an essential one. It's perfectly fine for you to find this usable or important, but generally, for something to be added into the library it has to provide value to most users.

state libraries such as useStore, useStoreSelector, etc.

You shouldn't really need any of this though. Part of the value of signals is that you can subscribe to rather granular bits of data. These old React APIs and restrictions can usually get dropped. They don't tend to make much sense w/ signals.

the ideology of preact/signals is to get rid of unnecessary rerenders

In part, sure.

I'll leave this open, but I don't think it's a good candidate for addition. Too niche.