-
Notifications
You must be signed in to change notification settings - Fork 935
feat(cli-platform-ios): add macOS auto-linking support #1126
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
Changes from all commits
515671a
58802c3
c595ef5
81fb8ef
9ef86dd
1a411b3
430c4ba
f570baf
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,6 +3,7 @@ import { | |
| IOSProjectParams, | ||
| IOSDependencyConfig, | ||
| IOSDependencyParams, | ||
| IOSNativeModulesConfig, | ||
| } from './ios'; | ||
| import { | ||
| AndroidProjectConfig, | ||
|
|
@@ -127,7 +128,7 @@ export type ProjectConfig = { | |
| * @property platforms - Map of available platforms (build-ins and dynamically loaded) | ||
| * @property commands - An array of commands that are present in 3rd party packages | ||
| */ | ||
| export type Config = { | ||
| export interface Config extends IOSNativeModulesConfig { | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is done purely to ensure the |
||
| root: string; | ||
| reactNativePath: string; | ||
| project: ProjectConfig; | ||
|
|
@@ -149,7 +150,7 @@ export type Config = { | |
| [name: string]: PlatformConfig<any, any, any, any>; | ||
| }; | ||
| commands: Command[]; | ||
| }; | ||
| } | ||
|
|
||
| /** | ||
| * Shares some structure with Config, except that root is calculated and can't | ||
|
|
@@ -180,6 +181,7 @@ export { | |
| IOSProjectParams, | ||
| IOSDependencyConfig, | ||
| IOSDependencyParams, | ||
| IOSNativeModulesConfig, | ||
| }; | ||
|
|
||
| export { | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -6,6 +6,13 @@ | |||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||
| export interface IOSProjectParams { | ||||||||||||||||||||||||||||||||||||||||||||||||
| project?: string; | ||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||
| * @deprecated A podspec should always be at the root of a package and | ||||||||||||||||||||||||||||||||||||||||||||||||
| * have the name of the package. This property will be | ||||||||||||||||||||||||||||||||||||||||||||||||
| * removed in a future major version. | ||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||
| * @todo Log a warning when this is used. | ||||||||||||||||||||||||||||||||||||||||||||||||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Have to follow-up on this in a future iteration.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We'll need to identify libraries that follow a pattern of adding a Podspec into |
||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||
| podspecPath?: string; | ||||||||||||||||||||||||||||||||||||||||||||||||
| sharedLibraries?: string[]; | ||||||||||||||||||||||||||||||||||||||||||||||||
| libraryFolder?: string; | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -31,3 +38,55 @@ export interface IOSProjectConfig { | |||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| export interface IOSDependencyConfig extends IOSProjectConfig {} | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||
| * @see https://www.rubydoc.info/gems/cocoapods-core/Pod/Podfile/DSL#script_phase-instance_method | ||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||
| * The only difference is that `script` may be omitted in favour of a | ||||||||||||||||||||||||||||||||||||||||||||||||
| * `path`, relative to the root of the package, whose content will be | ||||||||||||||||||||||||||||||||||||||||||||||||
| * used. | ||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||
| export type IOSScriptPhase = ({script: string} | {path: string}) & { | ||||||||||||||||||||||||||||||||||||||||||||||||
| name: string; | ||||||||||||||||||||||||||||||||||||||||||||||||
| shell_path?: string; | ||||||||||||||||||||||||||||||||||||||||||||||||
| input_files?: string[]; | ||||||||||||||||||||||||||||||||||||||||||||||||
| output_files?: string[]; | ||||||||||||||||||||||||||||||||||||||||||||||||
| input_file_lists?: string[]; | ||||||||||||||||||||||||||||||||||||||||||||||||
| output_file_lists?: string[]; | ||||||||||||||||||||||||||||||||||||||||||||||||
| show_env_vars_in_log?: boolean; | ||||||||||||||||||||||||||||||||||||||||||||||||
| dependency_file?: string; | ||||||||||||||||||||||||||||||||||||||||||||||||
| execution_position?: 'before_compile' | 'after_compile' | 'any'; | ||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||
| * This describes the data that is expected by `native_modules.rb`. It is only | ||||||||||||||||||||||||||||||||||||||||||||||||
| * meant to ensure the `Config` interface follows exactly what is needed, so | ||||||||||||||||||||||||||||||||||||||||||||||||
| * only make changes to this interface (or `IOSScriptPhase`) if the data | ||||||||||||||||||||||||||||||||||||||||||||||||
| * requirements of `native_modules.rb` change. | ||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||
| export interface IOSNativeModulesConfig { | ||||||||||||||||||||||||||||||||||||||||||||||||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ❗ So this should only change if the Ruby source changes and should not be refactored/shared with other typings, even if they are similar.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is that? Wouldn't it be better to just reference cli/packages/cli-types/src/index.ts Lines 131 to 153 in f570baf
Could you help me understand where and how it is used? Maybe it's not the right place for it then (in shared package), but next to a utility?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because TS cannot type-check the Ruby code, so people making changes to this |
||||||||||||||||||||||||||||||||||||||||||||||||
| project: { | ||||||||||||||||||||||||||||||||||||||||||||||||
| ios?: { | ||||||||||||||||||||||||||||||||||||||||||||||||
| sourceDir: string; | ||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||
| dependencies: { | ||||||||||||||||||||||||||||||||||||||||||||||||
| [name: string]: { | ||||||||||||||||||||||||||||||||||||||||||||||||
| root: string; | ||||||||||||||||||||||||||||||||||||||||||||||||
| platforms: { | ||||||||||||||||||||||||||||||||||||||||||||||||
| ios?: null | { | ||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||
| * @deprecated A podspec should always be at the root of a package and | ||||||||||||||||||||||||||||||||||||||||||||||||
| * have the name of the package. This property will be | ||||||||||||||||||||||||||||||||||||||||||||||||
| * removed in a future major version. | ||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||
| * @todo Log a warning when this is used. | ||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||
| podspecPath: string; | ||||||||||||||||||||||||||||||||||||||||||||||||
| scriptPhases?: Array<IOSScriptPhase>; | ||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||
| android?: null | {}; | ||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,8 +1,14 @@ | ||
| # This is a function which is used inside your Podfile. | ||
| # It uses `react-native config` to grab a list of dependencies, and pulls out.all of the ones | ||
| # which declare themselves to be iOS dependencies (via having a Podspec) and automatically | ||
| # imports those into your current target. | ||
| # | ||
| # It uses `react-native config` to grab a list of dependencies, and pulls out | ||
| # all of the ones which declare themselves to be iOS/macOS dependencies (by | ||
| # virtue of having a Podspec) and automatically imports those into your current | ||
| # target. | ||
| # | ||
| # See the `IOSNativeModulesConfig` interface in `cli-types/src/ios.ts` to | ||
| # understand what the input data should look like. Be sure to update that file | ||
| # in lock-step with additional data being used here. | ||
|
|
||
| require 'pathname' | ||
| require 'cocoapods' | ||
|
|
||
|
|
@@ -54,6 +60,16 @@ def use_native_modules!(config = nil) | |
|
|
||
| spec = Pod::Specification.from_file(podspec_path) | ||
|
|
||
| # Skip pods that do not support the platform of the current target. | ||
| if platform = current_target_definition.platform | ||
| next unless spec.supported_on_platform?(platform.name) | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is the actual semantic change of the PR, so tiny 😂
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for that comment! And also, I am impressed by the CocoaPods API :D |
||
| else | ||
| # TODO: In a future RN version we should update the Podfile template and | ||
| # enable this assertion. | ||
| # | ||
| # raise Pod::Informative, "Cannot invoke `use_native_modules!` before defining the supported `platform`" | ||
| end | ||
|
|
||
| # We want to do a look up inside the current CocoaPods target | ||
| # to see if it's already included, this: | ||
| # 1. Gives you the chance to define it beforehand | ||
|
|
@@ -101,169 +117,39 @@ def use_native_modules!(config = nil) | |
|
|
||
| if found_pods.size > 0 | ||
| pods = found_pods.map { |p| p.name }.sort.to_sentence | ||
| Pod::UI.puts "Detected React Native module #{"pod".pluralize(found_pods.size)} for #{pods}" | ||
| Pod::UI.puts "Auto-linking React Native #{"module".pluralize(found_pods.size)} for target `#{current_target_definition.name}`: #{pods}" | ||
| end | ||
|
|
||
| config | ||
| end | ||
|
|
||
| # You can run the tests for this file by running: | ||
| # $ ruby packages/platform-ios/native_modules.rb | ||
| # $ yarn jest packages/platform-ios/src/config/__tests__/native_modules.test.ts | ||
| if $0 == __FILE__ | ||
| require "minitest/spec" | ||
| require "minitest/autorun" | ||
|
|
||
| describe "use_native_modules!" do | ||
| before do | ||
| @script_phase = { | ||
| "script" => "123", | ||
| "name" => "My Name", | ||
| "execution_position" => "before_compile", | ||
| "input" => "string" | ||
| } | ||
| @ios_package = ios_package = { | ||
| "root" => "/root/app/node_modules/react", | ||
| "platforms" => { | ||
| "ios" => { | ||
| "podspecPath" => "/root/app/node_modules/react/React.podspec", | ||
| }, | ||
| "android" => nil, | ||
| }, | ||
| } | ||
| @android_package = { | ||
| "root" => "/root/app/node_modules/react-native-google-play-game-services", | ||
| "platforms" => { | ||
| "ios" => nil, | ||
| "android" => { | ||
| # This is where normally more config would be | ||
| }, | ||
| } | ||
| } | ||
| @project = { | ||
| "ios" => { | ||
| "sourceDir" => "/root/app/ios" | ||
| } | ||
| } | ||
| @config = { | ||
| "project" => @project, | ||
| "dependencies" => { | ||
| "ios-dep" => @ios_package, | ||
| "android-dep" => @android_package | ||
| } | ||
| } | ||
|
|
||
| @activated_pods = activated_pods = [] | ||
| @current_target_definition_dependencies = current_target_definition_dependencies = [] | ||
| @printed_messages = printed_messages = [] | ||
| @added_scripts = added_scripts = [] | ||
| @target_definition = target_definition = Object.new | ||
| @podfile = podfile = Object.new | ||
| @spec = spec = Object.new | ||
|
|
||
| spec.singleton_class.send(:define_method, :name) { "ios-dep" } | ||
|
|
||
| podfile.singleton_class.send(:define_method, :use_native_modules) do |config| | ||
| use_native_modules!(config) | ||
| end | ||
| require "json" | ||
| runInput = JSON.parse(ARGF.read) | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The Ruby script receives a JSON payload via |
||
|
|
||
| Pod::Specification.singleton_class.send(:define_method, :from_file) do |podspec_path| | ||
| podspec_path.must_equal ios_package["platforms"]["ios"]["podspecPath"] | ||
| spec | ||
| end | ||
|
|
||
| Pod::UI.singleton_class.send(:define_method, :puts) do |message| | ||
| printed_messages << message | ||
| end | ||
|
|
||
| podfile.singleton_class.send(:define_method, :pod) do |name, options| | ||
| activated_pods << { name: name, options: options } | ||
| end | ||
|
|
||
| podfile.singleton_class.send(:define_method, :script_phase) do |options| | ||
| added_scripts << options | ||
| end | ||
|
|
||
| target_definition.singleton_class.send(:define_method, :dependencies) do | ||
| current_target_definition_dependencies | ||
| end | ||
|
|
||
| target_definition.singleton_class.send(:define_method, :abstract?) do | ||
| false | ||
| end | ||
| unless runInput["captureStdout"] | ||
| Pod::Config.instance.silent = true | ||
| end | ||
|
|
||
| podfile.singleton_class.send(:define_method, :current_target_definition) do | ||
| target_definition | ||
| podfile = Pod::Podfile.new do | ||
| if runInput["podsActivatedByUser"] | ||
| runInput["podsActivatedByUser"].each do |name| | ||
| pod(name) | ||
| end | ||
| end | ||
|
|
||
| it "activates iOS pods" do | ||
| @podfile.use_native_modules(@config) | ||
| @activated_pods.must_equal [{ | ||
| name: "ios-dep", | ||
| options: { path: "../node_modules/react" } | ||
| }] | ||
| target 'iOS Target' do | ||
| platform :ios | ||
| use_native_modules!(runInput["dependencyConfig"]) | ||
| end | ||
|
|
||
| it "does not activate pods that were already activated previously (by the user in their Podfile)" do | ||
| activated_pod = Object.new | ||
| activated_pod.singleton_class.send(:define_method, :name) { "ios-dep" } | ||
| @current_target_definition_dependencies << activated_pod | ||
| @podfile.use_native_modules(@config) | ||
| @activated_pods.must_equal [] | ||
| target 'macOS Target' do | ||
| platform :osx | ||
| use_native_modules!(runInput["dependencyConfig"]) | ||
| end | ||
| end | ||
|
|
||
| it "does not activate pods whose root spec were already activated previously (by the user in their Podfile)" do | ||
| activated_pod = Object.new | ||
| activated_pod.singleton_class.send(:define_method, :name) { "ios-dep/foo/bar" } | ||
| @current_target_definition_dependencies << activated_pod | ||
| @podfile.use_native_modules(@config) | ||
| @activated_pods.must_equal [] | ||
| end | ||
|
|
||
| it "prints out the native module pods that were found" do | ||
| @podfile.use_native_modules({ "project" => @project, "dependencies" => {} }) | ||
| @podfile.use_native_modules({ "project" => @project, "dependencies" => { "pkg-1" => @ios_package }}) | ||
| @podfile.use_native_modules({ | ||
| "project" => @project, "dependencies" => { "pkg-1" => @ios_package, "pkg-2" => @ios_package } | ||
| }) | ||
| @printed_messages.must_equal [ | ||
| "Detected React Native module pod for ios-dep", | ||
| "Detected React Native module pods for ios-dep and ios-dep" | ||
| ] | ||
| end | ||
|
|
||
| describe "concerning script_phases" do | ||
| it "uses the options directly" do | ||
| @config["dependencies"]["ios-dep"]["platforms"]["ios"]["scriptPhases"] = [@script_phase] | ||
| @podfile.use_native_modules(@config) | ||
| @added_scripts.must_equal [{ | ||
| :script => "123", | ||
| :name => "My Name", | ||
| :execution_position => :before_compile, | ||
| :input => "string" | ||
| }] | ||
| end | ||
|
|
||
| it "reads a script file relative to the package root" do | ||
| @script_phase.delete("script") | ||
| @script_phase["path"] = "./some_shell_script.sh" | ||
| @config["dependencies"]["ios-dep"]["platforms"]["ios"]["scriptPhases"] = [@script_phase] | ||
|
|
||
| file_read_mock = MiniTest::Mock.new | ||
| file_read_mock.expect(:call, "contents from file", [File.join(@ios_package["root"], "some_shell_script.sh")]) | ||
|
|
||
| File.stub(:read, file_read_mock) do | ||
| @podfile.use_native_modules(@config) | ||
| end | ||
|
|
||
| @added_scripts.must_equal [{ | ||
| :script => "contents from file", | ||
| :name => "My Name", | ||
| :execution_position => :before_compile, | ||
| :input => "string" | ||
| }] | ||
| file_read_mock.verify | ||
| end | ||
| end | ||
| unless runInput["captureStdout"] | ||
| puts podfile.to_hash.to_json | ||
| end | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. …and spits out a serialised version of the |
||
| end | ||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we know which version of Node is run on this image? Would be cool if it was the latest or so
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I couldn’t spot an explicit listing of the version in the CI output, but it’s using
circleci/ruby:2.4-nodeand in this listing I seeENV NODE_VERSION=12.16.2. So probably that?