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

Add better support for SwiftPM through the new Bundled protocol. #110

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

jjamminjim
Copy link

Better support for Swift Package Manager through a new Bundled protocol.

✍️ A few of the protocols' (i.e. StoryboardBased, NibLoadable and NibOwnerLoadable) default implementations attempt to load resources from the bundle containing the resource. With this PR it now does this through the Bundled protocol. Each time you declare conformance to StoryboardBased, NibLoadable or NibOwnerLoadable, you will have to provide access to the appropriate bundle. This can be achieved through one of the following methods...

1. For Cocoapod frameworks or main app bundles.

Declare your Reusable based classes to conform to BundledSelf. 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.

final class CustomCell: UITableViewCell, Reusable, BundledSelf { /* And that's it! */ }

2. For Swift Package Manager bundles.

Declare your Reusable based classes to conform to BundledModule. This provides a default implementation that uses the bundle associated with a Swift Package Manager Bundle (Bundle.module).

final class CustomCell: UITableViewCell, Reusable, BundledModule { /* And that's it! */ }

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...

public extension BundledModule {
    static var bundle: Bundle {
        Bundle.module
    }
}

3. For other uses.

Provide you own implementation of the static var bundle: Bundle property directly in your Reusable based class.

final class CustomCell: UITableViewCell, Reusable { 
  static var bundle: Bundle {
    return Bundle.myCustomBundle()
  }
}

Copy link
Owner

@AliSoftware AliSoftware left a 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 {
Copy link
Owner

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 {
Copy link
Owner

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:

  1. 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
  2. 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)

@AliSoftware AliSoftware self-requested a review September 10, 2021 20:20
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.

2 participants