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

Ambiguous signature of Observable.subscribe() #168

Closed
laughinghan opened this issue Sep 8, 2017 · 5 comments
Closed

Ambiguous signature of Observable.subscribe() #168

laughinghan opened this issue Sep 8, 2017 · 5 comments

Comments

@laughinghan
Copy link

laughinghan commented Sep 8, 2017

It's unclear to me from the current spec what the following does:

const foo = val => console.log('foo: ' + val);
foo.next = val => console.log('next: ' + val);
anObservable.subscribe(foo);

When the observable publishes an event, what gets logged, foo: or next: or both or neither?

Also, it would be nice if the proposal provided some justification for why both signatures are necessary. Why not, similar to promises, only have the subscribe(onNext : Function, onError? : Function, onComplete? : Function) : Subscription signature?

@benjamingr
Copy link

The spec is specific about this: foo would get called.

@appsforartists
Copy link
Contributor

I believe the section @benjamingr is referring to is subscribe(), which checks if its first argument is callable, and uses that to create an observer in the shape { next: arguments[0], error: arguments[1], complete: arguments[2] } if it is.

@psxcode
Copy link

psxcode commented Nov 23, 2017

Maybe it should be like in Promises
Promise checks if something is thenable
Maybe here we should check if something is nextable

@appsforartists
Copy link
Contributor

Can someone please propose a concrete use case for monkeypatching a next method onto a function?

It seems to be such an unlikely case that I don't care which one gets called. Preferring next might be a little simpler to reason about/test in code, but I feel like we're wasting time solving for something that has little practical value.

As we've discussed, the current proposal is not ambiguous about how subscribe tries its argument. Therefore, unless someone has a compelling reason to change the specced behavior, we should close this issue.

@laughinghan
Copy link
Author

It was my mistake, I thought the README was the spec. (Maybe this should be clear in the README too? Or is this too edge-case-y?)

I can't think of any concrete use case for why someone would monkeypatch a .next() method onto a function and desire that it be called. However, I do think there is practical value in being consistent with Promises.

I also do think there's practical value in being simpler by having only a single signature for one of this proposal's principle APIs.

Closing because my original question was answered.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants