Support Observable

[BarryThePenguin]

Fixes #84

Still working through it. Happy to hear any feedback. I've just duplicated the tests for Promises. I started out writing tests using RxJs which all passed successfully. Switched to zen-observable half way through so some tests are currently skipped as a result.

[sindresorhus]

Might we worth extracting all the Observable related test into a separate test file, since there are so many now.

[BarryThePenguin]

Ok, and the Promise related tests too? I'm thinking the end result would be:

• test.js
• test.Promise.js
• test.Observable.js

[sindresorhus]

Yeah, but that should be done in a separate PR.

• test.js
• promise.js
• observable.js

[sindresorhus]

Generally looks good :)

Will need docs and tests are currently failing.

[BarryThePenguin]

Happy to write some documentation. I'll keep looking at the failing tests, I'm not sure what the problem is yet...

[BarryThePenguin]

I think 'es6-symbol' is causing the tests to fail. What is the preferred solution for Symbols in 0.10 and 0.12?

[BarryThePenguin]

Hmm... also zen-observable requires a Promise polyfill...

[jamestalmage]

We will have to run zen-observables through the Babel runtime plugin. As well as anything using Symbol.

[jamestalmage]

@BarryThePenguin #200 - will fix the Node 0.10 error - it exists on master (I don't know how the latest master build passed)

[BarryThePenguin]

Great, I'll squash and rebase again once that gets in

[BarryThePenguin]

I'm not sure if this is a Great Idea™

Any opinions?...

[jamestalmage]

Nah - just set it. That doesn't do anything to prevent pollution of globals.

[BarryThePenguin]

It's in test/test.js for Observables only as we want a lightweight implementation of the observable spec to test with. zen-observable "Requires ES6 Promises or a Promise polyfill" and this looked like a simple way to do that without also having a Symbol polyfil at the same time.

Maybe @Blesh or @zenparsing have a suggestion?

[benlesh]

I'm unsure of what the goal is here for "Supporting Observables". What do you need them to do? Do you need to support subscription to them? Do you need to support returning them?

If it's the former, you should be able to test for obj[Symbol.observable] and call it (like a function) to get back an observable object with a subscribe() function on it that follows the zen-observable spec. For that, there's really no need to import a library.

[benlesh]

Okay, I think I'm caught up with what the desire is here.

Observables can "end" in one of three ways:
They terminate because:

1. the producer errors.
2. the producer completes successfully.
3. the consumer unsubscribed.

You only really need to care about 1 and 2, because 3 would be caused by the test itself. To handle 1 and 2 for zen-observable, RxJS 5+, Kefir, Most and Bacon, you'd just need:

let obj = getSomethingThatMightBeObservable();

if (obj[Symbol.observable]) {
  obj[Symbol.observable]().subscribe({
    complete() { /* done successfully */ },
    error(err) { reportError(err); /* done because error */ }
  });
}

But the problem there is while you've determined that it's "done", you haven't analyzed the output.

If the idea is to analyze the output, the best short-term solution is to subscribe to RxJS 5 or zen-observable Observables with forEach, which returns a promise:

return myObservable.forEach(x => doSomeAssertion(x === 'whatever'));

There would be no mileage from Kefir, Most or Bacon from that unless the spec changes to include that forEach must be implemented on returns from [Symbol.observable](). cc/ @zenparsing

[benlesh]

There would be no mileage from Kefir, Most or Bacon from that unless the spec changes to include that forEach must be implemented on returns from Symbol.observable

... actually in that case it wouldn't matter anyhow, because the libraries that aren't RxJS 5 or zen-observable would need to be subscribed to in a different way.

I think that using forEach is probably your safest bet. It returns a promise, so you wouldn't have to make a change.

[sindresorhus]

I think that using forEach is probably your safest bet. It returns a promise, so you wouldn't have to make a change.

👍

[sindresorhus]

Off topic to this PR, but kinda weird that you have to call Observable.forEach(function () {}); to convert it to a promise. I would have preferred something like Promise.resolve(observable).

[benlesh]

I would have preferred something like Promise.resolve(observable).

Good luck with that one. It's not a bad idea, but I think it will be hard to get traction on changes to Promise to support Observable.

However, with RxJS, you can just use Observable.prototype.toPromise() which will convert the Observable to a Promise that resolves with the last value of the Observable when the Observable completes, or reject with the Observable's error if it errors.

[benlesh]

Also, I think it would be Promise.from(observable) ;)

[sindresorhus]

@BarryThePenguin Can you rebase from master and fix the merge conflict? This looks good to me.

@vdemedes @jamestalmage Anything final you'd like to point out?

[vadimdemedes]

Everything looks good to me, very straightforward, thanks @BarryThePenguin!

[BarryThePenguin]

👍

[BarryThePenguin]

Feel free to cancel build 168

That was a mistake

[sindresorhus]

Feel free to cancel build 168

Done.

[sindresorhus]

Landed! Thank you @BarryThePenguin :)

[BarryThePenguin]

🎉

[sindresorhus]

I've added some docs for it e57eff0, but I don't really know how Observables work, so any improvements welcome! For example, can you await an Observable in an async function?

// @thejameskyle

[zenparsing]

@sindresorhus You can't directly await an observable. We've discussed it at length and implicit conversion to promise for use by await is controversial. You can consume the sequence with forEach and await the returned promise, or we might add something like observable.toPromise() in the future.

[sindresorhus]

We've discussed it at length and implicit conversion to promise for use by await is controversial.

That's too bad... I was hoping await could be used directly by any async primitive. Do you happen to have a link to that discussion?

In that case, observable.toPromise() is definitely needed. The forEach-to-convert-to-promise thing is super weird.