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

feat(ios): use @objc for bridge and auxiliary APIs #4

Merged

Conversation

NathanWalker
Copy link
Contributor

This allows Capacitor plugins to be registered with the bridge, for example:
NativeScript/plugins@644b896#diff-8ccbf3f8847b6a1b15944609e0e0610409a8b5e910076d7298aeab62cd998e84R46

@Steven0351 Steven0351 merged commit 186fb97 into ionic-team:main Mar 15, 2022
@Steven0351
Copy link
Member

LGTM, thanks for the contribution!

@Steven0351
Copy link
Member

@NathanWalker I looked a little closer at your code example there, the plugin(withName:) method on the bridge does not register the plugin. Currently all plugins on iOS are registered on initialization without the developer needing to do anything.

@NathanWalker
Copy link
Contributor Author

Oh nice lemme try the latest version and will see if we can get that to work - previously at least on 0.4.1 the only way we could make the plugins active was to use plugin(withName:)

@Steven0351
Copy link
Member

The actual registration actually happens down in Capacitor, so that's interesting that the plugin wouldn't load for you before. I know Android requires explicitly registering them, but in our e-commerce demo one of our Portals (the profile page specifically) uses the CapacitorCamera plugin, which is never explicitly registered anywhere in the iOS application.

Also, on a side note, we had an issue with how we were cutting releases for a specific branch of Capacitor we use and you may have some breakage. Unfortunately, you'll have to update to 0.5.1 as it's the only version of Portals that will work. Specifically the frame parameter in the PortalWebView initializer is no longer needed.

@NathanWalker
Copy link
Contributor Author

Ok excellent thanks for this info - will take me few days to confirm things but will post back here once I know more (likely early next week).

@NathanWalker
Copy link
Contributor Author

NathanWalker commented Mar 17, 2022

@Steven0351 I've been having trouble with 0.5.1 due to IonicLiveUpdates? Even when manually specifying a deploy target of 14 or even 15 it continues to complain?
Do you know if there's a current issue related to this (I could not find anywhere but seems odd as I've tried a number of things but continues to complain about IonicLiveUpdates or ZIPFoundation.
Screen Shot 2022-03-16 at 9 05 15 PM
Screen Shot 2022-03-16 at 9 05 40 PM

@Steven0351
Copy link
Member

I’ll take a look at this tomorrow, as far as I’m aware IonicLiveUpdates was built with swift module evolution support. We had some issues like that with Carthage though. Is this reproducible with the NativeScript plug-in?

@NathanWalker
Copy link
Contributor Author

NathanWalker commented Mar 17, 2022

We haven't published the update yet but working on getting 0.5.1 operational in this branch:
https://github.com/NativeScript/plugins/tree/feat/updates

Explored several things like setting BUILD_LIBRARY_FOR_DISTRIBUTION = 'YES' as discussed in some other posts as well as overriding some build settings in the Podfile to no avail thus far (still exploring).

If you ended up trying it, could try these exact steps (would require npm i -g nativescript for the cli):

$ git clone https://github.com/NativeScript/plugins.git
$ cd plugins
$ npm run setup
$ npm start
   // type 'focus.ionic', ENTER
   // this will focus the workspace to only the ionic portals plugin enabled (to avoid noise from other plugins)

$ npm start
   // type 'demo.ios', ENTER
   // this would start the ios demo which will initiate a build which should result in same error

If helpful, you can open the demo app in Xcode via:

open apps/demo/platforms/ios/demo.xcworkspace

Some relevant files to see some settings in place:

@Steven0351
Copy link
Member

Steven0351 commented Mar 17, 2022

I was able to reproduce with the NativeScript demo. I was also able to get it to compile by making IonicLiveUpdates a local podspec reference and building from source, so there's definitely something going on with the compiled binary SDK. What makes me curious (and concerned) is not having seen this same issue pop up with our demo application. We also didn't have any issues with that when doing some stuff with React Native.

@Steven0351
Copy link
Member

If you get a chance, would you try compiling the iOS project of our e-commerce demo and let me know if it builds fine for you? The thing that's boggling my mind the most is that contrary to the build error, ZipFoundation's minimum iOS target is actually 12.0. In fact, the only library in the entire dependency tree that has a 14.0 minimum target is IonicPortals itself.

@NathanWalker
Copy link
Contributor Author

NathanWalker commented Mar 18, 2022

ECommerce demo built fine - curious indeed; I'll continue to try few things and I'm sure can figure out what's going on there.
Screen Shot 2022-03-17 at 10 07 26 PM

@NathanWalker
Copy link
Contributor Author

@Steven0351 just quick update - I got above to build; was a combination of ensuring the 'IonicPortals' pod had declared at least platform :ios, '14.0' and the more important detail is that the build settings (or rather build.xcconfig) specified IPHONEOS_DEPLOYMENT_TARGET = 12.0 specifically.

@NathanWalker
Copy link
Contributor Author

Still working through the API changes and will update more here as I know this week if the changes in this PR are indeed still needed.

@Steven0351
Copy link
Member

I think the changes are fine to stay regardless, exposing those pieces to objective-c isn't really much of a maintenance burden.

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

3 participants