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

Update static operators to accept promises across the board #1483

Closed
trxcllnt opened this issue Mar 17, 2016 · 9 comments
Closed

Update static operators to accept promises across the board #1483

trxcllnt opened this issue Mar 17, 2016 · 9 comments

Comments

@trxcllnt
Copy link
Member

A lot of this is my fault, but given that we've decided to interop with promises in most places, we should probably ensure we do it consistently. That means updating the family of static defer creation operators (defer, if, using, etc.) to accept promises, lowercase-o observables, etc. to use our internal subscribeToResult function.

@kwonoj
Copy link
Member

kwonoj commented Mar 17, 2016

Is this related to #1246?

@trxcllnt
Copy link
Member Author

@kwonoj yes it is

kwonoj added a commit to kwonoj/rxjs that referenced this issue Mar 18, 2016
kwonoj added a commit to kwonoj/rxjs that referenced this issue Mar 18, 2016
kwonoj added a commit to kwonoj/rxjs that referenced this issue Mar 18, 2016
kwonoj added a commit to kwonoj/rxjs that referenced this issue Mar 18, 2016
@benlesh
Copy link
Member

benlesh commented Mar 18, 2016

Really, there shouldn't be anything public facing that "accepts" Observable<T>, it should always accept SubscribableOrPromise<T>. Or at the very least Subscribable<T>. Having a function that accepts a strict class type is a 101 mistake, we should have been using interfaces this whole time.

@benlesh
Copy link
Member

benlesh commented Mar 18, 2016

We should probably make separate issues for the operators in question.

@staltz
Copy link
Member

staltz commented Mar 19, 2016

Can I remind people that if future PRs change the signature of an operator, please remember to update also the docs that I wrote for them. Otherwise docs will be a moving target

kwonoj added a commit to kwonoj/rxjs that referenced this issue Mar 19, 2016
kwonoj added a commit to kwonoj/rxjs that referenced this issue Mar 21, 2016
kwonoj added a commit to kwonoj/rxjs that referenced this issue Mar 22, 2016
kwonoj added a commit to kwonoj/rxjs that referenced this issue Mar 22, 2016
kwonoj added a commit to kwonoj/rxjs that referenced this issue Mar 23, 2016
kwonoj added a commit to kwonoj/rxjs that referenced this issue Mar 23, 2016
kwonoj added a commit to kwonoj/rxjs that referenced this issue Mar 25, 2016
kwonoj added a commit to kwonoj/rxjs that referenced this issue Mar 25, 2016
benlesh pushed a commit that referenced this issue Mar 29, 2016
benlesh pushed a commit that referenced this issue Mar 29, 2016
@kwonoj
Copy link
Member

kwonoj commented Apr 10, 2016

I think this one's covered mostly, can this be closed maybe? (Assume ConnectableObservable is not fall into this?)

@kwonoj
Copy link
Member

kwonoj commented Jun 21, 2016

I think most static operators are now covered as commented, /cc @Blesh for visibility before closing this out and switching to open issues for each individuals, in case I miss something should be covered before closing.

@kwonoj
Copy link
Member

kwonoj commented Jun 23, 2016

Closing this, if there's remaining operator will file separate.

@kwonoj kwonoj closed this as completed Jun 23, 2016
@lock
Copy link

lock bot commented Jun 7, 2018

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.

@lock lock bot locked as resolved and limited conversation to collaborators Jun 7, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants