[ASNodeController] Improvements for initial, new class

[garrettmoon]

From @hannahmbanana on February 1, 2017 6:16

Opening this to track Adlai's comments in #2945.

A couple thoughts, perhaps just for the future:

If we:

• Add an initializer with initWithNodeBlock:
• Add support for setting a size range
• Decide it's appropriate

We could add a collection data source API method collectionNode:nodeControllerForItemAtIndexPath: which would nicely combine the size range & node block information for each item.

One major question with this, however, is how we would determine the new size range for the item if the collection view changes its bounds size. The containerSize field on trait collections is completely undocumented (!!) but maybe it could be useful so that a node controller could recompute its size range? Or maybe some other mechanism/protocol for the data controller to ask the node controller for its new size range?

Copied from original issue: facebookarchive/AsyncDisplayKit#2964

[garrettmoon]

From @ImJCabus on March 6, 2017 11:40

Are there any performance considerations when we introduce another 'layer' above nodes?

I'm currently employing a similar architecture in one of my apps. A ViewModel protocol that has a simple func createNode() -> ASCellNode - basically the same as NodeController without the neat embedding in the framework, though (interfaceState callbacks, etc.). These ViewModels are used to build layouts upfront and abstract away the 'ad-hoc design' that usually comes with ASCollectionDataSource (node[Block]ForItemAt:). The abstraction is handled by an Adapter class that conforms to ASCollectionDataSource and ASCollectionViewLayoutInspecting and takes care of the mapping. ViewModels store the content associated with the nodes they vend. - e.g. a simple String for a 'TableRow', or something more complicated like an AVPlayer for nodes that play music or video.

See this demo repo: https://github.com/ImJCabus/AdapterViewModel There is also an example of the data modelling in there (SettingsAdapter).

Now the documentation states that we should:

• Use node blocks if possible. These are run in parallel on a background thread, resulting in 10x performance gains.
• Use nodes to store things about your rows.

Since we ought to store the contents in nodes, the ViewModel or NodeController layer seems a bit redundant - Most of the ViewModels implementations do nothing but create the ASCellNode and set their data, something that could also be done by the node itself. - So why not get rid of the extra layer, right? However, there are ViewModels (at least in my app) like e.g. a UserProfileHeaderModel, the implementation of which is much more sophisticated. (We're talking networking, KVO, etc.) For these cases, separating the mere 'View' aspect of Nodes (size calculation, layout, etc.) from data storage, updates, networking, etc. seems like an reasonable approach.

Wanting to keep the abstraction a thing like Adapter grants, the alternative of providing a 'Wrapper Layer' like ViewModel or NodeController would mean creating ASCellNodes upfront and store them in the Adapter, simply returning them in nodeForItemAt:. This seems like a very bad idea, given that we would lose the benefit of creating Nodes in the background, because data modelling is done primarily on the main thread - this is were networking callbacks usually arrive.

I remember a discussion about an alternative data modelling approach for ASCollectionNode somewhere on GitHub - but I can't find it anymore :/

Update

I just realised I didn't actually pose a question here 🙈. Just wanted to contribute to the discussion about the NodeController layer. I'm not quite sure if I'm happy with the architecture I've built myself - so any progress in ASDK regarding this topic will be a useful pointer in the right direction for me, too. :)

[grangej]

Just curious is this going anywhere ? I am starting to implement Texture in an app and was curious if I should try ASNodeController or stick with ASTableNode ?

On a side note the app heavily uses RxDataSources currently and trying to find a smooth path for migration.