Skip to content

Alternative to both ref-counted and current proposal - force strategy selection at creation time #198

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Jamesernator opened this issue Feb 12, 2025 · 1 comment

Comments

@Jamesernator
Copy link
Contributor

Jamesernator commented Feb 12, 2025

So there is now a pr for ref-counted, however I'm not convinced this is the best route to take.

For one it limits the ability of observables to even have cold-like behaviour such as replays (or custom implementations of ref-counting).

Secondly even with ref-counting some behaviours in the other motivating issue remain confusing for notably:

Making accidental repeated API requests (ex. calling subscribe twice and forgetting a replay)

continues to be confusing with the ref-counted behaviour as ref-counting only prevents double requests if subscriptions happen concurrently, but if a previous request has already finished then a new subscription will cause a new request regardless even if the author intended replaying the request.

And even more disastrously, if a request subscribes in the immediate between the .next() and .complete() the request will be ignored all-together, i.e. this:

const req = new Observable(subscriber => {
    fetch("foo.txt").then((response) => {
        subscriber.next(response);
        subscriber.complete();
    });
});

reqObservable.subscribe({
    next: (res) => {
        console.log("Got response outer");
        // in some nested call, synchronously:
        req.subscribe({
            next: (res) => {
                console.log("Got response inner");
            },
            complete: () => {
                console.log("Complete inner");
            },
        });
    },
    complete: () => {
        console.log("Complete outer");
    },
});

prints:

Got respone outer
Complete outer
Complete inner

in other words, the inner next is never actually called because it subscribed late, so misses the value, pretty useless for network requests or other promise adoption.


As a solution to both the limitations/confusions/disasters of both current approaches I suggest forcing the decision of whether observables are hot or cold, and whether they are eager or replay, to construction time. This means producers of observables actually have to consider what behaviour they want the observable to have rather than being surprised later.

To do this I propose making the actual constructor private and instead expose the different strategies as static methods:

type SubscribeCallback<T> = (subscriber: Subscriber<T>) => void;

class Observable<T> {
    /* Behaves like the current proposal */
    static lazyUnshared<T>(subscribeCallback: SubscribeCallback<T>): Observable<T>;

    /* Behaves like the ref-counted PR */
    static lazyShared<T>(subscribeCallback: SubscribeCallback<T>): Observable<T>;

    /* Not currently proposed, but allows for re-using values, for example from requests */
    static lazyReplay<T>(subscribeCallback: SubscribeCallback<T>): Observable<T>;

    /* Not currently proposed, but helps address the feedback from issue #170 of
       > API requests at unpredictable and undesirable times (too late when the UI is rendering or too early during critical loading)
    */
    static eagerReplay<T>(subscribeCallback: SubscribeCallback<T>): Observable<T>;
}

these could be used like:

// Each subscription gets it's own timer
const unsharedInterval = Observable.lazyUnshared(subscriber => {
    const interval = setInterval(() => subscriber.next(), 1000);
    subscriber.signal.onabort = () => clearInterval(interval);
});

// Each subscription shares the same timer so will be aligned
const sharedInterval = Observable.lazyShared(subscriber => {
    const interval = setInterval(() => subscriber.next(), 1000);
    subscriber.signal.onabort = () => clearInterval(interval);
});

// The call isn't started until subscription, however once it is called the same values will be replayed
// on all future subscriptions
const networkRequest = Observable.lazyReplay(subscriber => {
     fetch("foo.txt").then(res => { subscriber.next(res); subscriber.complete(res); });
});

// Same the previous example, except the callback is invoked immediately so subscription
// might be able to receive the values earlier than needed
const eagerNetworkRequest = Observable.eagerReplay(subscriber => {
     fetch("foo.txt").then(res => { subscriber.next(res); subscriber.complete(res); });
});
@domfarolino
Copy link
Collaborator

This means producers of observables actually have to consider what behaviour they want the observable to have rather than being surprised later.

I'm assuming by "surprise" you mean the example you provided where you subscribe in between subscriber.next() and subscriber.complete(), and the new subscriber only receives complete and no values? I'm not sure that your proposal fixes this—if someone explicitly chooses the refcounted behavior, they can still subscribe in that fragile time and they have to handle that case.

We haven't heard that subscribing at that fragile time in a real footgun that we can expect to run into. And certainly haven't heard that it's more of a footgun than the always-cold version of this proposal which had side effects on every single subscribe() call.

I think additional configuration options could be added later if clear patterns develop that imply their need. But before we optimize prematurely and complicate the proposal, I think it makes sense to land the most predictable set of minimally-foot-gunny behavior we've agreed on in other discussions.

@domfarolino domfarolino closed this as not planned Won't fix, can't repro, duplicate, stale Feb 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants