-
Notifications
You must be signed in to change notification settings - Fork 238
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
Add better support for SwiftPM through the new Bundled
protocol.
#110
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR, and sorry it took me so long to reply!
Left a quick question, and I'll also let you update the PR to fix the conflicts that arose after the 4.1.2 release.
Btw, don't forget to credit yourself by adding en entry under the main branch
section in the CHANGELOG.md
! 🙂
@@ -20,7 +20,7 @@ import Reusable | |||
* Which in fact is just a convenience typealias that combines | |||
* `NibLoadable` & `Reusable` protocols. | |||
*/ | |||
final class CollectionHeaderView: UICollectionReusableView, NibReusable { | |||
final class CollectionHeaderView: UICollectionReusableView, BundledSelf, NibReusable { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nitpick: for consistency with other examples, what do you think about putting BundledSelf
after NibReusable
instead of before? This is not a big deal and doesn't change the behaviour, but I just thought it would be nice to align with the rest 🙂
/// } | ||
/// ``` | ||
|
||
public protocol BundledModule: AnyObject, Bundled { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if it's really worth making Reusable
itself provide this protocol, given that we don't provide a default implementation for it (I think that's because we can't do it in a way that would make it still work and compile if not integrated via SPM? Though I'll have to re-read the SE proposal about SPM Resource Bundles again as I thought they made a point trying to make this work…)? 🤔
If we can't provide a default implementation for those ☝️ reasons, and we have to let the users of Reusable
declare the default implementation themselves, I'd say it's probably simpler to let them declare the protocol too (in addition to the default implementation), that way:
- They won't be surprised if it does work if they conform their class to our
BundleModule
but forgot to follow the instructions to provide the default implementation - They could choose their own name (in case e.g. that name conflicts with a protocol or class of the same name in their own codebase or if they prefer something else)
Better support for Swift Package Manager through a new
Bundled
protocol.1. For Cocoapod frameworks or main app bundles.
Declare your
Reusable
based classes to conform toBundledSelf
. This provides a default implementation that uses the bundle associated with the class (Bundle(for: self)
). In previous versions of the framework this was the default.2. For Swift Package Manager bundles.
Declare your
Reusable
based classes to conform toBundledModule
. This provides a default implementation that uses the bundle associated with a Swift Package Manager Bundle (Bundle.module
).In order for the resource to be loaded from the bundle of your package, you will also need to add the following extension somewhere in your package framework...
3. For other uses.
Provide you own implementation of the
static var bundle: Bundle
property directly in yourReusable
based class.