Rxswift: 👷‍♂️👷‍♀️RxSwift 5 Pre-Release discussion

[freak4pc]

Hey @kzaher & others -

Since RxSwift 5 is around the corner, I took the liberty of making a list of PRs/Issues that were delayed for RxSwift 5.

The goal here is to see if:

• Any of them are easy enough to get into RxSwift 5; or,
• Any are hard to add but are needed for RxSwift 5 (and worth delaying the release for).

Since this is a major version bump we should take some time to think about any breaking changes we might want to introduce.

This is what I gathered so far:

• [x] doAfter* overloads to match RxJava. #1898 @freak4pc
• [x] Move Relays to their own framework. #1924 @freak4pc
• [x] Allow binding to multiple observers. #1702 @freak4pc
• [x] Using DispatchTimeInterval instead of TimeInterval for Schedulers. #1472 @kzaher
• [x] combineLatest of an empty array completes immediately. #1879
• [x] Change toArray to return Single. #1923 @freak4pc
• [x] Complete once any Completable completes. #1674~
• [x] Decide how to deal with soft-deprecated things (Variable, global RxTest things) @freak4pc #19223
• [x] Add compactMap #1925. @hmlongco
• [x] Fix take amount vs take interval ambiguity. @freak4pc (blocked by #1472)
• [x] Add resultsSelector missing closure labels for some overloads of combineLatest & zip.
• [x] Deprecate Completable.merge in favor of Completable.zip. #1929 #1931
• [x] Carthage Static Library Support. #1940

Generic Renames Checklist:

• [x] E and ElementType to Element, TraitType to Trait. #1945
• [x] R to Result #1948
• [x] C to Collection #1949
• [x] S to Sequence #1949
• [x] CompatibleType to ReactiveBase #1947
• [x] S to SharingStrategy #1951
• [x] S to Subject #1950
• [x] SubjectType.SubjectObserverType to SubjectType.Observer #1950
• [x] O to Observer #1952
• [x] O to Observable

If there are any other PRs/Issues you feel should participate in this discussion, please add a comment below.

[freak4pc]

@kzaher the purpose is mainly to see which of these we want to do for RxSwift 5, as long as we're making breaking changes here.

[valerianb]

Maybe Variable can be definitely removed from the repo ? It has been in Deprecated for a while now.

[freak4pc]

@valerianb Great point. I don't think we should entirely remove it, but we probably should move everything from runtime deprecations to hard deprecation (e.g. @available(*, deprecated)). Thoughts?

[valerianb]

Oh I thought there was a similar mechanism already in place with DeprecationWarner. But yes if that's not considered enough to push users to transition away from it then we should start with @available(*, deprecated) so it can be dropped in a future release.

[kzaher]

Hi @freak4pc, I think that these are small changes we can add.

doAfter* overloads to match RxJava. #1898
Move Relays to their own framework. #1501
Allow binding to multiple observers. #1702
combineLatest of an empty array completes immediately. #1879
Change toArray to return Single. #1543

If you can help with list on top, that would be great.

Decide how to deal with soft-deprecated things (Variable, global RxTest things)

I think we can deprecate Variable.

Using DispatchTimeInterval instead of TimeInterval for Scheduleers. #1472

I think we should also include this. This one is probably the biggest and requires intimate knowledge of the internals so I can take that one.

Complete once any Completable comples. #1674

I've responded to that thread, I think we can close this one. Not sure there is an action point on it. zip and merge are equivalent. We can add zip operator if you like, but we need to change the way zip operator works and not dispose subscriptions when no new element can be produced anymore.

[freak4pc]

I can take over the top list, no problem.

I’m also ok with closing the Completable one- we might be able to tackle this at a later release, I’d leave that to your discretion.

[hmlongco]

@freak4pc Would love to have the compactMap PR in 5.

[freak4pc]

It's in the list (look at the end) ;-)

[hmlongco]

Ummmm.... I could have sworn.... ;)

