IGListKit Support II: Electric Boogaloo #2942
Conversation
| ASDisplayNodeAssertThreadAffinity(self); | ||
| ASDN::MutexUnlocker l(__instanceLock__); | ||
| body(self); | ||
| } else if (_onDidLoadBlocks == nil) { |
There was a problem hiding this comment.
Changed this behavior to call -didLoad before any onDidLoadBlocks, and no longer to throw if they call on main on a loaded node (just runs block).
There was a problem hiding this comment.
Fantastic effort this patch represents! Thanks for including a really solid gauntlet of tests (it will be a good base to use in the automated performance tests, and we'll add sophistication over time)
Definitely interested in anything you might want to put up which implements this capability while taking advantage of some of these codepaths. #2948
|
|
||
| @implementation ASListKitTests | ||
|
|
||
| - (void)setUp { |
There was a problem hiding this comment.
nit: The default-inserted methods use the brace-at-end-of-name style, so we should bump it down.
| return [[ASListTestObject alloc] initWithKey:self.key value:self.value]; | ||
| } | ||
|
|
||
|
|
There was a problem hiding this comment.
nit: Extra return here. Is there a standard on protocol names? Would it make sense for the pragma mark to say or protocol ?
| @@ -26,8 +27,8 @@ | |||
| #pragma mark - _ASCollectionPendingState | |||
|
|
|||
| @interface _ASCollectionPendingState : NSObject | |||
| @property (weak, nonatomic) id <ASCollectionDelegate> delegate; | |||
| @property (weak, nonatomic) id <ASCollectionDataSource> dataSource; | |||
| @property (weak, nonatomic) id <ASCollectionDelegate> delegate; | |||
There was a problem hiding this comment.
Revert this change unless you see a benefit
| @@ -27,6 +28,8 @@ | |||
| #import "ASSectionContext.h" | |||
| #import "ASCollectionView+Undeprecated.h" | |||
| #import "_ASHierarchyChangeSet.h" | |||
| #import <objc/runtime.h> | |||
There was a problem hiding this comment.
Put runtime.h at the bottom (or top). This imports list should have its order cleaned up (alphabetized or grouped by functional area)
|
|
||
| memset(&_asyncDataSourceFlags, 0, sizeof(_asyncDataSourceFlags)); | ||
| _asyncDataSourceFlags = {}; | ||
|
|
There was a problem hiding this comment.
Should not have space after. I'm split on this change -- I think the memset is much more explicit, at least since I'm not sure it is widespread knowledge that {} will zero memory for the specific size of a struct type (particularly in an equality like this, where we need to overwrite existing values, as opposed to initialization-time).
There was a problem hiding this comment.
I've applied all of your suggestions save this one, with the rationale that:
- This is a higher level of abstraction (symbol vs memory)
- If a dev doesn't understand what
= {}means then they should look it up and learn, tough love 💪
There was a problem hiding this comment.
The Sample applications each give a valuable reference for new users of *ListKit, too!
cc @rnystrom, @jessesquires, @zhubofei thanks for building a great framework in IGListKit! It helped reduce the complexity of ASDK by removing our custom diffing implementation 👍
|
💯 🎉 |
|
Here we go! |
|
So awesome, great work @Adlai-Holler, going to have to play around w/ this! Also 👋 @appleguy ❤️, we've come full circle! |
This supersedes #2848 which was a bit hacky and messy.
ASCollectionDataSourceInteropandASCollectionDelegateInteropprotocols to formalize what hooks we need to support IGListKit.IGListAdapterBasedDataSourceto connect IGListKit to AsyncDisplayKit using the existing dataSource/delegate architecture.To use IGListKit:
ASSectionController.ASSupplementaryNodeSource.IGListAdapter, call-setASDKCollectionNode:to make the connection.Note: Use the provided implementations from
ASIGListKitMethodImplementations.hto cover required IGListKit methods that don't apply when used with ASDK.@ay8s @appleguy @nguyenhuy @maicki @garrettmoon @jessesquires @ocrickard