subscribeToResult isObservable check

[milutinovici]

2nd if checks if the result is an observable, using instanceof. This check is too strict. It should be enough that result has subsrcribe method. Something like:

 if (result instanceof Observable || result.subscribe !== undefined)

[kwonoj]

Do you mean this checker?

[trxcllnt]

I know it's annoying, but you should use Symbol.observable to duck-type custom Observables cc: @Blesh @zenparsing

[milutinovici]

@kwonoj yes, that's the one.

@trxcllnt I haven't used Symbol before, but looking at the code, I should do something like

const custom = createCustomObservable();
custom[SymbolShim.observable] = () => custom;

But it's not working.

[benlesh]

We went with the instanceof check for perf reasons. In some of the resource constrained devices Netflix has to deal with, property checking is much slower.

I think it's probably a reasonable compromise to have an instanceof check first (since it will pass in most cases), then have a typeof result.subscribe === 'function' check somewhere after that. The only thing is, I think it will be much more common to return an Array or Promise in there before a custom "subscribable", or whatever. Perhaps that could be checked at the same time as custom observable (ala Symbol.observable).

Really though, Symbol.observable is what you want to do. It's got a guaranteed signature that should accept an Observer. Something that's "subscribable" in Rx terms, would have to accept not only three functions, but optionally an Observer.

[milutinovici]

I think it's probably a reasonable compromise to have an instanceof check first (since it will pass in most cases), then have a typeof result.subscribe === 'function' check somewhere after that.

I'm fine with that solution.

Really though, Symbol.observable is what you want to do.

I'm having a bit of trouble getting that to work. I'll dig a bit more to see why.

[milutinovici]

Well with beta 3, Symbol.observable just started to work. Although, I still think that object having subscribe function should be enough.

[benlesh]

closing as inactive.

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.