Consider de-emphasizing `ViewRegistry`.

[rjrjr]

@davidapgar just caught me up on a conversation that's been happening on the Kotlin side: why not get rid of ViewRegistry?

In both the Swift and Kotlin implementations, ViewRegistry is our last remaining source of runtime errors. Meanwhile, the indirection it provides is probably redundant with that already provided by having no restrictions on the workflow rendering type.

Swift already has a Screen marker interface, with a method that makes a screen responsible for instantiating its own ViewController. Isn't that enough?

• Android could just as easily have a Screen.buildView interface / method. That should still work nicely with the new LayoutRunner and ViewShowRendering code introduced in Hello LayoutRunner, goodbye Coordinator, BackStackEffect. #383 .
• Headless workflows aren't forced to use it as their rendering type
• It'd be easy enough to use mapRendering, or provide a mapViewController method

The only downside we see is that a workflow's view classes become a public concern. That doesn't seem like much of an issue for internal code.

For public code, e.g. a workflow-based library that might want to allow clients to bind its own view code and for which "default" view classes could be a wart, ViewRegistry could be revived as a tool for use within a root workflow. That's practically what's happening inside WorkflowLayout / ContainerViewController already.

[bencochran]

I played with a typing of this idea for Swift a while back and came up with this thought for that side of the world:

public protocol Screen {
    associatedtype ViewControllerType: ScreenViewControllerProtocol where ViewControllerType.ScreenType == Self
}

extension Screen {
    public func provideViewController() -> ViewControllerType {
        return ViewControllerType(screen: self)
    }
}


public protocol ScreenViewControllerProtocol: UIViewController {
    associatedtype ScreenType
    init(screen: ScreenType)
    func update(screen: ScreenType)
}


open class ScreenViewController<ScreenType>: UIViewController, ScreenViewControllerProtocol {
    public required init(screen: ScreenType) {
        super.init(nibName: nil, bundle: nil)
    }

    @available(*, unavailable)
    public required init?(coder aDecoder: NSCoder) { fatalError() }

    open func update(screen: ScreenType) {
    }
}

I like this idea quite a bit. One of my points of pain with the ViewRegistry and Swift is that I can’t make a generic screen like LoadingScreen<Wrapping: Screen> because I would have to register a LoadingScreen<MyFeatureScreen> for every screen I wanted to use wrapped in LoadingScreen.

With this change I could keep my LoadingViewController and LoadingScreen generic (which allows my workflow to render a non-erased type, making mapRendering have enough information to do something useful).

[rjrjr]

Your ScreenViewControllerProtocol looks very similar to what's going on in the ViewShowRendering.kt extensions introduced in #383 . Good sign.

[rjrjr]

@bencochran Why bother with the extension? If we really go for it, having the Screen protocol force a provideViewController method to be implemented seems like the whole point.

public protocol Screen {
    associatedtype ViewControllerType: ScreenViewControllerProtocol where ViewControllerType.ScreenType == Self

    public func provideViewController() -> ViewControllerType {
        return ViewControllerType(screen: self)
    }
}

[rjrjr]

And for that matter: what's the benefit of the associated type for the `ViewController?

public protocol Screen {
    public func provideViewController() -> ViewController 
}

[davidapgar]

WRT the associatedtype of ViewController, you have to do that for generics on protocols in Swift (so we have type safety)

[bencochran]

Yeah, we need the associated type so that we can update the resulting view controller with a new screen. We could theoretically remove that need by doing something like

public protocol Screen {
    func provideViewController(updates: Signal<Self, NoError>) -> UIViewController
}

But then every implementor has to do the ReactiveSwift plumbing, and … no thanks.

The extension’s necessary to provide a default implementation. There’s been talk of cleaning it up to look like you’ve written, but not in the language yet.

[rjrjr]

But then every implementor has to do the ReactiveSwift plumbing, and … no thanks.

Totally. That's exactly what #383 eliminates for Android devs.

[rjrjr]

One important counter argument that might torpedo this for Android is our (probably terrible) habit of sometimes injecting dependencies into our ViewController equivalents. ViewRegistry works really well with that. I suspect that making Screen.buildView injection friendly could lead to quite a mess.

[zach-klippenstein]

I mean, presumably it has access to the Context, so they could fall back to using Components as service locators.

[rjrjr]

I mean, presumably it has access to the Context, so they could fall back to using Components as service locators.

Yeah, I keep going back and forth on whether that would be such a bad thing -- it's a pretty terrible habit anyway, forcing the code that does it to be more terrible might be a good signal.

[bencochran]

Was chatting with @davidapgar about this again this afternoon and realized another approach Swift could take, somewhat out of Blueprint/SwiftUI’s book:

protocol Screen {
    associatedtype ViewControllerType: UIViewController

    func makeViewController() -> ViewControllerType
    func update(viewController: ViewControllerType)
}

And for convenience’s sake we could still provide a base view controller:

open class ScreenViewController<ScreenType>: UIViewController{
    public required init(screen: ScreenType) {
        super.init(nibName: nil, bundle: nil)
    }

    @available(*, unavailable)
    public required init?(coder aDecoder: NSCoder) { fatalError() }

    open func update(screen: ScreenType) {
    }
}

extension Screen where ViewControllerType: ScreenViewController<Self> {

    func makeViewController() -> ViewControllerType {
        return ViewControllerType(screen: self)
    }

    func update(viewController: ViewControllerType) {
        viewController.update(screen: self)
    }

}

A nice thing about this is that allows a Screen to use whatever view controller type it wants without the need for the protocol (and especially, doesn’t require a specific initializer).

[rjrjr]

I worry about this making the UI / logic boundary just blurry enough that we lose that natural separation of concerns that's been such a big win so far. Right now it's basically impossible for a workflow to monkey around with view instances and play dirty tricks. That ends the moment the rendering type has a method like makeViewController. And we know very well that anything which can be abused will be abused, nearly instantly.