-
Notifications
You must be signed in to change notification settings - Fork 3
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
Function Builder API #7
Comments
@JosephDuffy I'd love your input on this if you have the time to review it. |
I'm not sure that function builders will be useful or possible in they ways you're outlining. I think the motivation for this could be fairly easily solved by using a closure-based section: open class ClosureSection: Section {
public let wrappedSection: Section
public var numberOfElements: Int { wrappedSection.numberOfElements }
open weak var updateDelegate: SectionUpdateDelegate?
open required init(wrappedSection: Section) {
self.wrappedSection = wrappedSection
}
}
extension ClosureSection: SectionUpdateDelegate {
// Forward all `SectionUpdateDelegate` functions to `wrappedSection`. Ideally the `updateDelegate` would simply `get`/`set` on `wrappedSection`, but `===` is used in a few places internally that cause weird crashes or incorrect data.
} The name isn't really right but This by itself is not that useful, but can then be subclassed to provide similar functionality to what you're saying in the motivation: open class CollectionClosureSection<Cell: UICollectionViewCell>: ClosureSection, CollectionSectionProvider {
public typealias ConfigureCell = (_ cell: Cell, _ index: Int, _ section: Section) -> Void
public let dequeueMethod: DequeueMethod<Cell>
public let cellConfigure: ConfigureCell
open required init<Section: Composed.Section>(wrappedSection: Section, dequeueMethod: DequeueMethod<Cell>, cellConfigure: @escaping ConfigureCell) {
self.cell = cell
self.dequeueMethod = dequeueMethod
self.cellConfigure = cellConfigure // Here you would do similar to what's in `CollectionCellElement` to force cast the section to `Section`
super.init(wrappedSection: wrappedSection)
}
open section(with traitCollection: UITraitCollection) -> CollectionSection {
let cell = CollectionCellElement(section: self, dequeueMethod: dequeueMethod, configure: cellConfigure>
return CollectionSection(section: self, cell: cell)
}
} The caller can then: let peopleSection = CollectionClosureSection(
wrappedSection: ArraySection([Person(name: "Foo"), Person(name: "Bar")]),
dequeueMethod: .nib(PersonCell.self)
) { cell, index, section in
let element = section.element(at: index)
cell.prepare(with: element)
} Extensions could be added to add an Of course it would need fleshing out to provide e.g. headers and footers but I think it fixes:
It does not fix:
Although I don't think that's possible to fix via the API (with out or without function builders), are you suggesting allowing the view to be any I think that functions builders aren't the silver bullet that can fix the API for this. A big part of this is because SwiftUI makes heavy use of Your first example could be quite easily implemented using regular closures and I do not see the advantage of using function builders: Section(people) { index in
Cell(.fromNib(PersonCell.self) { cell in
cell.prepare(people[index])
}
} This is essentially the If this were to a function builder like the example: Section {
Cell(.fromNib(PersonNameCell.self) { cell in
cell.prepare(person.name)
}
Cell(.fromNib(PersonAgeCell.self) { cell in
cell.prepare(person.age)
}
} I don't see how Composed can know which cell to show for each row. As you say you could implement If you were to implement this: @_functionBuilder
struct SectionBuilder {
static func buildBlock(_ cells: Cell...) -> [Cell] {
cells
}
}
struct Section {
let cells: [Cell]
init(@SectionBuilder _ content: () -> [Cell]) {
cells = content()
}
} What do you do with the array of cells here? It could be useful for registration but how do you get a cell for a given row? Another issue I see is diffing. SwiftUI is not slow because it does very good diffing of the view tree. The first step would be to iterate over every section and row to get the section and row counts, but at this point at lot of work has been done inside the Making the function builder API in your examples (with When considering:
I don't think function builders can be a piece of syntactic sugar atop the existing API, but I do think using closures for more simple sections would help.
I think this would be a monumental task that would pretty much be recreating SwiftUI but with less features. |
First off, thank you for the detailed input. Really great points @JosephDuffy
This was my concern as well, I said 'proposal' however I probably should've called it 'exploration' as that's more inline with what it is. I agree, it may not be possible, I just think the output/design can be a great driver towards for the conversation. Which I feel has served its purpose here.
That's a great idea, and might be a little more composable on top of the existing implementation.
True, but I'm up for renaming in v2.0 since its mostly used internally. Yes, you and other did/could create custom
Yeah, that's when it becomes more beneficial IMHO. I like how you're 'wrapping' the
Agreed.
I definitely wasn't suggesting this needed fixing, just that its a constraint we have to work within 👍
This is a really great observation actually. I was aware (have been using SwiftUI a lot) of the heavy usage of
This is a fair point, but it's also the most trivial example. We should review this again with a more complex structure once we have some initial implementation details in place.
Yeah I wasn't sure about this yet either.
Another valid point.
Good point but I did have some thoughts on that. So I wasn't advocating that was stop deferring resources until required. I felt that the new API could instead be more of a DSL (a definition of a
Actually I don't that's entirely true. My thinking was that
100% agreed! I think you've made some extremely valid point overall, so I'm going to explore the closure based approach. Its more of an enhancement for starters, but also, I like this for another reason – it doesn't require us to drop support for older iOS versions. The primary driver for this conversation is to address 3 key areas:
Usability I feel like at the moment, one of the main pain points is being forced to create a subclass for even trivial usage. I like the closure based wrapper idea, because it potentially solves this well. But as you mentioned, it definitely requires some fleshing out to see where its limitations fall and how we make that clear to an API consumer. Also when they hit those boundaries, how do they know what approach should be taken to resolve. Discoverability Another pain point is that you have to look up the docs or the headers to know what protocols you can conform to, in order to add additional behaviour to your sections. Furthermore, even if you conform to the protocol, the library generally provides reasonable default implementations of many methods to appease UIKit delegate/datasource methods. As a result it can be difficult to know what you need to implement to get your desired behaviour. Another side affect I've found that I'm not happy about is that if you update Composed and we've deprecated a method on a protocol, you likely won't get any help from the compiler, since you're not the one calling it. I've had this a few times during the development, where I simply changed the order of the arguments for example, and couldn't work out why the function was never called. Using protocol composition is great, but its problematic for the API consumer in these scenarios. Readability I think a subclassed However the main issue I find, is that defining the Just look at the tests and you can see that it requires careful 'reading' to work out if you've defined the structure correctly. Improving the readability for as a result improve the ability to reason about the structure. Summary With those targets in mind, I think the closure based approach is a worthy contender for the moment. I'll flesh it out some more and then perhaps make a Draft PR so we can discuss around something with more context. This issue can remain open for further discussions and explorations as well. Sound good @JosephDuffy ? |
So I've explored this a little more and found some challenges already. For reference, the implementation introduces a new type An issue arises where the closures need to reference the internal There are a couple solutions:
One other issues I've found is correctly propagating I'd love any thoughts/feedback you might have @JosephDuffy – you can checkout the code on the branch |
As part of the diffing I think using the identifier of the section – rather than the section itself – would be a good approach. This would make some of the delegate calls easier to understand and would probably help with debugging. It's a fairly large change from an API standpoint so I would guess starting with a new coordinator will be easier. Especially to build in more debugging. |
Totally agree! And also yeah, I did add a section identifier too. I also think it'll just make the whole DiffableDatasource integration simpler as a result as well. |
So although I got a fair way into this. Ultimately it's not looking likely due to the code-generation that would need to be included to make this work nicely. I'm keen to explore closure based sections but my early experiments hit a few difficult to solve issues. Closing for now. |
Adding
FunctionBuilder
supportIntroduction
I'd like to add support for the new function builder APIs.
Motivation
The current implementation requires the creation of a
class
with the inclusion of at least 1method
. In addition, it is required to create at least 1 cell class (and associatedXIB
?).This doesn't sound like a lot but in practice it can still quickly become cumbersome when building reusable code.
Most often its necessary to define your own generic types on top of Composed's options:
Even in this rudimentary example, that's still a fair amount of code. What's important to capture here is:
Array
UICollectionView
PersonCell
Proposed solution
Add a
FunctionBuilder
style DSL to simplify usage for an API consumer as well reduce the code required to achieve the same result.There are a few added benefits to this approach:
Array
)Presentation
One thing to note,
Cell
needs to be inferred so the above wouldn't compile until it's inserted into a view. A nice solution to this might look like the following:Using this approach we can see:
struct
removes reference semantics and the potential for retain cyclesUICollectionView
is intended for the presentationUICollectionViewCell
is what we're expecting for ourCell
configurationHeaders & Footers
However another added benefit to using the view to infer our
Section
type is that we can also now determine what 'features' ourSection
might have.For example, we know that our
UICollectionView
supports header/footer elements:We could even go further, providing a convenience header type:
Static Views
One final advantage over this API is that it makes it trivial to build both dynamic as well as static
Section
's.Existing Sections
In the current implementation we have a few
Section
types that define the backing data that will be used for that section.SingleElementSection
ArraySection
ManagedSection
As well, we have two
SectionProvider
types that help us compose sections:ComposedSectionProvider
SegmentedSectionProvider
Using the proposed implementation, we can see all of these become completely unnecessary.
SingleElementSection
A
SingleElementSection
can be replaced by simply defining a static section:ArraySection & ManagedSection
Both can be defined by simply providing the relevant data to the
Section
ComposedSectionProvider
Simply including multiple sections removes the need for a specific type as well.
SegmentedSectionProvider
That leaves us with just one final type. The purpose of this type is to hold onto 1 or more child
Section
's while keeping only 1 active at a time.Well, this is easily achieved with the above implementation simply with the use of conditional statements.
Conclusion
It's now apparent that this implementation should remove the need for an API consumer (at least) to ever have any knowledge of multiple
Section
types. Instead focus on providing the data (or not) for aSection
and specifying whichCell
they want to be used for representing the element at a specified index.As for composition, using Swift knowledge you already have, you should easily be able to build composable
Section
's using nothing more than conditional statements.The use of
FunctionBuilder
APIs also removes the additional syntax of brackets and comma's that would otherwise (and currently) litter your code.Behaviours
ComposedUI
also extends yourSection
through the use of protocol conformance to provide behaviours like editing, selection, drag and drop and more.One potential solution here would be the use of a
Modifier
-style API as seen inSwiftUI
:There are a few obvious benefits to this approach:
Composed
will likely support SwiftUI and this API will 'feel' a lot more familiar and consistent.Final Thoughts
This approach is not only simpler and cleaner, its also much more user friendly and discoverable. It removes a few key pain points for users as well:
One last thing...
There are 2 other areas that still need to be thought through.
Layout, can likely be solved by initialising the view with a layout (if required). The bigger question is how to tie individual section layouts to the outer view layout. However we've solved it already, so I'm sure this can be worked through.
Environment variables are passed to functions or closures currently. These could replaced by something more like
@Environment
available in SwiftUI where Composed ensures the variables are set automatically for you at the right moment. I'm not sure atm if this is possible or something the compiler does for free in SwiftUI alone. I will need to investigate this further. Alternatively, the functions/closures could continue to provide these values as required.Considerations
The current approach to handle section updates and ensure they get propagated up to the coordinator is to use the delegate pattern. In order to support this 1 of 2 things should be considered.
My current instincts are to consider the usage of
Combine
and the new Swift diffing APIs, however this would mean dropping support for iOS <13. I'm not adverse to this, but it should be considered carefully.Source compatibility
This is likely to cause breaking API changes and require significant renaming in both ComposedUI and Composed. As such its planned for 2.0.
Alternatives considered
N/A
The text was updated successfully, but these errors were encountered: