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

App Extension support #1895

Closed
wants to merge 9 commits into from
Closed

App Extension support #1895

wants to merge 9 commits into from

Conversation

yusefnapora
Copy link
Contributor

This adds workarounds for the code that was preventing React from compiling when linked against an iOS App Extension target.

Some iOS APIs are unavailable to App Extensions, and Xcode's static analysis will catch attempts to use methods that have been flagged as unavailable.

React currently uses two APIs that are off limits to extensions: [UIApplication sharedApplication] and [UIAlertView initWith ...].

This commit adds a helper function to RCTUtils.[hm] called RCTRunningInAppExtension(), which returns YES if, at runtime, it can be determined that we're running in an app extension (by checking whether the path to [NSBundle mainBundle] has the "appex" path extension).

It also adds a RCTSharedApplication() function, which will return nil if running in an App Extension. If running in an App, RCTSharedApplication() calls sharedApplication by calling performSelector: on the UIApplication class. This passes the static analysis check, and, in my opinion, obeys the "spirit of the law" by only calling sharedApplication after we've determined that it's kosher to do so.

Existing calls to [UIApplication sharedApplication] have been replaced with calls to RCTSharedApplication(), with nil checks where appropriate.

The absence of UIAlertView is similarly handled with a runtime workaround. If we're running in an App, the RCTAlertView(NSString *title, NSString *message, id delegate, NSString *cancelButtonTitle, NSArray *otherButtonTitles) function uses NSInvocation to call the extension-unavailable initWithTitle:message:delegate:cancelButtonTitle:otherButtonTitles: selector. In an Extension it just logs an error and returns nil.

As a result of these changes, React can be linked into a shared library that is then linked into both an App and an App Extension. When running in the App, all existing functionality will continue to work as before. When running in the Extension, AlertIOS will be unavailable, as will APIs that depend on access to [UIApplication sharedApplication].

APIs that are unavailable to extensions are:

  • RCTActionSheetManager - uses sharedApplication to access the window of the app delegate
  • RCTImagePickerManager - uses sharedApplication to get the keyWindow property
  • RCTLinkingManager - uses sharedApplication to open URLs
  • RCTPushNotificationManager - uses sharedApplication to badge the app icon and register for permissions
  • RCTDevMenu - uses sharedApplication to get the keyWindow property
  • RCTPerfStats - uses sharedApplication to access the window of the app delegate

RCTAppState has been updated to return "extension" if running in an app extension.

This PR doesn't attempt to expose the NSExtensionContext to javascript, although a native module could be written to do so.

UIExplorer integration and unit tests all pass. I have a basic proof of concept app, but it needs some cleanup; I'll push it to a repo soonish

@facebook-github-bot facebook-github-bot added GH Review: review-needed CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. labels Jul 7, 2015
@nicklockwood
Copy link
Contributor

OK, if you remove the extra error, this looks good to me.

@yusefnapora
Copy link
Contributor Author

@nicklockwood last commit removes the error. Thanks for reviewing this!

@brentvatne
Copy link
Collaborator

Lovely, nice work @yusefnapora

@yusefnapora
Copy link
Contributor Author

@brentvatne thanks!

@brentvatne
Copy link
Collaborator

@yusefnapora - can you rebase this please so it can be a clean merge? Thanks!

@yusefnapora
Copy link
Contributor Author

@brentvatne I just force-pushed a rebase onto this existing branch; hope that's alright. I encountered a small conflict in RCTPushNotificationManger.m, but it was straightforward. Integration tests still pass. Don't have time to poke at it more tonight, but if there's anything else you need let me know and I'll circle back tomorrow :)

@brentvatne
Copy link
Collaborator

Great, ping me when you've had a chance to circle back and verify, then I will check it out and update the tag @yusefnapora!

@yusefnapora
Copy link
Contributor Author

@brentvatne last commit fixes up a couple new invocations of [UIApplication sharedApplication] that I missed last night.

I added a brain-dead example project here: https://github.com/yusefnapora/ReactExtensionDemo/ which is pegged to the last commit and works as expected

@brentvatne
Copy link
Collaborator

cc @nicklockwood - this looks ready for review and is rebased against master so it's sync ready

@nicklockwood
Copy link
Contributor

@yusefnapora I notice you've not done anything to disable UIActionSheet - are action sheets available in extensions?

@yusefnapora
Copy link
Contributor Author

@nicklockwood I'm glad you brought that up, since I hadn't really thought it through. I looked in the headers, and the designated initializer is marked as unavailable:

- (instancetype)initWithTitle:(NSString *)title delegate:(id<UIActionSheetDelegate>)delegate cancelButtonTitle:(NSString *)cancelButtonTitle destructiveButtonTitle:(NSString *)destructiveButtonTitle otherButtonTitles:(NSString *)otherButtonTitles, ...  NS_REQUIRES_NIL_TERMINATION NS_EXTENSION_UNAVAILABLE_IOS("Use UIAlertController instead.");

It didn't trigger the static analysis because RCTActionSheetManager calls [[UIActionSheet alloc] init] instead. But that means we should bail out and hit the failure callback in showActionSheetWithOptions..., since I assume trying to use it from an extension will trigger a runtime error.

I'll do that as soon as I've got time, probably tonight.

@yusefnapora
Copy link
Contributor Author

Although, the fact that the call to alloc/init slipped by the static analysis makes me think that we might not need to use NSInvocation for creating a UIAlertView. We could just use alloc/init and call the setters, like RCTActionSheetManager is doing now.

I'm not sure, but the fact that the call to init is allowed makes me think that Apple is just blacklisting selectors. Since init is everywhere they can't outlaw that, but there's nothing else using the initWithAMillionSpecificParameters selectors.

@yusefnapora
Copy link
Contributor Author

Hey @nicklockwood, this took me a little longer to get to than I thought, sorry about that. I added a check to RCTActionSheetManager to bail out if running in an extension, and also changed RCTAlertView() to use [[UIAlertView alloc] init] which does pass the static analysis check. That seems cleaner than using NSInvocation.

Also rebased onto latest master.

Let me know if anything else is blocking this, and I'll try to get to it soon. Thanks :)

@brentvatne
Copy link
Collaborator

@yusefnapora - sorry for the delay, could you rebase this again so it would be a clean merge?

@idibidiart
Copy link

What is the status of App Extensions support for React Native? @brentvatne

@yusefnapora can you rebase as requested? :) excited to try!

This adds workarounds for the code that was preventing React from compiling when linked against an iOS App Extension target.

Some iOS APIs are unavailable to App Extensions, and Xcode's static analysis will catch attempts to use methods that have been
flagged as unavailable.

React currently uses two APIs that are off limits to extensions: `[UIApplication sharedApplication]` and `UIAlertView`.

This commit adds a helper function to `RCTUtils.[hm]` called `RCTRunningInAppExtension()`, which returns `YES` if, at runtime,
it can be determined that we're running in an app extension (by checking whether the path to `[NSBundle mainBundle]` has the
`"appex"` path extension).

It also adds a `RCTSharedApplication()` function, which will return `nil` if running in an App Extension.  If running in an App,
`RCTSharedApplication()` calls `sharedApplication` by calling `performSelector:` on the `UIApplication` class.  This passes
the static analysis check, and, in my opinion, obeys the "spirit of the law" by only calling `sharedApplication` after we've
determined that it's kosher to do so.

Existing calls to `[UIApplication sharedApplication]` have been replaced with calls to `RCTSharedApplication()`, with `nil` checks
where appropriate.

The absence of `UIAlertView` is similarly handled with a runtime workaround.  If we're running in an App, the
 `RCTAlertView(NSString *title, NSString *message, id delegate, NSString *cancelButtonTitle, NSArray *otherButtonTitles)`
function uses `NSInvocation` to call the extension-unavailable `initWithTitle:message:delegate:cancelButtonTitle:otherButtonTitles:`
selector.  In an Extension it just logs an error and returns `nil`.

As a result of these changes, React can be linked into a shared library that is then linked into both an App and an App Extension.
When running in the App, all existing functionality will continue to work as before.  When running in the Extension, `AlertIOS` will
be unavailable, as will APIs that depend on access to `[UIApplication sharedApplication]`.

APIs that are unavailable to extensions are:
`RCTActionSheetManager` - uses `sharedApplication` to access the `window` of the app delegate
`RCTImagePickerManager` - uses `sharedApplication` to get the `keyWindow` property
`RCTLinkingManager` - uses `sharedApplication` to open URLs
`RCTPushNotificationManager` - uses `sharedApplication` to badge the app icon and register for permissions
`RCTDevMenu` - uses `sharedApplication` to get the `keyWindow` property
`RCTPerfStats` - uses `sharedApplication` to access the `window` of the app delegate
`RCTAlertView()` already logs an error if running in an extension, so this isn't needed.
UIActionSheet is off limits, so exit early if we're running in an app extension.  The menu wouldn't show anyway, since we can't get the app delegate's keyWindow property.  But this prevents us from trying to use UIActionSheet at all, so is probably safer.
@yusefnapora
Copy link
Contributor Author

@brentvatne: I just rebased this & ran the UIExplorer integration tests. Don't have much time today, but hopefully soon I can put up a decent example of using react in an extension.

@idibidiart: right now the support is pretty basic; you can link react into an extension and use it to render (so long as you create your own RCTRootView), but you're missing out on the Dev Menu, redbox, etc.

This PR also doesn't do anything to expose extension inputs to javascript, although I've got a proof-of-concept native module that can load images shared from the camera roll, via the extension. Needs cleanup though & I don't know when I'll have time to work on it.


[app registerUserNotificationSettings:notificationSettings];
[app registerForRemoteNotifications];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: can you remove the whitespace here?

delegate:self
cancelButtonTitle:nil
otherButtonTitles:nil];

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: whitespace

@javache
Copy link
Member

javache commented Sep 21, 2015

Looks good to me. I left some small remarks, if you could address those, I'll merge this in master.

@javache
Copy link
Member

javache commented Sep 22, 2015

@facebook-github-bot shipit

@facebook-github-bot
Copy link
Contributor

Thanks for importing. If you are an FB employee go to https://our.intern.facebook.com/intern/opensource/github/pull_request/1626368660962669/int_phab to review.

@ghost ghost closed this in 2f9bd1f Sep 22, 2015
MattFoley pushed a commit to skillz/react-native that referenced this pull request Nov 9, 2015
Summary: This adds workarounds for the code that was preventing React from compiling when linked against an iOS App Extension target.

Some iOS APIs are unavailable to App Extensions, and Xcode's static analysis will catch attempts to use methods that have been flagged as unavailable.

React currently uses two APIs that are off limits to extensions: `[UIApplication sharedApplication]` and `[UIAlertView initWith ...]`.

This commit adds a helper function to `RCTUtils.[hm]` called `RCTRunningInAppExtension()`, which returns `YES` if, at runtime, it can be determined that we're running in an app extension (by checking whether the path to `[NSBundle mainBundle]` has the `"appex"` path extension).

It also adds a `RCTSharedApplication()` function, which will return `nil` if running in an App Extension. If running in an App, `RCTSharedApplication()` calls `sharedApplication` by calling `performSelector:` on the `UIApplication` class.  This passes the static analysis check, and, in my opinion, obeys the "spirit of th
Closes facebook#1895

Reviewed By: @​svcscm

Differential Revision: D2224128

Pulled By: @nicklockwood
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants