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

UIApplication avoidance #46

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

KieranHarper
Copy link
Contributor

Added an optional closure parameter that can be invoked when the user wants to open the cocoapods URL, rather than using UIApplication.

This is needed in situations where the view controller lives in a framework target that doesn't allow UIApplication to be used.

… wants to open the cocoapods URL, rather than using UIApplication.

This is needed in situations where the view controller needs to live in a framework target that doesn't allow UIApplication to be used.
@vtourraine
Copy link
Owner

Hello Kieran, and thanks for the pull request.

I appreciate that not relying on UIApplication in those situations would be valuable, but that shouldn’t come at the expense of the most-used configuration. Unfortunately, I don’t think there’s a good solution for that, because you can’t really test at build time if the target supports UIApplication. Maybe add a preprocessor check (#if etc)?

If somebody has a better solution, please let us know.

@ApolloZhu
Copy link

ApolloZhu commented Oct 25, 2018

In Objective-C, you can use NS_EXTENSION_UNAVAILABLE macro, but in Swift, there isn't really an replacement for that.

So what I did is this:

https://github.com/LiulietLee/LLDialog/blob/master/Source/LLDialog.swift#L69-L73

Since apple/swift#12410 got merged, you can just use @available to check if it's app extension.

I don't think there's enough sample to prove this implementation is App Store compatible, but I discussed it with one of the Apple engineers during WWDC17. He said it "should" be fine.

@vtourraine
Copy link
Owner

That’s super interesting. Thank you very much for pointing that out, @ApolloZhu!

I guess we should update this pull request to replace the closure with the appropriate @available checks, then I‘ll be happy to merge it in.

@vtourraine
Copy link
Owner

So I’ve tried adding the @available checks on openCocoaPodsWebsite(), as pointed out by @ApolloZhu. It works, but unfortunately that only moves the problem. Now the compilation error is on the UITapGestureRecognizer instantiation, because the selector is not available for Extensions:

'openCocoaPodsWebsite' is unavailable: This method is NS_EXTENSION_UNAVAILABLE.

It seems like this should be easily solvable by testing the availability when creating the gesture recognizer, but as fas as I understand, this type of condition does not exist. Something like if #available(iOSApplicationExtension 1.0, *) {...} wouldn’t help, because the * is mandatory, so we can’t limit just for Extensions.

To reiterate: I want to keep the current behavior in a non-Extension context, and I don’t want to require more code from the app using this library.

Am I missing something, or do we need to wait for more flexible compiler instructions?

Base automatically changed from master to main February 1, 2021 13:59
@ifrins
Copy link

ifrins commented Jul 31, 2021

FYI in XCode 13b3 this started causing issues, as the SwiftPM packages are being built with -application-extension flag, so now any app that imports AcknowList package fails to build.

Looking at the discussion in Swift Forums, right now the only solution seems to be annotating either the class or some part of it with @available(iOSApplicationExtension, unavailable).

I see two options here:

  • We can create two different VC from the current one, one for main apps (with the unavailable annotation) and the other for extensions
  • Use this PR as a base and add some additional constructors, mark those with the unavailable annotation, and only those would have an urlOpener.

@vtourraine Are you open to receiving any PR for this or you'd rather wait and see if there's any fixes in this area on further betas?

@vtourraine
Copy link
Owner

@ifrins Hello Francesc, and thanks a lot for raising this issue!

It looks like we should do something about it, but maybe there’s an easier solution. I think we can add the @available() statement to the whole AcknowListViewController class, which would silence the compilation error. I’ve never used this library from an app Extension, but maybe I’m missing other use-cases?

@available(iOS 9.0.0, tvOS 9.0.0, *)
@available(iOSApplicationExtension, unavailable)
open class AcknowListViewController: UITableViewController {

@ifrins
Copy link

ifrins commented Aug 2, 2021

What you suggested works (I created a fork doing exactly that to unblock myself 😊); but it would break if anyone is using this VC in an extension (not really sure if that's a real use case though).

@vtourraine
Copy link
Owner

Right, so we’re on the same page. I think you should create a pull request for this. We’ll merge it to main, then see if that turns out problematic for some apps. I’d really prefer to avoid adding alternative implementations/methods just to dance around this issue.

@vtourraine
Copy link
Owner

@ifrins Well... another pull request was already submitted with the @available annotation (#85), so I’ve merged it. I’ve also mentioned you in the changelog as a co-author, I hope that’s OK.

Everyone can now test the Xcode 13 support with the xcode-13 branch. And I’ve opened a dedicated pull request (#86) to discuss any further changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants