Make public classes open

[kl]

Short description of the issue:

Classes such as Variable have a public access modifier. Since Swift 3 this means that they cannot be subclassed from outside of the RxSwift module.

see: 0117-non-public-subclassable-by-default

Self contained code example that reproduces the issue:

public class MyVariable<Element> : Variable<Element> {
}

Xcode version:

8.0

Expected outcome:

I should be able to subclass for example Variable from my code.

What actually happens:

You get the error message: "Cannot inherit from non-open class 'Variable' outside of its defining module"

[kzaher]

Hi @kl ,

This is actually ok behavior. Why do you think you need to subclass Variable? It actually isn't meant to be subclassed.

[kl]

Hi!

In my project I have two frameworks, a Model framework and a View framework. The Model framework is responsible for making network request, parsing JSON and so on. Int the Model framework I have an object called State

public class State {

  public let gists: Variable<[GistEntity]?> = Variable(nil)

  public func reset() {
    gists.value = nil
  }

}

An instance of this object is references by the some classes in the Model and also by the View. The View subscribes to the variables in the State to update the UI. The Model in turn updates the State when new data is fetched.

This works but I don't want the View framework to be able to write the State, only register as a subscriber. Right now the View can mutate the State because the Variables have to be public, and then the View can just call state.variable.value = new_value. I only want the Model to be able to mutate the State.

So to solve this I thought I could subclass variable and overwrite the public value setter to just fatalError and then create a new internal setter property that is only available to the Model framework. Admittedly that's not a perfect solution but it would prevent the View from being able to mutate the State.

Actually now that I wrote this down I thought of another way to do it:

import Foundation
import RxSwift
import RxCocoa

public class State {

  let gists: Variable<[GistEntity]?> = Variable(nil)
  public var gistsAsDriver: Driver<[GistEntity]?> { return gists.asDriver() }
  public var gistsAsObservable: Observable<[GistEntity]?> { return gists.asObservable() }

  public func reset() {
    gists.value = nil
  }

}

That works but it's a bit noisy. Maybe there's a better way that I'm missing?

[krider2010]

This is an issue beyond that too. For example DelegateProxy is marked as open but only one function in there is open right now IIRC. Certainly most are still public. So fixing up a bunch of other pods using RxSwift is impossible right now as they override the other methods which are still public and not open.

There are a lot of libraries and extensions of code out there that subclass all sorts of things. Currently, they cannot ever be fixed without swizzling because of Overriding non-open instance method outside of its defining module compilation errors.

[kzaher]

Hi @kl ,

you don't need public var gistsAsObservable: Observable<[GistEntity]?> { return gists.asObservable() }

every Driver can be automatically converted to Observable.

gistsAsDriver.asObservable()

Hi @krider2010 ,

I didn't say we won't make any public classes open, just that I don't think it would be good idea to make Variable open.

Can you maybe provide us with some examples of problems so we can discuss them here?

[evgeny-sureev]

Hello. I got into same problem when tried to add reactive behavior to SpriteKit nodes. Reactive struct is defined public and this prevents to create extensions on it.

[krider2010]

Sure @kzaher

Once simple example (I kind of gave up migrating stuff after finding this issue) is that to make RxAppState compliant, I had issues converting https://github.com/krider2010/RxAppState/blob/master/Pod/Classes/RxApplicationDelegateProxy.swift for the reason mentioned above.

[kl]

@kzaher So if I do someVariable.asDriver().asObservable() is that semantically the same as calling someVariable.asObservable()? Or will the former always be observed on the main thread because it was converted to a driver in the middle?

[kzaher]

Hi guys,

ok, so ...

@evgeny-sureev unsure what you mean by problems with Reactive. We have extended it successfully in RxAlamofire with current code base.

@krider2010 we can make setForwardToDelegate, setForwardToDelegate, assignProxy, assignedProxyFor, delegateAssociatedObjectTag open.

@kl

the former always be observed on the main thread because it was converted to a driver in the middle`

yes

[evgeny-sureev]

Sorry, it was my mistake.

[pixeldock]

+1 on making setForwardToDelegate open.

[kzaher]

I've made all of those suggested methods on DelegateProxy open. If there are some other methods that need to be open, please open another issue.

Closing this one for now.