Skip to content

[generator-macos] Use CocoaPods to build iOS and macOS targets #298

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 9 commits into from
Apr 14, 2020
Merged

[generator-macos] Use CocoaPods to build iOS and macOS targets #298

merged 9 commits into from
Apr 14, 2020

Conversation

alloy
Copy link
Member

@alloy alloy commented Apr 7, 2020 •
edited
Loading

Please select one of the following

  • I am removing an existing difference between facebook/react-native and microsoft/react-native 👍

  • I am cherry-picking a change from Facebook's react-native into microsoft/react-native 👍

  • I am making a fix / change for the macOS implementation of react-native

  • I am making a change required for Microsoft usage of react-native

  • Updates the template from upstream v0.62 so we’ll have less merge issues going forward.

  • Renames iOS target to also have a platform suffix. (E.g. HelloWorld-iOS)

  • Renames macOS product to remove the platform suffix. (E.g. HelloWorld.app)

  • Adds Podfile which is able to build both iOS and macOS target.

In the next PR I’ll work on auto-linking community modules.

~/tmp » npx react-native init TestFinalMacOS --version=0.60.0
[…]
✔ Downloading template
✔ Copying template
✔ Processing template
✔ Installing CocoaPods dependencies (this may take a few minutes)

  Run instructions for iOS:
    • cd /Users/eloy/tmp/TestFinalMacOS && npx react-native run-ios
    - or -
    • Open TestFinalMacOS/ios/TestFinalMacOS.xcworkspace in Xcode or run "xed -b ios"
    • Hit the Run button

  Run instructions for Android:
    • Have an Android emulator running (quickest way to get started), or a device connected.
    • cd /Users/eloy/tmp/TestFinalMacOS && npx react-native run-android

~/tmp » cd TestFinalMacOS

~/t/TestFinalMacOS » yarn add react-native-macos-init
[…]
✨  Done in 3.94s.

~/t/TestFinalMacOS » yarn react-native-macos-init --version=0.60.0-microsoft.79
[…]
new macos/Podfile
new macos/TestFinalMacOS-iOS/AppDelegate.h
new macos/TestFinalMacOS-iOS/AppDelegate.m
new macos/TestFinalMacOS-iOS/Base.lproj/LaunchScreen.xib
new macos/TestFinalMacOS-iOS/Images.xcassets/AppIcon.appiconset/Contents.json
new macos/TestFinalMacOS-iOS/Images.xcassets/Contents.json
new macos/TestFinalMacOS-iOS/Info.plist
new macos/TestFinalMacOS-iOS/main.m
new macos/TestFinalMacOS-macOS/AppDelegate.h
new macos/TestFinalMacOS-macOS/AppDelegate.m
new macos/TestFinalMacOS-macOS/Assets.xcassets/AppIcon.appiconset/Contents.json
new macos/TestFinalMacOS-macOS/Assets.xcassets/Contents.json
new macos/TestFinalMacOS-macOS/Base.lproj/Main.storyboard
new macos/TestFinalMacOS-macOS/TestFinalMacOS.entitlements
new macos/TestFinalMacOS-macOS/Info.plist
new macos/TestFinalMacOS-macOS/ViewController.h
new macos/TestFinalMacOS-macOS/ViewController.m
new macos/TestFinalMacOS-macOS/main.m
new macos/TestFinalMacOS.xcodeproj/project.pbxproj
new macos/TestFinalMacOS.xcodeproj/xcshareddata/xcschemes/TestFinalMacOS-iOS.xcscheme
new macos/TestFinalMacOS.xcodeproj/xcshareddata/xcschemes/TestFinalMacOS-macOS.xcscheme
new react-native.config.js
new metro.config.macos.js
To run your app on macOS:
   pushd macos && pod install && popd
   open macos/TestFinalMacOS.xcworkspace
   yarn start:macos
In Xcode switch to the TestFinalMacOS-macOS scheme then click Run.
✨  Done in 13.60s.

~/t/TestFinalMacOS » pushd macos && pod install && popd
[…]
Pod installation complete! There are 24 dependencies from the Podfile and 24 total pods installed.

~/t/TestFinalMacOS » open macos/TestFinalMacOS.xcworkspace

~/t/TestFinalMacOS » yarn start:macos
$ node node_modules/react-native-macos/local-cli/cli.js start --use-react-native-macos
[…]
 BUNDLE  [macos, dev] ./index.js ▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓ 100.0% (1/1), done.

 BUNDLE  [ios, dev] ./index.js ▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓ 100.0% (568/568), done.

Screenshot 2020-04-07 at 20 48 17

Microsoft Reviewers: Open in CodeFlow

@alloy alloy requested a review from tom-un April 7, 2020 18:54
@alloy alloy requested a review from acoates-ms as a code owner April 7, 2020 18:54
alloy
alloy commented Apr 7, 2020
View reviewed changes
alloy
alloy commented Apr 7, 2020
View reviewed changes
local-cli/generator-macos/templates/macos/HelloWorld-macOS/Info.plist
<key>NSSupportsAutomaticTermination</key>
<true/>
<key>NSSupportsSuddenTermination</key>
<true/>
Copy link
Member Author

Choose a reason for hiding this comment

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

