useObservable forceRender race condition

[pkieltyka]

I've encountered a very obscure race condition when using multiple useObservable hooks across a variety of components.

The gist of it is when two different React components are observing multiple observables in the same store, a change in one observable will trigger a forceRender, which will cause the useEffect() effect function to be called, and when it returns it will call the unsubscribe method of the onChange method, and prevents the change notification to be delivered (as its no longer listening in between the change), https://github.com/BeTomorrow/micro-observables/blob/master/src/hooks.ts#L8. There is a timing issue where the onChange unsubscribers will be called by the effect's return function, removing the listener of an observable that in-turn will also be updated a set call from the store.

The result is one component updates, and the other does not as it never receives the onChange listener call and never calls its own forceRender. This happens especially with multiple shared observables across multiple components. I've experienced this in my own application that uses stores declaratively (like a spreadsheet).

One solution which works is to defer the unsubscribe calls:

export function useObservable<T>(observable: Observable<T>): T {
    const [, forceRender] = useState({});

    useEffect(() => {
        const unsubscribe = observable.onChange(() => forceRender({}));
        return () => {
            setTimeout(() => unsubscribe(), 150);
        }
    }, [observable]);

    return observable.get();
}

Using a timeout is certainly not ideal here but it does solve the issue. Its possible there is a circular dependency between observables and components, but this case should be considered. Please lmk if you have other ideas.

[pkieltyka]

if a post-effect unsubscribe was deferred, then it's probably best to unsubscribe after a change:

useEffect(() => {
        const unsubscribe = observable.onChange(() => {
            unsubscribe()
            forceRender({})
        });
        return () => {
            setTimeout(() => unsubscribe(), 150);
        }
    }, [observable]);

also, to elaborate on above, I was using a computed observable created via transform(), and calling useObservable() on the value returned from the transform.

[simontreny]

Hi Peter ! Thanks for raising this issue.

I think a better solution is to copy the listeners before iterating on them. This way, if a listener is removed while iterating, it won't skip the next listener. Actually, when reading your message, I was sure it was already the case in the current version of the library, but it seems I reverted it for some reasons.

An even better solution would be to implement a copy-on-write mechanism, i.e. copy the listeners only if a listener is unsubscribed while iterating. This is more optimized as most of the time the copy is not needed.

I will implement the first solution quickly, I'll let you know when it's available.

[simontreny]

This should be fixed in latest version. Let me know if you still meet the issue.