[ASCellNode] Fix applyLayoutAttributes not called, add readonly layoutAttributes property (#2321)

AsyncDisplayKit/ASCellNode+Internal.h

@@ -12,6 +12,8 @@
 
 #import "ASCellNode.h"
 
+NS_ASSUME_NONNULL_BEGIN
+
 @protocol ASCellNodeInteractionDelegate <NSObject>
 
 /**
@@ -49,4 +51,13 @@
 - (void)__setSelectedFromUIKit:(BOOL)selected;
 - (void)__setHighlightedFromUIKit:(BOOL)highlighted;
 
+/**
+ * @note This could be declared @c copy, but since this is only settable internally, we can ensure
+ *   that it's always safe simply to retain it, and copy if needed. Since @c UICollectionViewLayoutAttributes
+ *   is always mutable, @c copy is never "free" like it is for e.g. NSString.
+ */
+@property (nonatomic, strong, nullable) UICollectionViewLayoutAttributes *layoutAttributes;
+
 @end
+
+NS_ASSUME_NONNULL_END

AsyncDisplayKit/ASCellNode.h

@@ -74,6 +74,15 @@ typedef NS_ENUM(NSUInteger, ASCellNodeVisibilityEvent) {
  */
 @property (nonatomic, assign) BOOL neverShowPlaceholders;
 
+/*
+ * The layout attributes currently assigned to this node, if any.
+ *
+ * @discussion This property is useful because it is set before @c collectionView:willDisplayNode:forItemAtIndexPath:
+ *   is called, when the node is not yet in the hierarchy and its frame cannot be converted to/from other nodes. Instead
+ *   you can use the layout attributes object to learn where and how the cell will be displayed.
+ */
+@property (nonatomic, strong, readonly, nullable) UICollectionViewLayoutAttributes *layoutAttributes;
+
 /*
  * ASTableView uses these properties when configuring UITableViewCells that host ASCellNodes.
  */

AsyncDisplayKit/ASCellNode.mm

@@ -233,6 +233,17 @@
 
 #pragma clang diagnostic pop
 
+- (void)setLayoutAttributes:(UICollectionViewLayoutAttributes *)layoutAttributes
+{
+  ASDisplayNodeAssertMainThread();
+  if (ASObjectIsEqual(layoutAttributes, _layoutAttributes) == NO) {
+    _layoutAttributes = layoutAttributes;
+    if (layoutAttributes != nil) {
+      [self applyLayoutAttributes:layoutAttributes];
+    }
+  }
+}
+
 - (void)applyLayoutAttributes:(UICollectionViewLayoutAttributes *)layoutAttributes
 {
   // To be overriden by subclasses

AsyncDisplayKit/ASCollectionView.mm

@@ -46,12 +46,15 @@ static NSString * const kCellReuseIdentifier = @"_ASCollectionViewCell";
 
 @interface _ASCollectionViewCell : UICollectionViewCell
 @property (nonatomic, weak) ASCellNode *node;
+@property (nonatomic, strong) UICollectionViewLayoutAttributes *layoutAttributes;
 @end
 
 @implementation _ASCollectionViewCell
 
 - (void)setNode:(ASCellNode *)node
 {
+  ASDisplayNodeAssertMainThread();
+  node.layoutAttributes = _layoutAttributes;
   _node = node;
   [node __setSelectedFromUIKit:self.selected];
   [node __setHighlightedFromUIKit:self.highlighted];
@@ -71,14 +74,25 @@ static NSString * const kCellReuseIdentifier = @"_ASCollectionViewCell";
 
 - (void)prepareForReuse
 {
+  _layoutAttributes = nil;
+  _node.layoutAttributes = nil;
+
   // Need to clear node pointer before UIKit calls setSelected:NO / setHighlighted:NO on its cells
   self.node = nil;
   [super prepareForReuse];
 }
 
+/**
+ * In the initial case, this is called by UICollectionView during cell dequeueing, before
+ *   we get a chance to assign a node to it, so we must be sure to set these layout attributes
+ *   on our node when one is next assigned to us in @c setNode: . Since there may be cases when we _do_ already
+ *   have our node assigned e.g. during a layout update for existing cells, we also attempt
+ *   to update it now.
+ */
 - (void)applyLayoutAttributes:(UICollectionViewLayoutAttributes *)layoutAttributes
 {
-  [_node applyLayoutAttributes:layoutAttributes];
+  _layoutAttributes = layoutAttributes;
+  _node.layoutAttributes = layoutAttributes;
 }
 
 @end

AsyncDisplayKitTests/ASCollectionViewTests.mm

@@ -21,6 +21,7 @@
 @interface ASTextCellNodeWithSetSelectedCounter : ASTextCellNode
 
 @property (nonatomic, assign) NSUInteger setSelectedCounter;
+@property (nonatomic, assign) NSUInteger applyLayoutAttributesCount;
 
 @end
 
@@ -32,6 +33,11 @@
   _setSelectedCounter++;
 }
 
+- (void)applyLayoutAttributes:(UICollectionViewLayoutAttributes *)layoutAttributes
+{
+  _applyLayoutAttributesCount++;
+}
+
 @end
 
 @interface ASCollectionViewTestDelegate : NSObject <ASCollectionViewDataSource, ASCollectionViewDelegate>
@@ -69,6 +75,16 @@
   };
 }
 
+- (void)collectionView:(ASCollectionView *)collectionView willDisplayNode:(ASCellNode *)node forItemAtIndexPath:(NSIndexPath *)indexPath
+{
+  ASDisplayNodeAssertNotNil(node.layoutAttributes, @"Expected layout attributes for node in %@ to be non-nil.", NSStringFromSelector(_cmd));
+}
+
+- (void)collectionView:(ASCollectionView *)collectionView didEndDisplayingNode:(ASCellNode *)node forItemAtIndexPath:(NSIndexPath *)indexPath
+{
+  ASDisplayNodeAssertNotNil(node.layoutAttributes, @"Expected layout attributes for node in %@ to be non-nil.", NSStringFromSelector(_cmd));
+}
+
 - (NSInteger)numberOfSectionsInCollectionView:(UICollectionView *)collectionView {
   return _itemCounts.size();
 }
@@ -360,6 +376,34 @@
   } completion:nil]);
 }
 
+- (void)testCellNodeLayoutAttributes
+{
+  updateValidationTestPrologue
+  NSSet *nodeBatch1 = [NSSet setWithArray:[cv visibleNodes]];
+  XCTAssertGreaterThan(nodeBatch1.count, 0);
+
+  // Expect all visible nodes get 1 applyLayoutAttributes and have a non-nil value.
+  for (ASTextCellNodeWithSetSelectedCounter *node in nodeBatch1) {
+    XCTAssertEqual(node.applyLayoutAttributesCount, 1, @"Expected applyLayoutAttributes to be called exactly once for visible nodes.");
+    XCTAssertNotNil(node.layoutAttributes, @"Expected layoutAttributes to be non-nil for visible cell node.");
+  }
+
+  // Scroll to next batch of items.
+  NSIndexPath *nextIP = [NSIndexPath indexPathForItem:nodeBatch1.count inSection:0];
+  [cv scrollToItemAtIndexPath:nextIP atScrollPosition:UICollectionViewScrollPositionTop animated:NO];
+  [cv layoutIfNeeded];
+
+  // Ensure we scrolled far enough that all the old ones are offscreen.
+  NSSet *nodeBatch2 = [NSSet setWithArray:[cv visibleNodes]];
+  XCTAssertFalse([nodeBatch1 intersectsSet:nodeBatch2], @"Expected to scroll far away enough that all nodes are replaced.");
+
+  // Now the nodes are no longer visible, expect their layout attributes are nil but not another applyLayoutAttributes call.
+  for (ASTextCellNodeWithSetSelectedCounter *node in nodeBatch1) {
+    XCTAssertEqual(node.applyLayoutAttributesCount, 1, @"Expected applyLayoutAttributes to be called exactly once for visible nodes, even after node is removed.");
+    XCTAssertNil(node.layoutAttributes, @"Expected layoutAttributes to be nil for removed cell node.");
+  }
+}
+
 /**
  * https://github.com/facebook/AsyncDisplayKit/issues/2011
  *