Rxjs: subscribing should return a safer Subscription

Created on 24 Mar 2016  路  6Comments  路  Source: ReactiveX/rxjs

RxJS version: 5.0.0-beta.3

Code to reproduce:

const subscription = Observable.never().subscribe(::console.log);

subscription.next('haha');

Expected behavior:

It should throw an error because next doesn't exist.

Actual behavior:

Console logs "haha".

Additional information:

This is a byproduct of our Subscriber architecture.

Most helpful comment

This is no longer going to block release.

Since it's not documented anywhere that people can use the returned subscription as though it were a subscriber, it's not a necessarily a breaking change for anyone but people that are abusing that API (which at that point should be private).

When we change this at a later point (if we change it) it will be a minor version change perhaps, and we can add deprecation messages to the APIs we're removing.

All 6 comments

I'm labeling this as a "discussion" for now. Any changes here will likely come at the cost of a nasty refactor and precious, precious speed.

Even simply wrapping the Subscriber in a Subscription on it's way out breaks a dozen or more tests, because we have operators that are relying on the return value of subscribe as being the Subscriber.

Hmmm. This is tricky. I would vote for leaving it and not telling anyone. Those who use it will know they are doing something weird and abusing it.

I still would like to introduce changes, not immediately but gradually at some point. Behaviorwise it's anyway not expected, there's no breaking changes will be introduced. Problem is effort of refactoring, cost of performance, etcs as already pointed out.

This is no longer going to block release.

Since it's not documented anywhere that people can use the returned subscription as though it were a subscriber, it's not a necessarily a breaking change for anyone but people that are abusing that API (which at that point should be private).

When we change this at a later point (if we change it) it will be a minor version change perhaps, and we can add deprecation messages to the APIs we're removing.

This is still an issue. We're leaking implementation details with the Subscription. Leaving it open for now.

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.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

peterbakonyi05 picture peterbakonyi05  路  4Comments

benlesh picture benlesh  路  3Comments

haf picture haf  路  3Comments

LittleFox94 picture LittleFox94  路  3Comments

giovannicandido picture giovannicandido  路  4Comments