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

Specify use_frameworks within OneSignalNotificationServiceExtension target in iOS Podfile #128

Merged
merged 1 commit into from
Oct 4, 2022

Conversation

brismithers
Copy link
Contributor

@brismithers brismithers commented Oct 3, 2022

Description

One Line Summary

Within OneSignalNotificationServiceExtension target, add use_frameworks line conditional on ios.useFrameworks property existing

Details

The BuildProperties Plugin gives an useFrameworks option for iOS which enables use_frameworks in the iOS Podfile application target. When specified in the expo app, the OneSignal expo plugin creates a OneSignalNotificationServiceExtension target in the Podfile that does not specify use_frameworks in the target. The build will fail with this inconsistency:

<main_app> (true) and OneSignalNotificationServiceExtension (false) do not both set use_frameworks!.

This change updates the OneSignal Expo Plugin to specify use_frameworks in the OneSignalNotificationServiceExtension target when also specified in the app's target. The line was pulled from the standard use_frameworks line that is used on the application target. It relies on a property that may or may not exist within the Podfile.properties.json file.

Motivation

Fixes #127

Scope

There is no runtime changes expected, this change is limited to the building of the iOS app generated by Expo.

Testing

Manual testing

After the change was made, a sample application with the following variations were confirmed to build and start up on iOS:

  • No BuildProperties plugin.
  • BuildProperties plugin with the useFrameworks option not specified.
  • BuildProperties plugin with the useFrameworks option specified.

As this is iOS build specific, android build/run was not evaluated.

Checklist

Overview

  • I have filled out all REQUIRED sections above
  • PR does one thing
    • If it is hard to explain how any codes changes are related to each other then it most likely needs to be more than one PR
  • Any Public API changes are explained in the PR details and conform to existing APIs

Testing

  • I have personally tested this on my device, or explained why that is not possible
  • I have tested this on the latest version of the plugin
  • I have tested this on both Android and iOS, or explained why that is not possible

Final pass

  • Code is as readable as possible.
    • Simplify with less code, followed by splitting up code into well named functions and variables, followed by adding comments to the code.
  • I have reviewed this PR myself, ensuring it meets each checklist item
    • WIP (Work In Progress) is ok, but explain what is still in progress and what you would like feedback on. Start the PR title with "WIP" to indicate this.

…works` line conditional on `ios.useFrameworks` property existing
@brismithers brismithers requested review from rgomezp and emawby October 3, 2022 18:04
@brismithers brismithers merged commit 5d711bf into main Oct 4, 2022
@brismithers brismithers deleted the use-frameworks-in-extension branch October 4, 2022 12:57
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.

[Bug]: ios useFrameworks is false
3 participants