[kzaher]

@freak4pc I've refactored schedulers. I don't think there is any more confusion with take and count.

[freak4pc]

I mentioned on the other thread that I still think it's worth adding a label for one of them to differentiate. Types alone aren't enough in my personal opinion.

[DevAndArtist]

@kzaher @freak4pc I opened a PR to fix resultSelector mistakes.

https://github.com/ReactiveX/RxSwift/pull/1930

[kzaher]

I think that the last blocker for 5.0 is #1799.

[freak4pc]

We won't be able to release this for 5.0 because of the Xcode 10.2 Swift 5 issue I've mentioned. We should release without it.

Should we also leave the take() overloads as is? I still think it's worth adding a label to one of them at least.

[kzaher]

This is swift prefix api. Which is similar to our take.

https://developer.apple.com/documentation/swift/array/1689487-prefix

It doesn't have label for count and we don't have a label for time operators.

I think that the problem was worse before when you could specify a constant which could mean two different overloads.

Right now it should be more clear. I'm personally fine with the current state. Adding a label to count would be against Swift prefix API, and if we add label to time version we would need to add that label everywhere for consistency. I'm not even sure what label would we use.

[freak4pc]

Sounds good. I think this concludes the RxSwift 5 content we discussed unless you have any more thoughts.

[freak4pc]

@kzaher Actually prefix has multiple overloads and anything aside from the "count" one has a label ..

[kzaher]

Actually prefix has multiple overloads and anything aside from the "count" one has a label ..

@freak4pc I know, that's what I've posted that link.

Adding a label to count would be against Swift prefix API, and if we add label to time version we would need to add that label everywhere for consistency. I'm not even sure what label would we use.

[freak4pc]

Ah, I misunderstood. I was also referring to labeling the duration overload, not the count one.

I though of either take(duration:), or take(for duration:). WDYT?

[kzaher]

I think that take(for:) reads better, but for is a keyword and not really describing temporal component IMHO. Just Google take for and you'll get something like "take for a word" as a top hit.

You look at this as a decision for a single operator. I look at this as a decision to go through all temporal operators and decide new labels for each of them and deprecate the old ones also.

Without extremely convincing reason I would not want to do all of that.

[kzaher]

Does anyone know of any project that works with Carthage and static libraries?

[freak4pc]

That's a fair point @kzaher , but actually there is a precedence for using for as a label:

https://developer.apple.com/documentation/uikit/uibutton/1624022-title
https://developer.apple.com/documentation/foundation/datecomponents/2292889-value

And we even have a DelegateProxy(for proxyType:) initializer inside RxSwift ATM.

There are many more. There's no problem with using for: as a label even though it's a keyword.

It seems to me the Swift Guidelines are really about making signatures that read as sentences so take(for: .seconds(3)) sounds like "take for 3 seconds" which makes sense.

[danielt1263]

I think take(for:) (or any other conjunction) is only a good idea if the type of the argument is clear in context. In the case of duration, not only would the floating point value not be clear in context, but there are a couple of different types that could be passed. Better would be take(forDuration:) or take(duration:).

[freak4pc]

Don't forget it's now a DispatchTimeInterval, so I think the context is quite obvious (as opposed to the previous implementation that used TimeInterval).

In any way, I agree with Kruno that we shouldn't rush this, we can always do this later on. The disambiguation achieved with the schedulers refactor should be satisfactory for now.

[AlexisQapa]

@freak4pc @kzaher is https://github.com/ReactiveX/RxSwift/issues/1320 considered for rxswift 5 ?

[freak4pc]

No. It was too much of an ambitious goal that is more realistic for a future version of RxSwift.

[DevAndArtist]

@freak4pc @kzaher may I propose another breaking change for RxSwift 5?

• Can we rename the associated type E to Element in ObservableConvertibleType?
• Is there any reason why this not the case already?

The reason I'm asking for this change is to make this breaking change as early as possible to prepare for qualified type lookup on generic types. I think that we all want that Observable<T>.Element is the same as Observable<T>.E without the ugliness of E.

Here is an SE thread where I pitched about the missing type lookup for generic type parameters.

https://forums.swift.org/t/allow-member-lookup-to-find-generic-parameters/7768

[juancazalla]

Do you have an estimation on when will be RxSwift 5 released?

[kzaher]

@juancazalla I was planning to release it last Monday, but then we've decided to perform some renames. I hope we'll finish with the renames this week.

[freak4pc]

@kzaher The renames you're referringto are internal to my understanding (non-public API) - should we block release on that? I think it would take quite some work to rename every single-letter generic to something more informative.

[kzaher]

@freak4pc I was only talking about letters in the public API. The internal ones can wait.

There is a bunch of them in our public interface. Since we've renamed Element it seems like we should cleanup our public interface.

https://github.com/ReactiveX/RxSwift/blob/53cd723d40d05177e790c8c34c36cec7092a6106/RxSwift/ObservableConvertibleType.swift#L12

https://github.com/ReactiveX/RxSwift/blob/0df62b4d562f8620d4b795b18e4adf0b631527a1/RxSwift/Event.swift#L95

https://github.com/ReactiveX/RxSwift/blob/master/RxCocoa/Traits/SharedSequence/SharedSequence%2BOperators.swift#L20

https://github.com/ReactiveX/RxSwift/blob/master/RxCocoa/Traits/SharedSequence/SharedSequence%2BOperators.swift#L36

...

[freak4pc]

These are all on master - did you look in develop? I'll take another look anyways.

[freak4pc]

@kzaher I've done the R -> Result changes here:
https://github.com/ReactiveX/RxSwift/pull/1948

I'll do C next. O is a bit tricky because some are Observable and some are Observer but I'll give it a go as well.

[freak4pc]

@kzaher I've made a list of some generic renames worth doing (at the first comment)

Mostly done - I can't open PRs for the latter ones without the former ones as there would be too many conflicts, so I'll wait for you to review #1948 & #1949

[bobgodwinx]

@freak4pc + @kzaher regarding Result in 1948 Don't you think we should avoid this kind of naming? since Swift now has a Result Type I know it's generics but for readability we could just keep it R or even T. let me know what you think.

[freak4pc]

Fair point but I think since it’s a constraint and not a concrete type it’s totally fine. @bobgodwinx

[freak4pc]

@kzaher I think all Generic renames are basically done except for one, and I'm not sure how to proceed on it.

O to Observable - the name Observable is also a concrete type which would cause a bunch of confusion and force using RxSwift.Observable at some places. Not sure if it's the best idea.

On the other side just leaving it as "O" while we changed all other single-letter generics might be a bummer. I'm wondering if there's a better constraint to use here (Maybe Output: ObservableCOnvertibleType or Stream: ObservableConvertibleType) ?

Thoughts ?

[DevAndArtist]

What about AnObservable?

_{Sent with GitHawk}

[freak4pc]

Also a possibility - but it's already a long constant, so if we can avoid making it even longer, that would be better. I'm open to whatever's acceptable by Krunoslav here.

[DevAndArtist]

That‘s one thing I don‘t like about the style used by the library, but it‘s a totally different topic (I‘d prefer stdlib coding style, where I can prettify such long constraints in a readable way).

If you have concrete example, please link them here, I can take a closer look at them soon(-ish).

_{Sent with GitHawk}

[freak4pc]

Don't really understand what you're referring to with stdlib, feel free to share with us.

The concrete examples are everywher, just look for O: ObservableConvertibleType or O: ObservableType.

[DevAndArtist]

I‘m currently only on my iPad, I‘ll check later. But in general I would prefer a stdlib like coding style, that makes things easier to read (that is ofcourse my opinion).

