-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
[Gutenberg] Tweak Pod dependencies based on React Native version used by Gutenberg #21021
Changes from all commits
381d49a
ebd4cca
2b1138f
65e5454
2d40a68
598b038
341f617
551968e
9ceb947
4530832
170d7f4
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 |
---|---|---|
|
@@ -2,6 +2,7 @@ | |
|
||
# Helpers and configurations for integrating Gutenberg in Jetpack and WordPress via CocoaPods. | ||
|
||
require 'json' | ||
require_relative './version' | ||
|
||
DEFAULT_GUTENBERG_LOCATION = File.join(__dir__, '..', '..', 'gutenberg-mobile') | ||
|
@@ -29,43 +30,45 @@ | |
RNCMaskedView | ||
RNCClipboard | ||
RNFastImage | ||
React-jsc | ||
].freeze | ||
|
||
def gutenberg_pod(config: GUTENBERG_CONFIG) | ||
# We check local_gutenberg first because it should take precedence, being an override set by the user. | ||
return gutenberg_local_pod if should_use_local_gutenberg | ||
|
||
options = config | ||
|
||
# We check local_gutenberg first because it should take precedence, being an override set by the user. | ||
if should_use_local_gutenberg | ||
options = { path: local_gutenberg_path } | ||
id = options[:tag] || options[:commit] | ||
|
||
raise "Could not find Gutenberg pod at #{options[:path]}. You can configure the path using the #{LOCAL_GUTENBERG_KEY} environment variable." unless File.exist?(options[:path]) | ||
# Notice there's no period at the end of the message as CocoaPods will add it. | ||
raise 'Neither tag nor commit to use for Gutenberg found' unless id | ||
|
||
pod 'Gutenberg', podspec: "https://cdn.a8c-ci.services/gutenberg-mobile/Gutenberg-#{id}.podspec" | ||
end | ||
|
||
puts "[Gutenberg] Installing pods using local Gutenberg version from #{local_gutenberg_path}" | ||
def gutenberg_local_pod | ||
options = { path: local_gutenberg_path } | ||
|
||
react_native_path = require_react_native_helpers!(gutenberg_path: local_gutenberg_path) | ||
raise "Could not find Gutenberg pod at #{options[:path]}. You can configure the path using the #{LOCAL_GUTENBERG_KEY} environment variable." unless File.exist?(options[:path]) | ||
|
||
use_react_native! path: react_native_path | ||
puts "[Gutenberg] Installing pods using local Gutenberg version from #{local_gutenberg_path}" | ||
|
||
pod 'Gutenberg', options | ||
pod 'RNTAztecView', options | ||
react_native_path = require_react_native_helpers!(gutenberg_path: local_gutenberg_path) | ||
|
||
gutenberg_dependencies(options: options) | ||
else | ||
id = options[:tag] || options[:commit] | ||
use_react_native! path: react_native_path | ||
|
||
# Notice there's no period at the end of the message as CocoaPods will add it. | ||
raise 'Neither tag nor commit to use for Gutenberg found' unless id | ||
pod 'Gutenberg', options | ||
pod 'RNTAztecView', options | ||
|
||
pod 'Gutenberg', podspec: "https://cdn.a8c-ci.services/gutenberg-mobile/Gutenberg-#{id}.podspec" | ||
end | ||
gutenberg_dependencies(options: options) | ||
end | ||
|
||
def gutenberg_dependencies(options:) | ||
# When referencing via a tag or commit, we download pre-built frameworks. | ||
return if options.key?(:tag) || options.key?(:commit) | ||
|
||
podspec_prefix = options[:path] | ||
gutenberg_path = options[:path] | ||
|
||
raise "Unexpected Gutenberg dependencies configuration '#{options}'" if podspec_prefix.nil? | ||
|
||
|
@@ -74,28 +77,31 @@ def gutenberg_dependencies(options:) | |
|
||
computed_dependencies = DEPENDENCIES.dup | ||
|
||
# Use a custom RNReanimated version while we coordinate a fix upstream | ||
computed_dependencies.delete('RNReanimated') | ||
pod 'RNReanimated', git: 'https://github.com/wordpress-mobile/react-native-reanimated', branch: 'mokagio/fix-custom-node_modules-bypass-multiple-versions-check-2.17.0' | ||
react_native_version = react_native_version!(gutenberg_path: gutenberg_path) | ||
# We need to apply a workaround for the RNReanimated library when using React Native 0.71+. | ||
apply_rnreanimated_workaround!(dependencies: computed_dependencies, gutenberg_path: gutenberg_path) unless react_native_version[1] < 71 | ||
|
||
computed_dependencies.each do |pod_name| | ||
pod pod_name, podspec: "#{podspec_prefix}/#{pod_name}.#{podspec_extension}" | ||
end | ||
end | ||
|
||
def gutenberg_pre_install | ||
return unless should_use_local_gutenberg | ||
def apply_rnreanimated_workaround!(dependencies:, gutenberg_path:) | ||
# RNReanimated needs React-jsc | ||
# Reference: https://github.com/WordPress/gutenberg/pull/51386/commits/9538f8eaf73dfacef17382e1ab7feda40231061f | ||
dependencies.push('React-jsc') | ||
|
||
raise "[Gutenberg] Could not find local Gutenberg at given path #{local_gutenberg_path}" unless File.exist?(local_gutenberg_path) | ||
# Use a custom RNReanimated version while we coordinate a fix upstream | ||
dependencies.delete('RNReanimated') | ||
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. For reference, software-mansion/react-native-reanimated#4684 |
||
|
||
# This is required to workaround an issue with RNReanimated after upgrading to version 2.17.0 | ||
rn_node_modules = File.join(local_gutenberg_path, 'node_modules') | ||
|
||
rn_node_modules = File.join(gutenberg_path, '..', 'gutenberg', 'node_modules') | ||
raise "Could not find node modules at given path #{rn_node_modules}" unless File.exist? rn_node_modules | ||
|
||
ENV['REACT_NATIVE_NODE_MODULES_DIR'] = rn_node_modules | ||
|
||
puts "[Gutenberg] Set REACT_NATIVE_NODE_MODULES_DIR env var for RNReanimated to #{rn_node_modules}" | ||
|
||
pod 'RNReanimated', git: 'https://github.com/wordpress-mobile/react-native-reanimated', branch: 'wp-fork-2.17.0' | ||
end | ||
|
||
def gutenberg_post_install(installer:) | ||
|
@@ -105,7 +111,7 @@ def gutenberg_post_install(installer:) | |
|
||
react_native_path = require_react_native_helpers!(gutenberg_path: local_gutenberg_path) | ||
|
||
puts "[Gutenberg] Running Gunberg post install hook (RN path: #{react_native_path})" | ||
puts "[Gutenberg] Running Gutenberg post install hook (RN path: #{react_native_path})" | ||
|
||
# It seems like React Native prepends $PWD to the path internally in the post install hook. | ||
# When using an absolute path, we get this error, notice the duplicated "/Users/gio/Developer/a8c/wpios": | ||
|
@@ -155,3 +161,14 @@ def react_native_path!(gutenberg_path:) | |
|
||
react_native_path | ||
end | ||
|
||
def react_native_version!(gutenberg_path:) | ||
react_native_path = react_native_path!(gutenberg_path: gutenberg_path) | ||
package_json_path = File.join(react_native_path, 'package.json') | ||
package_json_content = File.read(package_json_path) | ||
package_json_version = JSON.parse(package_json_content)['version'] | ||
|
||
raise "[Gutenberg] Could not find React native version at #{react_native_path}" unless package_json_version | ||
|
||
package_json_version.split('.').map(&:to_i) | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,8 +11,8 @@ | |
# | ||
# LOCAL_GUTENBERG=../my-gutenberg-fork bundle exec pod install | ||
GUTENBERG_CONFIG = { | ||
# commit: '' | ||
tag: 'v1.100.0-alpha1' | ||
commit: 'd2eab790e3c4333a059f920ce6c4ced5b9c11112' | ||
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. Points to wordpress-mobile/gutenberg-mobile#5973 to use the workaround for the Hermes framework. |
||
# tag: 'v1.100.0-alpha1' | ||
} | ||
|
||
GITHUB_ORG = 'wordpress-mobile' | ||
|
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.
This diff removes
React-jsc
from the list of dependencies at the start of the file, only to add it back here, despite those dependencies only being used in the code path that hits this method.Is the rationale that
React-jsc
is required only because of the particularRNReanimated
setup that we are using here?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.
We had to do something similar for the Gutenberg demo app (WordPress/gutenberg@9538f8e) when using the newer version of Reanimated. It's unclear to me why it really needs it, but now that we are using Hermes, probably it's not required. I'll remove it and check if it produces any build failure.