These are the defaults for a macOS app nowadays 🤷‍♂

alloy
alloy commented Apr 7, 2020
View reviewed changes
alloy
alloy commented Apr 7, 2020
View reviewed changes
local-cli/generator-macos/templates/macos/Podfile
@@ -0,0 +1,40 @@
# require_relative '../node_modules/@react-native-community/cli-platform-ios/native_modules'
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the auto-linking that doesn’t work yet.

Copy link

Choose a reason for hiding this comment

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

How are we tracking getting it working?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is up next, I just wanted to make PRs for distinct units of work.

alloy
alloy commented Apr 7, 2020
View reviewed changes
local-cli/generator-macos/templates/macos/Podfile
# require_relative '../node_modules/@react-native-community/cli-platform-ios/native_modules'

abstract_target 'Shared' do
# use_native_modules!
Copy link
Member Author

Choose a reason for hiding this comment

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

Ditto.

alloy
alloy commented Apr 7, 2020
View reviewed changes
tom-un
tom-un reviewed Apr 8, 2020
View reviewed changes
local-cli/generator-macos/templates/macos/Podfile
# use_native_modules!

pod 'React', :path => "../node_modules/react-native-macos/"
pod 'React-Core', :path => "../node_modules/react-native-macos/React"
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think the React-Core file is missing some source files for macOS. When i run the init'd app I get runtime errors about StatusBar and other components. It looks like the React/Modules/RCTStatusBarManager.m needs to be added to React-Core-macOS as it is in React-Core-iOS. Its one of the things I had to fix in the static React.xcodeproj too for the initial PR for react-native-macos-init.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ya, I verified the fix locally. Just remove the line in React-Core.podspec that is excluding "Modules/RCTStatusBarManager.*" for osx.

There are probably a few more files that have to be un-excluded to re-align with the changes I made in the static XCodeproj. I'll do that in a separate PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah nice one 👍 I was going to ask about this one, but assumed it was currently a mobile only API. I’ll add this one in this PR and you can follow up with others.

Copy link
Member Author

Choose a reason for hiding this comment

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

As per the comment on the guarded imports, we should perhaps see if we can have a CI task that does a build [of e.g. RNTester] that only builds for iOS and then only for macOS.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Good idea. I was going to update the CI to build RNTester using pods. Its currently sliced by platform/flavor now but using the static xcodeproj's. @HeyImChris FYI

tom-un
tom-un reviewed Apr 8, 2020
View reviewed changes
HeyImChris
HeyImChris reviewed Apr 14, 2020
View reviewed changes
local-cli/generator-common/index.js
}

/**
* @todo Move this upstream to @react-native-community/cli
Copy link

Choose a reason for hiding this comment

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

How are we tracking this work?

Copy link
Member Author

Choose a reason for hiding this comment

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

I’ll be touching the CLI package for the next iteration of this when I make auto-linking work and will make this change then.

local-cli/generator-macos/templates/macos/Podfile
@@ -0,0 +1,40 @@
# require_relative '../node_modules/@react-native-community/cli-platform-ios/native_modules'
Copy link

Choose a reason for hiding this comment

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

How are we tracking getting it working?

local-cli/generator-macos/templates/macos/Podfile
end

target 'HelloWorld-iOS' do
platform :ios, '9'
Copy link

Choose a reason for hiding this comment

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

We try and do currentVersion-1 for iOS, so probably worth making this 12 unless that'd break with RN still being on 9 somehow.

Also why is there an iOS target in the macos directory/Podfile?

Copy link
Member Author

Choose a reason for hiding this comment

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

We try and do currentVersion-1 for iOS, so probably worth making this 12 unless that'd break with RN still being on 9 somehow.

Nothing would break, just trying to keep this in sync with upstream.

Also why is there an iOS target in the macos directory/Podfile?

Great question and one I asked myself too! 😄

@tom-un Explained to me that he likes to have one in the macos dir for now as it can target the iOS bits of our RN fork to quickly test if we’ve broken code as opposed to the code in the ios dir that targets upstream RN. We did briefly discuss to perhaps hide this extra iOS target by default in the future, though.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ya, and I think it'll be easier to implement a --add-ios-target flag once this PR lands. Because it could be implemented as Ruby script. Right now I think we'd have to have two project.pbxproj templates: one with only a mac target and another with mac and ios targets and that would be a pain to maintain.

@alloy alloy force-pushed the alloy/cocoapods-template branch 2 times, most recently from 002f7b8 to 3ac07b9 Compare April 14, 2020 18:46
@alloy alloy force-pushed the alloy/cocoapods-template branch from 3ac07b9 to 53ea1c5 Compare April 14, 2020 19:08
@alloy alloy force-pushed the alloy/cocoapods-template branch from 15c2fb4 to 880787a Compare April 14, 2020 20:17
@alloy alloy merged commit 4bbea7c into master Apr 14, 2020
@alloy alloy deleted the alloy/cocoapods-template branch April 14, 2020 22:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tom-un tom-un tom-un approved these changes

@acoates-ms acoates-ms Awaiting requested review from acoates-ms

+1 more reviewer

@HeyImChris HeyImChris HeyImChris left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@alloy @tom-un @HeyImChris