Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Observable constructor doesn't require any parameter #3153

Closed
martinsik opened this issue Dec 3, 2017 · 4 comments
Closed

Observable constructor doesn't require any parameter #3153

martinsik opened this issue Dec 3, 2017 · 4 comments
Labels
TS Issues and PRs related purely to TypeScript issues

Comments

@martinsik
Copy link
Contributor

RxJS version: 5.5.3

Code to reproduce:

import { Observable } from 'rxjs/Observable';

const o = new Observable();
// const o = Observable.create();

o.subscribe();

Expected behavior:

Throw compilation error or run fine and do nothing.

Actual behavior:

Compiles fine but throws error when run.

Additional information:

I've seen this question on StackOverflow at least three times where people misunderstood the difference between Subject and Observable classes. They created an instance of Observable and then didn't know what to do next.

What's confusing is that you can create an Observable with just new Observable() (or Observable.create()) because the subscribe callback is optional but when they try to subscribe it throws an error:

/Users/.../node_modules/rxjs/Observable.js:165
                throw sink.syncErrorValue;
                ^

TypeError: Cannot read property 'subscribe' of undefined
    at Observable._subscribe (/Users/.../node_modules/rxjs/Observable.js:231:27)
    at Observable._trySubscribe (/Users/.../node_modules/rxjs/Observable.js:172:25)
    at Observable.subscribe (/Users/.../node_modules/rxjs/Observable.js:160:65)
    at Object.<anonymous> (/Users/.../general/subscribe_01.js:6:3)
    at Module._compile (module.js:573:30)
    at Object.Module._extensions..js (module.js:584:10)
    at Module.load (module.js:507:32)
    at tryModuleLoad (module.js:470:12)
    at Function.Module._load (module.js:462:3)
    at Function.Module.runMain (module.js:609:10)

The error is thrown because this line by default checks if this.source exists:

sink.add(this.source ? this._subscribe(sink) : this._trySubscribe(sink));

but then it tries to use this.source anyway:

return this.source.subscribe(subscriber);

So maybe the constructor should require either the subscribe callback or a source to be always set. For example like this:

constructor(subscribeOrSource: (this: Observable<T>, subscriber: Subscriber<T>) => TeardownLogic | Observable<any>) {
@kwonoj
Copy link
Member

kwonoj commented Dec 4, 2017

If I recall correctly, this was due to signature incompatibility with Subejct as same as signature of static create (which accepts general Function only, cannot have strong types). May need to revisit if that's still cases, but since Subject doesn't require param on construct not sure unless we bend over some types.

@kwonoj kwonoj added the TS Issues and PRs related purely to TypeScript issues label Dec 4, 2017
@kwonoj
Copy link
Member

kwonoj commented Dec 4, 2017

I still have to check once again and see if that's true though.

@martinsik
Copy link
Contributor Author

So at the end this might be related to this: #2004

@martinsik
Copy link
Contributor Author

martinsik commented Dec 28, 2018

I checked it again today with [email protected] and it doesn't throw any error and does nothing in both cases which is fine I think so I'm going to close this.

https://stackblitz.com/edit/rxjs-7diu1c?file=index.ts

@lock lock bot locked as resolved and limited conversation to collaborators Jan 27, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
TS Issues and PRs related purely to TypeScript issues
Projects
None yet
Development

No branches or pull requests

2 participants