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

fix(WebSocketSubject): WebSocketSubject will now chain operators prop… #1752

Merged
merged 1 commit into from
Jun 8, 2016
Merged

fix(WebSocketSubject): WebSocketSubject will now chain operators prop… #1752

merged 1 commit into from
Jun 8, 2016

Conversation

benlesh
Copy link
Member

@benlesh benlesh commented Jun 7, 2016

Removed the custom lift method from WebSocketSubject. This is the quick and dirty fix for #1745. I think this class needs to be refactored again so it no longer overwrites it's own destination.

@rgbkrk
Copy link
Contributor

rgbkrk commented Jun 8, 2016

Awesome, thanks.

@benlesh benlesh merged commit bf54db4 into ReactiveX:master Jun 8, 2016
@trxcllnt
Copy link
Member

@Blesh isn't the real culprit here the incomplete WebSocketSubject constructor? It doesn't set this.source if the first parameter is an Observable, and the optional second parameter destination is ignored entirely.

@benlesh
Copy link
Member Author

benlesh commented Jun 17, 2016

@trxcllnt so you're saying here, we could add a check to see if destination is passed and if so, use that, otherwise use the ReplaySubject?

@trxcllnt
Copy link
Member

@Blesh yes I think that should work. and probably shouldn't do the assign() if the first argument is an Observable.

@lock
Copy link

lock bot commented Jun 6, 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 6, 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

Successfully merging this pull request may close these issues.

3 participants