Change Promise returning methods to return Observable (except forEach)

[benlesh]

Promises bearing events have problems

Because promises force scheduling, any API that returns a promise runs the risk of confusing users because:

1. It causes subscription, where other methods do not.
2. The promise fires asynchronously, meaning it's too late to e.preventDefault().

Basically, we should have everything accept forEach return an Observable.

Semantics of forEach

forEach would still synchronously execute its callback, but the completion being a promise would mean completion is scheduled. This is okay, because completion doesn't bear a value.

const et = new EventTarget();

const testOver = et.on('test').take(1).forEach(e => {
  // This is fine.
  e.preventDefault();
  console.log('handled!');
})
.then(() => {
  console.log('forEach resolved as done');
});

console.log('before dispatch');
et.dispatchEvent(new Event('test'));
console.log('after dispatch');

// Logs
// "before dispatch"
// "handled!"
// "after dispatch"
// "forEach resolved as done"

[domenic]

I strongly disagree with this direction, as previously discussed.

[domfarolino]

It causes subscription, where other methods do not.

If we want to really stick with the "Observable is a dual of Iterator" analogy, then I think we should keep the promise-returning methods as they are. [1, 2, 3, 4].reduce((a, b) => { return a + b; }) doesn't return an iterator that, when exhausted, reduces to a final sum value. Instead, it eagerly "uses up" an iterator over the Iterable that it's called on, and immediately yields the final value. I think that'd be pretty clear with Observables too (hence the one-shot Promise return value).

The promise fires asynchronously, meaning it's too late to e.preventDefault().

As discussed a bit at TPAC, I don't think the concerns about the promise-returning methods interacting poorly with events is very serious. Consider every() and some() for example, which take a predicate callback whose return value feeds into an ultimate, single boolean.

1. The point of the callback is to determine if an event passes some predicate test, not filter events or pass them along, so I don't think people will confuse these callbacks as the ideal opportunity to call preventDefault(). Although if they did, that's still fine since the callbacks run synchronously with respect to events that are emitted!
2. The final value you get is a boolean, so you can't even expect to call preventDefault() on what the promise resolves to, so there's no footgun here.

There seems to be no reason to make those helpers return Observables, and making them return promises more-closely matches what Iterator helpers do.

Now regarding the other methods: toArray() and find(), I think toArray() very much makes sense as a promise-returning method since it's returning a one-shot collection of all of the values up until complete(). The alternative is to return an observable whose next() is called a single time once the input observable finally completes(), and next() would be given an array of all values seen up until that complete()... That's not too bad, but I still think the one-shotness of that operator translates to the promise world very intuitively[^1]. And while find() is expected to return one of the values emitted by the promise (and thus suffer "Concerns"), I still think (1) above applies which makes it less likely it'd be a footgun, and making it consistent with the others seems good.

[^1]: The downside here is not so much with event handling / prevent default, but rather the more general fact that you're forcing yourself into async Promise scheduling. If, for example, you call toArray() on an Observable that emits all of its values and completes synchronously, there's just no way to get the final array synchronously without re-implementing a synchronous version of this on your own basically.

[domfarolino]

In short, promise-returning methods reduce to a single value, which seem less suited to be wrapped in an Observable, with all of the compositional operators. For example, what would be the purpose of calling .filter() .take(), or .flatMap() on the result of someObservable.find(predicateHere)? Since find() conceptually gives you the first single value that satisfies your predicate, the usual operators don't seem to be useful on that value in the same way as they would a stream of many values.

[esprehn]

I think Microtask should not be too late to e.preventDefault() for platform dispatched events. The microtask checkpoint is upon exiting the event handler back into native code before the defaultPrevented value is read. It's only "too late" on a custom EventTarget where you dispatchEvent() because the microtask checkpoint is no longer between every event handler.

That's always been a challenge with custom EventTargets having different microtask behavior which should probably be solved in a different way (ex. an explicit checkpoint API, or having a way to queue a task to dispatch the event like native would target.queueDispatch(e) => Promise<boolean>.

[benlesh]

More questions on the Promise-returning methods:

1. For find(), if the value is not found, should the returned promise reject? RxJS would reject in this case, because if you have source.find(value => value === undefined) the result becomes ambiguous. And if so, what does the rejection look like?
2. When they're aborted, do we reject with the same error that fetch does? A DOMException named AbortError?

[domfarolino]

For find(), if the value is not found, should the returned promise reject?

I think no, given https://www.w3.org/2001/tag/doc/promises-guide#reasons-should-be-errors, https://www.w3.org/2001/tag/doc/promises-guide#rejections-should-be-exceptional, and https://w3ctag.github.io/design-principles/#error-types.

When they're aborted, do we reject with the same error that fetch does? A DOMException named AbortError?

I think that's standard practice for the rejection value, yes. If there is some other principle that demands it to be something else, I am not aware of it.

[domfarolino]

I think there's not much more to be done here, and I think we are satisfied with the previous comments in this issue describing the rationale behind the promise-returning methods, so I'll go ahead and close this. If someone thinks this is immediately actionable, please let me know!