Re-fix isObservable() and add test - fixes #29

[andywer]

Moved isListr and isObservable into a separate util.js and added a unit test for the util.js. Verifies that isObservable() works with both rxjs/Observable and zen-observable.

isObservable() checks for obj.constructor.name === 'Observable' now. Still somewhat hacky, but at least a plausible way to go as long as there's no official solution.

Btw: Maybe propose this to sindresorhus/is-observable#1?

[SamVerschueren]

Change the name to lib/utils.js

[SamVerschueren]

Use t.false and t.true.

[andywer]

Initially I did. But it failed, since right now isListr() will return null or the listr instance itself if it is. So then I just added a ... ? true : false to isListr(), but xo failed, because for some weird reason it complains that the ternary is superfluous...

Any suggestions?

[SamVerschueren]

Wrap it in Boolean(...)

[SamVerschueren]

Export the function right away without intermediate step.

[andywer]

Fixed the code 👍

[SamVerschueren]

Can you rename util to utils? Filenames and variables.

[SamVerschueren]

Sweet! Thanks for looking into this @andywer 🍻 !

[andywer]

@SamVerschueren No problem 🙌

[TylorS]

This actually breaks the stuff that I'm working on because of the changes to isObservable. The stream library I'm using is most.js and it's constructor.name is Stream and not Observable. However it does implement Observable interfaces.

[SamVerschueren]

@TylorS I'm not interested to actually support all the possible observable-like implementations out there. The reason I use duck-typing currently is because of an issue with is-observable which uses Symbol to detect if it is an Observable. If that issue gets fixed someday, I will start using that one and most.js will break again. So currently I'm trying my best to support RxJs and zen-observable. The only thing I can say about your issue is that maybe you can wrap it in a real Observable?

[andywer]

@TylorS @SamVerschueren I can understand both your standpoints. I had a quick look and is-observable would indeed work with most.js.

So how about that: We check with the current custom logic and fallback to is-observable or vice versa. If one of both is true we can be almost certain it is an Observable.

PS: I can also open an issue on zen-observable to use the symbol-observable so is-observable will work with the zen-observable as well.

[SamVerschueren]

Let's discuss this further in #31