layout icon indicating copy to clipboard operation
layout copied to clipboard

RFC: Towards a more strictly-typed Layout

Open nicklockwood opened this issue 7 years ago • 3 comments

One of the main turn-offs with Layout as it stands is that it has relatively weak static type safety. There are two main areas where this is an issue:

  1. When creating a LayoutNode, the view/controller, constants and state are all weakly-typed. The view must be cast to the correct sub-type, the constants are a json-like dictionary of [String: Any], and while state can be specified as a struct, checking that the same struct type is used in multiple places is done at runtime, not compile time.

  2. Runtime expressions have statically defined return types, but weakly-typed arguments, and so the only way to type-check an expression is to evaluate it and verify the correct type is returned. Expressions are side-effect-free, so this is not a huge problem, but it would still be preferable if they could be checked statically.

For the purposes of this doc, I'm going to concentrate on the first problem. The current API for creating a LayoutNode programmatically is:

class LayoutNode {
    init(
        view: UIView? = nil,
        outlet: String? = nil,
        state: Any = (),
        constants: [String: Any]...,
        expressions: [String: String] = [:],
        children: [LayoutNode] = []
    )
}

The children and outlet are fine, but everything else is fairly loose by Swift standards. We could make this API much more strict by using generic types for everything:

class LayoutNode<
    View: UIView,
    State: ExpressibleByDictionaryLiteral,
    Constants: ExpressibleByDictionaryLiteral,
    Expressions: ExpressibleByDictionaryLiteral
> {
    init(
        view: View? = nil,
        outlet: String? = nil,
        state: State = [:],
        constants: Constants =  [:],
        expressions: Expressions = [:],
        children: [LayoutNode] = []
    )
}

This is certainly strict, but (among other problems) it would be very verbose to actually instantiate a node hierarchy if we had to specify all these types every time, since typically the majority of these properties are left blank.

Also, before we get too hung-up on the view and expressions, it's worth bearing in mind that LayoutNode.init() is not the most commonly-used API for creating a LayoutNode. The most common one is this:

<UIView outlet="foo" top="5" left="0">
    <UILabel text="Hello World"/>
</UIView>

That's right - XML. The view and expressions are both typically specified as strings at runtime, so there is not a lot to be gained by making a very type-strict API for those.

Notably absent from the XML above are constants and state. The reason for that is that those are always set using Swift code, typically when loading the XML, using an API like this (slightly simplified):

func loadLayout(
    named: String,
    state: Any = (),
    constants: [String: Any]...
)

This method is part of the LayoutLoading protocol, which is implemented by LayoutViewController, the typical integration point between Layout XML and Swift in a Layout-based app. The state can then later be updated by calling setState() on the LayoutNode, which is exposed as a property of the LayoutViewController:

self.layoutNode?.setState(...)

Constants and state are pretty similar, but constants are set only once, and tend to be shared across the whole application, whereas state may be set in multiple places, and is typically unique to each Layout tree.

Also worth noting is that both state and constants tend to be set only on the root node of a subtree (typically a screen, or an individually re-usable element like a table cell). It's possible, but not especially useful, to set state directly on a sub-node inside a Layout hierarchy.

So what if we removed the option to set state and constants on any node, and created a new type of node just for that purpose:

class LayoutNode {
    init(
        view: UIView? = nil,
        outlet: String? = nil,
        expressions: [String: String] = [:],
        children: [LayoutNode] = []
    )
)

class RootNode<
    State: ExpressibleByDictionaryLiteral,
    Constants: ExpressibleByDictionaryLiteral
>: LayoutNode {
    init(
        view: UIView? = nil,
        outlet: String? = nil,
        state: State = [:],
        constants: Constants = [:],
        expressions: [String: String] = [:],
        children: [LayoutNode] = []
    )
    func setState(_: state)
}

This seems a bit better. Only the root node of any give hierarchy gets the extra verbosity of requiring a State type to be specified, and we can now enforce that setState() is always called with the same type of state as init was.

It's not completely clear that the Constants generic type adds as much value, since it's set only once and then mostly accessed using runtime logic. Also, we lose the ability to easily add a couple of extra constants for each node by using the variadic dictionary merging feature of the current API.

Another problem is that, again, we don't typically use this API to create nodes; we use this one:

self.loadLayout(named: "Foo.xml", state: ..., constants: ...)

So perhaps instead of a stricter root node type, the burden of owning and updating state should be pushed back to the controller level:

class LayoutViewController<State: ExpressibleByDictionaryLiteral>: UIViewController(
    init(
        layoutName: String
        state: State,
        constants: [String: Any]
    )
    var layoutNode: LayoutNode?
    func setState(_: State)
)

This seems better, but making it a ViewController is rather inflexible. What about table views, where we need to set the state for each table cell node?

So perhaps instead of a LayoutViewController, we have a lightweight LayoutNodeController that's just a plain-old-object:

class LayoutNodeController<
    State: ExpressibleByDictionaryLiteral
>: UIViewController(
    init(
        state: State,
        constants: [String: Any]
    )
    var layoutNode: LayoutNode
    func setState(_: State)
)

And then LayoutViewController can own one of these, instead of a LayoutNode:

class LayoutViewController<State: ExpressibleByDictionaryLiteral>: UIViewController(
    init(
        layoutName: String
        state: State,
        constants: [String: Any]
    )
    var layoutController: LayoutNodeController?
)

Anyway, that's the thought process so far. Feedback welcome!

nicklockwood avatar Aug 21 '17 22:08 nicklockwood

How about, for starters, only making the state strict on LayoutNode:

class LayoutNode< State: ExpressibleByDictionaryLiteral> {
    init(
        view: UIView? = nil,
        outlet: String? = nil,
        state: State = [:],
        constants: [String: Any]...,
        expressions: [String: String] = [:],
        children: [LayoutNode] = []
    )
}

marius-serban avatar Aug 21 '17 23:08 marius-serban

@marius-serban you'd have to also add it as a parameter to all the various load functions, since you don't often create LayoutNodes directly.

If would also then mean that all children nodes would have to share the same State type, which would be problematic for situations like TableView where the cells don't share the state of their parent.

The only other problem I can see is that it would be a big breaking change to practically every API in the framework.

I'd like to understand what the end destination is before making such a significant change, because making LayoutNode generic would potentially prevent other changes in future. For example, I couldn't add a View type param in the same way because then child nodes would all have to be the same type.

nicklockwood avatar Aug 22 '17 00:08 nicklockwood

I quite like the LayoutNodeController solution. It’s always a good idea to not tie too heavily to UIViewController. I’d love to see a test PR to play with to give more feedback.

hartbit avatar Aug 22 '17 10:08 hartbit