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

Update iOS generation to write xcode project via expo mod rather than directly to file system #175

Merged
merged 1 commit into from
Apr 13, 2023

Conversation

brismithers
Copy link
Contributor

@brismithers brismithers commented Mar 31, 2023

Description

One Line Summary

Update iOS generation to write xcode project via expo mod rather than directly to file system

Details

This change relies on the xcode project parsed and provided to the plugin via withXcodeProject (via modResults) rather than the plugin loading and saving the xcode project itself. Changing the xcode project file within the plugin is not consistent with how expo plugins are expected to function. This is most visible when there are other plugins that also make changes to the xcode project file.

This change means we no longer have a direct dependency on the xcode package, though it typically is still used as an indirect dependency via expo.

Additionally, changing of the pod file and the writing of the NSE were also wrapped within calls to withDangerousMod to make clear these steps are writing files directly, rather than modifying the configuration.

Motivation

Fixes #166

Scope

This change affects how the xcode project file changes are made, establishing intent how the podfile is updated, and establishing intent on the NSE files being created. No runtime changes are in scope.

Testing

Manual testing

Ran expo prebuild for projects and manually inspected ios output folder. Verified all modifications are made. Also tested the specific scenario raised by #166, and ensured proper output when there are multiple plugins modifying the xcode project file.

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.

@emawby
Copy link
Contributor

emawby commented Apr 11, 2023

@brismithers Hey is this ready to be merged?

@RL-Jurica-Penjgusic
Copy link

@rgomezp Can this be approved so we can try it?
It seems this will cover couple issues.

Tnx

@emawby emawby merged commit 199c91c into main Apr 13, 2023
@emawby emawby deleted the use-expo-provided-xcodeproject branch April 13, 2023 17:11
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]: Second extension fails to build – Could not find target
4 participants