Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UIKit Synchronous IfLet #472

Closed
wants to merge 1 commit into from

Conversation

heyltsjay
Copy link
Contributor

@heyltsjay heyltsjay commented Apr 5, 2021

Adds a few utilities for synchronously unwrapping a Store with optional state.

Discussion:
In chatting with a few of the other UIKit + TCA developers, It seems like the DataSource pattern in UIKit is an awkward seam to transcend. Some folks are using the publisher version of ifLet to reconcile the optionality of indexing into an IdentifiedArray, and later (but in practice, immediately) assigning the unwrapped store to their cell. They are maintaining a parallel dictionary of AnyCancellables just to facilitate this unwrapping.

I'd wager its an anti-pattern to fight the datasource pattern and circumvent its reload mechanics with a direct subscription to CellState. I'd like to encourage folks to wire their UICollectionView.dataSource to be derived from their ViewStore, and for the dataSource to refresh in response to the Store.

A small step in the right direction, or a conversation starter at least, would be to expose some synchronous unwrapping of a store with optional state.

Here is how i'm using it in my project:

    func cell(collectionView: UICollectionView, indexPath: IndexPath, item: Item) -> UICollectionViewCell {
        let cellStore = self.store.scope(
            state: \.cells[item.id],
            action: { Action.cell(id: item.id, action: $0) }
        )
        .guardLet("data source is out of sync for item: \(item). This should never happen. Make sure collectionview data source is re-generated whenever viewStore changes.")
        
        return cellStore.cell(for: collectionView, at: indexPath)
    }

@stephencelis
Copy link
Member

Hey @heyltsjay! Thanks for the PR. We haven't had time to explore it yet, but hopefully will be able to provide feedback soon.

@heyltsjay
Copy link
Contributor Author

I think the naming/contract for the ifLet piece is not correct. Worst outcome would be for someone to accidentally use this API instead of the normal one because the compiler didn't yell at them for not storing their subscription.

Other options might be:

let cellStore = store.scope(...).ifLet()
let cellStore = store.scope(...).unwrap()
let cellStore = try? store.scope(...).unwrap()

@stephencelis
Copy link
Member

stephencelis commented Apr 8, 2021

@heyltsjay We dug into this a bit today and think we have a bit of a handle on the thorniness. A few initial thoughts came to us, but we may be overlooking things since we're still not super familiar with what you've been dealing with.

One thing that stands out is that we've specifically avoided synchronous access to state outside the view store because of the potential to miss out on updates. You can always "escape" this by calling ViewStore(store).state. So that could be an alternative way of handling things, but you'd need to also pass down the scoped send method. This is awkward, but totally doable, and maybe also helps emphasize the gotcha.

An alternative that we wonder if you've considered is pushing the cancellable into the cell. That way it would be the cell's responsibility to manage the lifecycle. Rather than configure the cell with a store directly, I think you could use publisherScope to configure the cell with a publisher of stores instead. This is the method store.ifLet calls under the hood. Sketching this out for a dequeued cell:

cell.publisher = self.store.publisherScope(
  state { storePublisher in storePublisher.compactMap(\.cells[id: item.id]) },
  action: { Action.cell(id: item.id, action: $0) }
}

This still isn't super ideal since the cell would have to unwrap the publisher's store and hold onto it separately, but it at least eliminates the cancellable responsibility in the view controller.

We also wanted to let you know that we've been planning a Point-Free Community for TCA projects, and we're going to start using that for additive functionality in TCA. In fact, we may even reduce some of the TCA surface area and move certain logic there, like all the current UIKit stuff. Most feature-based PRs we get are completely additive, so getting a central hub going for project features like this seems. We'll try to let you know when that's ready if you have things like this that might be a good fit 😄

@stephencelis
Copy link
Member

Oh, and in the meantime, what do you think of closing this PR and linking to it in a discussion as a proof-of-concept for folks to discuss/use?

@heyltsjay
Copy link
Contributor Author

Sure - moving discussion to #484

@heyltsjay heyltsjay closed this Apr 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants