kosich / rxjs-autorun

Re-evaluate an expression whenever Observable in it emits
MIT License
33 stars 2 forks source link

Completing with EMPTY observable #22

Closed kosich closed 3 years ago

kosich commented 3 years ago

this is a continuation of discussion in #19

When an observable in an expression completes without emitting any value — it often makes whole expression impossible to emit. Here are some examples:

// source streams
const a = timer(0, 1_000);
const b = EMPTY; // imagine b comes from an external API

// combinations:
computed(() => ({ a: $(a), b: $(b) }))
computed(() => [$(a), $(b)])
computed(() => $(a) || $(b))

Currently we swallow this issue. And it's not that easy to detect this completion if computed doesn't report it in any way! Users would probably need to plug in some combination of tap pushing to Subject that would complete computed via takeUntil.

Though when it comes to conditions, it's not that obvious:

computed(() => $(a) < 5 ? $(a) : $(b)); // GOOD: values > 5 are endless
computed(() => $(a) > 5 ? $(a) : $(b)); // BAD: values > 5 are meaningless

But we don't know which of the two cases we face. So to choose what behavior is more obvious — I'd vote for completion with EMPTY. It's very much expected in simpler cases and in conditional cases user would be able to easily handle it with defaultIfEmpty operator or concat(NEVER), depending on the need.

Completion with EMPTY also has a feature to it: users would be able to intentionally complete their streams:

computed(() => $(a) < 5 ? $(a) : _(EMPTY));

That's like a.pipe( takeWhile(x < 5) ) in an expression!

@Jopie64 as to the case you've mentioned with unsubscription and filtering: IMHO, it makes sense that if user wants to unsubscribe eagerly — they would mark it as weak, just as we intended:

const r = computed(() => {
   switch($(a)) {
      case 0: return $.weak(b); // weak b
      case 1: return $(c); // intentionally normal c
      default: return $(NEVER); // skip emission if not 0 or 1
   });

While default: return $(EMPTY) would mean that user wants to complete the stream.

LMKWYT 🙂

Jopie64 commented 3 years ago

The takeWhile example can be done without the new spec like this:

computed(() => _(a) < 5 ? $(a) : someFinalValue); // where someFinalValue could also be _(EMPTY)

Do you have another way to use this new spec as a feature?

My thinking is... if this spec is only there to help a user when he/she probably made a mistake, then this is less worth it if it blocks other users from using $(EMPTY) as a feature the way it is currently working (or could have been working when we would allow it to be used to pause the stream with unsubscribing normal bound observables). Sentence too long. Does it still make sense? :) I'd consider it less of a problem when a stream could be paused where it would unsubscribe unused streams, without explicitly binding it weakly.

But on the other hand, you might be correct that this semantic would help users significantly and maybe there will be other unforeseen use-cases on top of the takeWhile() like case. Also it explains nicely. NEVER for pause, EMPTY for stop.

These are my thoughts. So I guess I'd be ok when this is in the spec.

kosich commented 3 years ago

You're right, maybe a case with $(a) < 5 ? $(b) : $(EMPTY) ? But I'm just picking examples out of thin air to fit my point 🙂

And I agree that it can be a feature of a filter with completion unused normal subscriptions. But between the two feature options, I think the "NEVER for pause, EMPTY for stop" is simpler to understand, explain & reason about.

Yet, I'm most concerned about uncompletable expressions. Though it turns out that combineLatest stays open even if one of the two streams is EMPTY. Which confuses me 🙂

I'm not 100% sure at the moment.

I'll let this hang for a few days and if no more thoughts come up — I'll create a PR (also to see it in action). Still debatable & revertible!