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

Can't bind ObservableType to BehaviorRelay #1466

Closed
biboran opened this Issue Oct 20, 2017 · 6 comments

Comments

Projects
None yet
4 participants
@biboran
Contributor

biboran commented Oct 20, 2017

Short description of the issue:

It's not possible to bind ObservableType to BehaviorRelay as it was possible to bind ObservableType to Variable.

Expected outcome:

Since BehaviorRelay is considered a replacement for Variable, it should offer a hassle free migration. ObservableType should know how to bind itself to BehaviorRelay since that's the intention of bind(to:) method.

Self contained code example that reproduces the issue:

//This doesn't compile
Observable.just(false).bind(to: BehaviorRelay(value: true))
//This still compiles
Observable.just(false).bind(to: Variable(true))

RxSwift/RxCocoa/RxBlocking/RxTest version/commit

4.0

Platform/Environment

  • iOS
  • macOS
  • tvOS
  • watchOS
  • playgrounds

How easy is to reproduce? (chances of successful reproduce after running the self contained code)

  • easy, 100% repro
  • sometimes, 10%-100%
  • hard, 2% - 10%
  • extremely hard, %0 - 2%

Xcode version:

9.0.1
@kzaher

This comment has been minimized.

Show comment
Hide comment
@kzaher

kzaher Oct 21, 2017

Member

Ok, we can add that API.

Member

kzaher commented Oct 21, 2017

Ok, we can add that API.

@dakeshi

This comment has been minimized.

Show comment
Hide comment
@dakeshi

dakeshi Oct 23, 2017

Contributor

@kzaher Additionally, it needs to give a warning message. ex) Use BehaviorRelay. Variable is deprecated.

Contributor

dakeshi commented Oct 23, 2017

@kzaher Additionally, it needs to give a warning message. ex) Use BehaviorRelay. Variable is deprecated.

@kzaher

This comment has been minimized.

Show comment
Hide comment
@kzaher

kzaher Oct 29, 2017

Member

@dakeshi We've for now only put Variable in Deprecated.swift because it's meant to be deprecated in the following version.

We haven't marked it deprecated right now because I'm assuming people will have hard time migrating to 4.0 APIs, and unfortunately I'm assuming that there is a lot of Variable usage.

We wanted to help people for now by just informing them about the deprecation. It is possible we'll move Variable in RxCocoa in the following version and just do typealias Variable = BehaviorRelay.

That would enable newcomers to find relays more easily and we would have some consistency with behavior and publish relays.

We still haven't decided how exactly we'll deprecate Variable, so this is just preparation for that move.

When the exact deprecation path is decided, we'll potentially add deprecation warnings if that API will be removed completely or in case of typealias we'll just perform the move to RxCocoa without deprecation warnings.

Member

kzaher commented Oct 29, 2017

@dakeshi We've for now only put Variable in Deprecated.swift because it's meant to be deprecated in the following version.

We haven't marked it deprecated right now because I'm assuming people will have hard time migrating to 4.0 APIs, and unfortunately I'm assuming that there is a lot of Variable usage.

We wanted to help people for now by just informing them about the deprecation. It is possible we'll move Variable in RxCocoa in the following version and just do typealias Variable = BehaviorRelay.

That would enable newcomers to find relays more easily and we would have some consistency with behavior and publish relays.

We still haven't decided how exactly we'll deprecate Variable, so this is just preparation for that move.

When the exact deprecation path is decided, we'll potentially add deprecation warnings if that API will be removed completely or in case of typealias we'll just perform the move to RxCocoa without deprecation warnings.

@dakeshi

This comment has been minimized.

Show comment
Hide comment
@dakeshi

dakeshi Oct 30, 2017

Contributor

@kzaher Thanks for your detailed response.

What I feel strange is something which will be deprecated is placed in Deprecated.swift
Your intension isVariable will be deprecated but someone could understand Variable is deprecated.

And how do developers or new comers know Variable will be deprecated?
From Issue? Release page description? or Slack channel?
To make clear and smooth transition, I think a document should be updated to provide this info.
It is very helpful to add how to handle some deprecated methods or your plan to handle this(migration guide would be a good name) you mentioned in docs or README file.

Contributor

dakeshi commented Oct 30, 2017

@kzaher Thanks for your detailed response.

What I feel strange is something which will be deprecated is placed in Deprecated.swift
Your intension isVariable will be deprecated but someone could understand Variable is deprecated.

And how do developers or new comers know Variable will be deprecated?
From Issue? Release page description? or Slack channel?
To make clear and smooth transition, I think a document should be updated to provide this info.
It is very helpful to add how to handle some deprecated methods or your plan to handle this(migration guide would be a good name) you mentioned in docs or README file.

@kzaher

This comment has been minimized.

Show comment
Hide comment
@kzaher

kzaher Nov 4, 2017

Member

I can't think of a better way of informing people some API will be deprecated than adding it to a file named Deprecated.swift. People don't read docs a lot as far as I can tell.

When migration path is decided it will get it's availability attribute and become deprecated.

Member

kzaher commented Nov 4, 2017

I can't think of a better way of informing people some API will be deprecated than adding it to a file named Deprecated.swift. People don't read docs a lot as far as I can tell.

When migration path is decided it will get it's availability attribute and become deprecated.

@hmlongco

This comment has been minimized.

Show comment
Hide comment
@hmlongco

hmlongco Nov 27, 2017

If the current thinking is that Variable will be replaced by BehaviorRelay, will BehaviorRelay move to RxSwift from RxCocoa?

This would smooth migration as all of the current files that use Variable import RxSwift and not RxCocoa.

hmlongco commented Nov 27, 2017

If the current thinking is that Variable will be replaced by BehaviorRelay, will BehaviorRelay move to RxSwift from RxCocoa?

This would smooth migration as all of the current files that use Variable import RxSwift and not RxCocoa.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment