Skip to content

Commit

Permalink
Expose ListGenericSectionController.object and ListSectionController.…
Browse files Browse the repository at this point in the history
…collectionContext as implicitly-unwrapped optionals

Summary:
These methods are currently correctly annotated as `nullable`, because they can return `nil`. However, this is not practical for writing actual Swift code that uses the APIs. Overrides of `cellForItem(at:)` //must// return a `UICollectionViewCell`. Without a collection context, there's no good way to get one, so callsites must either:

- Force-unwrap the `collectionContext`.
- `guard let`/`else fatalError` the `collectionContext`.
- Return a default `UICollectionViewCell`, which just hides the bug.

None of these are good: we don't want to encourage things force-unwraps and `fatalError` in idiomatic product code, because people will see them and get the idea that these patterns are okay to use elsewhere, and they are not. This also happens with `IGListGenericSectionController`'s `object` value: to use it when creating a cell, it must be explicitly unwrapped.

We could make these `nonnull`, since there's no enforcement of the annotations in Objective-C, that feels a bit too far, and it would break existing code that uses the optional API, because you can't force-unwrap (etc.) a non-optional value. Instead, we can use `null_resettable` explicitly. [While it's not covered by name in the Apple docs](https://developer.apple.com/documentation/swift/objective-c_and_c_code_customization/designating_nullability_in_objective-c_apis):

> Without a nullability annotation or with a null_resettable annotation—Imported as implicitly unwrapped optionals

...it bridges the property to Swift as an implicitly-unwrapped optional. I suppose it's implied as the equivalent of "without a nullability annotation".

This works even for `readonly` properties that can't be reset, and it solves both issues: it feels like less of a "100% `nonnull`" guarantee, and it's backwards-compatible with code using optionals.

To demonstrate that this is backwards-compatible with existing Swift code, this test code, which runs through various optional/non-optional APIs, compiles:

```
final class TestSectionController: ListSectionController {
    override func cellForItem(at index: Int) -> UICollectionViewCell {
        let cell = collectionContext.dequeueReusableCell(for: self, at: index)
        let optional = collectionContext?.dequeueReusableCell(for: self, at: index)
        if let _ = collectionContext {}
        return optional ?? cell
    }
}

final class TestGenericSectionController: ListGenericSectionController<NSString> {
    override func cellForItem(at index: Int) -> UICollectionViewCell {
        let label = UILabel() // imagine it's from a cell
        label.text = object as String
        label.text = (object as String) + "Suffix" // wouldn't work with optional
        label.text = object.map { $0 as String }
        if let object = self.object {
            label.text = object as String
        }
        return collectionContext.dequeueReusableCell(for: self, at: index)
    }
}
```

Reviewed By: patters, lorixx

Differential Revision: D23603716

fbshipit-source-id: e4750dcfe0072d482951dbf2e9efb1ee3de46884
  • Loading branch information
natestedman authored and facebook-github-bot committed Sep 10, 2020
1 parent 02ffb11 commit a6526ce
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 3 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@ The changelog for `IGListKit`. Also see the [releases](https://github.com/instag

- Removed unneeded diffing functions `IGListDiffExperiment(...)` and `IGListDiffPathsExperiment(...)`. [Maxime Ollivier](https://github.com/maxolls) (tbd)

- `ListSectionController.collectionContext` and `ListGenericSectionController.object` are now implicitly-unwrapped optionals in Swift. [Nate Stedman](https://github.com/natestedman) (tbd)

- The argument of `IGListGenericSectionController`'s `-didUpdateToObject:` is now generic, not `id`. [Nate Stedman](https://github.com/natestedman) (tbd)

### Enhancements

- Introduce `IGListSwiftKit`, with Swift refinements for `dequeueReusableCellOfClass` methods. [Koen Punt](https://github.com/koenpunt) [(#1388)](https://github.com/Instagram/IGListKit/pull/1388).
Expand Down
5 changes: 3 additions & 2 deletions Source/IGListKit/IGListGenericSectionController.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,10 @@ NS_SWIFT_NAME(ListGenericSectionController)
returned.
@note This object is briefly `nil` between initialization and the first call to `didUpdateToObject:`. After that, it is
safe to assume that this is non-`nil`.
safe to assume that this is non-`nil`. For this reason, we bridge it to Swift as an implicitly-unwrapped Optional, so
that idiomatic IGListKit code is not forced to handle nullability with explicit `as!` or `fatalError`.
*/
@property (nonatomic, strong, nullable, readonly) ObjectType object;
@property (nonatomic, strong, null_unspecified, readonly) ObjectType object;

/**
Updates the section controller to a new object.
Expand Down
7 changes: 6 additions & 1 deletion Source/IGListKit/IGListSectionController.h
Original file line number Diff line number Diff line change
Expand Up @@ -147,8 +147,13 @@ NS_SWIFT_NAME(ListSectionController)
A context object for interacting with the collection.
Use this property for accessing the collection size, dequeuing cells, reloading, inserting, deleting, etc.
@note When created outside of `-listAdapter:sectionControllerForObject:`, this object is temporarily `nil`
after initialization. We bridge it to Swift as an implicitly-unwrapped Optional, so that idiomatic IGListKit
code is not forced to handle nullability with explicit `as!` or `fatalError`, as using a non-`nil` instance
of this object is essential for dequeueing cells.
*/
@property (nonatomic, weak, nullable, readonly) id <IGListCollectionContext> collectionContext;
@property (nonatomic, weak, null_unspecified, readonly) id <IGListCollectionContext> collectionContext;

/**
Returns the section within the list for this section controller.
Expand Down

0 comments on commit a6526ce

Please # to comment.