func foo<AnObservable>(
  _ observable: AnObservable
) -> Observable<AnObservable.Element> 
  where
  AnObservable: ObservableType
{
  ...
}

That ofcouse won‘t be rendered by the ASTPrinter like that when the user cmd + click on the API in Xcode.

---

That aside AnObservable would be fine by me to not to create any ambiguity that will force us or in worst case the users to use RxSwift.Observable explicitly.

_{Sent with GitHawk}

[freak4pc]

Yup, it's definitely a personal opinion. I don't mind either style but I actually find the style where the protocol of the constraint is inline more readable. Personal preference for sure :)

[kzaher]

I've solved O.
Pushed changes to develop.

Is there anything else we want to do in the public interface?

[freak4pc]

I think that takes care of only the public API - for the rest I already did the other APIs as well just to be thorough. Do you want to leave that for now? Or should we go with Source everywhere?

[kzaher]

Maybe we should use Source to resolve ambiguities.

[freak4pc]

@kzaher Rest of the cleanups should be in #1958

[DevAndArtist]

I just realized one interesting fact. From SE the basic direction for generics UI goes into some Collection spelling for simple generic parameters. In that sense you could name the parameters SomeObservable which requires two more characters, but later on you could use them as some ObservableType if you do not need referencing the type directly to extract some other type information. But the choice is yours.

_{Sent with GitHawk}

[freak4pc]

If and when that SE would actually happen, we can have another discussion. Even though to me "Some" is superfluous.

[DevAndArtist]

Sure :) My point was that Some prefixed generic types or some P describe an opaque type, but the type is not known for the implementor of the method, as it‘s an input (parameter) type. Opaque type as the return type is the opposite because it‘s only known to the implementor but not the user, this is also known as reverse generics in SE. So if you need a simple type you can consider prefixing it with Some in case you‘d want it to become some MyType one day.

_{Sent with GitHawk}

[kzaher]

I think this should be everything for 5.0, or do we have something else?

[freak4pc]

I can’t think of anything else, personally.

[kzaher]

I'm publishing 5.0, and everything went ok until ....

http://cocoapods.org/pods/RxRelay

Maintained by Wassim Seifeddine.

God damn it.

[DevAndArtist]

@kzaher the repository is dead for 2 years and it's basically doing exactly the same we want to do.

@wassimseif can we find an agreement and reclaim the cocoapods spec namespace for RxRelay?

[freak4pc]

Getting the privileges from @wassimseif would be optimal. Otherwise we can reach to the CocoaPods team and see if they'll be willing to help (I've seen several scenarios they were able to)

[kzaher]

I've had to delete 5.0.0 of RxSwift because of this :(

[kzaher]

I've sent an email to @wassimseif. I guess one thing is sure, this won't be released today probably :( Damn it.

[freak4pc]

I also e-mailed him haha :)

[DevAndArtist]

Any everyone pinged him here at least three times. 🤞

[freak4pc]

haha :) true.

[kzaher]

Well, we had some bad experiences with name camping. Somebody has https://github.com/RxSwift organization and didn't want to transfer it a while ago :) That's why we moved the project to ReactiveX :)

[DevAndArtist]

The good thing about the repository is that it's dead and it's doing what we're doing here now. :)

[freak4pc]

Actually think being part of ReactiveX is much nicer and more confident-inducing anyways :]

[freak4pc]

Pod name ownership was transferred successfully, we're good to go @kzaher ! 🎉

[kzaher]

@freak4pc I'll do it tonight, I need to change pod description because cocoapods were also complaining about that.

[freak4pc]

Sounds good. Once that's done you might want to do pod trunk delete RxRelay 0.1.3 to get rid of the old project.

[kzaher]

@freak4pc

This is done.

Sounds good. Once that's done you might want to do pod trunk delete RxRelay 0.1.3 to get rid of the old project.

We should never delete any tags.