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.

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

[Gutenberg] - React Native 0.69.4 Upgrade #17066

Merged
merged 38 commits into from
Dec 7, 2022
Merged

[Gutenberg] - React Native 0.69.4 Upgrade #17066

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
3be6f11
Remove hermes-mirror in favor of react-native-mirror having both herm…
Aug 22, 2022
67db3c8
Update Gutenberg ref
Aug 23, 2022
52acffc
Update gutenberg-mobile ref
derekblank Sep 20, 2022
650ad55
Merge branch 'trunk' into gutenberg/react-native-0.69
derekblank Sep 21, 2022
cc396b9
Update Gutenberg ref
Sep 21, 2022
8d352e6
Exclude com.facebook.fbjni from Gutenberg Mobile binary
Sep 21, 2022
91efb5f
Update Gutenberg ref and fix building issues
Sep 23, 2022
0aab584
Update Gutenberg mobile ref
Oct 5, 2022
e2c4645
Merge branch 'trunk' into gutenberg/react-native-0.69
Oct 5, 2022
5d16496
Update Gutenberg ref
Oct 5, 2022
14fed3f
Update Gutenberg ref
Oct 5, 2022
037f3de
Update Gutenberg ref
Oct 11, 2022
5dfcaf9
Exclude fabricjni.so
Oct 11, 2022
432afbf
Update gutenberg ref
derekblank Oct 19, 2022
863a75c
Remove hermes-mirror in favor of react-native-mirror having both herm…
Aug 22, 2022
45d4231
Exclude com.facebook.fbjni from Gutenberg Mobile binary
Sep 21, 2022
64602e1
Update Gutenberg ref and fix building issues
Sep 23, 2022
ab9b05c
Update gutenberg ref
derekblank Oct 19, 2022
9ca0844
Merge branch 'gutenberg/react-native-0.69' of github.com:wordpress-mo…
Oct 19, 2022
ccd15ef
Merge branch 'trunk' into gutenberg/react-native-0.69
Oct 19, 2022
4ca7bdd
Update Gutenberg ref
Oct 19, 2022
5dad8d5
Fix Soloader version to fix issue with instrumented tests
Oct 20, 2022
00cb4ce
Merge branch 'trunk' into gutenberg/react-native-0.69
Oct 21, 2022
8c53322
Merge branch 'trunk' into gutenberg/react-native-0.69
Oct 27, 2022
f884bfd
Update Gutenberg ref
Oct 27, 2022
b411bda
Merge branch 'trunk' into gutenberg/react-native-0.69
Nov 3, 2022
14d60e6
React Native Upgrade - Avoid duplicated classes
Nov 3, 2022
48b3019
Resolve merge conflicts
derekblank Nov 16, 2022
b31f20d
Remove unneeded code
Nov 16, 2022
ec3cd62
Merge branch 'trunk' into gutenberg/react-native-0.69
Nov 29, 2022
1483a61
Update Gutenberg ref
Nov 29, 2022
9de67b2
Update gutenberg ref
derekblank Dec 5, 2022
d0b256f
Merge branch 'trunk' into gutenberg/react-native-0.69
derekblank Dec 5, 2022
b8ecbfc
Update gutenberg-mobile tag to 1.86.0-alpha1
derekblank Dec 5, 2022
ee6e2ac
Fix typo in Gutenberg tag reference
derekblank Dec 6, 2022
a3e0f2f
Merge branch 'trunk' into gutenberg/react-native-0.69
derekblank Dec 6, 2022
e3e505d
Update gutenberg tag
derekblank Dec 7, 2022
24eb8f5
Merge branch 'gutenberg/react-native-0.69' of https://github.com/word…
derekblank Dec 7, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 6 additions & 9 deletions WordPress/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,6 @@ repositories {
includeGroup "com.automattic.tracks"
}
}
maven {
url "https://a8c-libs.s3.amazonaws.com/android/hermes-mirror"
content {
includeGroup "org.wordpress-mobile"
}
}
fluiddot marked this conversation as resolved.
Show resolved Hide resolved
maven {
url "https://a8c-libs.s3.amazonaws.com/android/react-native-mirror"
content {
Expand Down Expand Up @@ -269,10 +263,13 @@ android {
// MPAndroidChart uses androidX - remove this line when we migrate everything to androidX
exclude 'META-INF/proguard/androidx-annotations.pro'

// Exclude React Native's JSC and Hermes debug binaries
// Exclude React Native's JSC and Fabric JNI
exclude '**/libjscexecutor.so'
exclude '**/libhermes-inspector.so'
exclude '**/libhermes-executor-debug.so'
exclude '**/libfabricjni.so'

// Avoid React Native's JNI duplicated classes
pickFirst '**/libc++_shared.so'
pickFirst '**/libfbjni.so'

pickFirst 'META-INF/-no-jdk.kotlin_module'

Expand Down
2 changes: 1 addition & 1 deletion build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ ext {
automatticRestVersion = '1.0.8'
automatticStoriesVersion = '2.0.0'
automatticTracksVersion = '2.2.0'
gutenbergMobileVersion = 'v1.85.1'
gutenbergMobileVersion = 'v1.86.0-alpha1'
wordPressAztecVersion = 'v1.6.2'
wordPressEmailChecker2Version = '1.1.0'
wordPressFluxCVersion = '2.6.0'
Expand Down
30 changes: 22 additions & 8 deletions libs/editor/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,16 +21,9 @@ repositories {
includeGroup "com.facebook.react"
}
}
maven {
url "https://a8c-libs.s3.amazonaws.com/android/hermes-mirror"
content {
includeGroup "org.wordpress-mobile"
}
}
google()
mavenCentral()
maven { url "https://www.jitpack.io" }
maven { url "https://a8c-libs.s3.amazonaws.com/android/hermes-mirror" }
}

android {
Expand Down Expand Up @@ -74,10 +67,31 @@ dependencies {
strictly wordPressAztecVersion
}
}

// Forcing version due to https://github.com/facebook/SoLoader/issues/94
// To be removed with React Native 0.70+
implementation("com.facebook.soloader:soloader") {
version {
strictly '0.10.4'
}
}

implementation 'com.facebook.fresco:fresco:2.5.0'
implementation 'com.facebook.fresco:imagepipeline-okhttp3:2.5.0'
implementation 'com.facebook.fbjni:fbjni:0.2.2'
implementation 'com.facebook.react:hermes-engine:0.69.4:release@aar'
implementation 'com.facebook.react:react-native:0.69.4:release@aar'
Comment on lines +79 to +83
Copy link
Contributor

Choose a reason for hiding this comment

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

It also adds several implementations in libs/editor/build.gradle since they're not included with the React Native and Hermes binaries.

Are these the libraries not included in the React Native and Hermes binaries? I'm wondering if we'd need to define them here or if they could be part of the libraries required by Gutenberg Mobile, could we have them as dependencies in the react-native-bridge build gradle file?

I noticed that we are also setting the React Native version, is this required? Would be a way to let the Gutenberg Mobile library to define this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since we encountered issues with using the debug artifacts for both Hermes and React Native instead of the release ones, when excluding them and including them again some libraries needed to be referenced again.

I'm working on a draft PR as a follow-up to improve these, although to be able to force the release artifacts for all builds like (Jalapeno Debug) which are the builds PRs generate, we will have to keep a few implementations. 😅


// This dependency will be substituted if the `local-builds.gradle` file contains
// `localGutenbergMobilePath`. Details for this can be found in the `settings.gradle` file.
implementation "$rootProject.gradle.ext.gutenbergMobileBinaryPath:$rootProject.ext.gutenbergMobileVersion"
implementation ("$rootProject.gradle.ext.gutenbergMobileBinaryPath:$rootProject.ext.gutenbergMobileVersion") {
exclude module: 'hermes-engine'
exclude module: 'react-native'
}

// Required Aztec dependencies (they should be included but Jitpack seems to be stripping these out)
implementation "org.jsoup:jsoup:$jsoupVersion"
Comment on lines +92 to +93
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we haven't updated the Aztec version, so I'm wondering why we need to add this dependency if we didn't have it before 🤔, what changed with the RN upgrade to producing this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This comment is also related for this question.

Copy link
Contributor

Choose a reason for hiding this comment

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

👋 @geriux @fluiddot !

I also stumbled upon this just now, that is, while working on the org.jsoup:jsoup dependency update (see here). I just removed this dependency from the libs:editor module (it being unused and all) and the WPAndroid seems to be building/working just fine.

I'm working on a draft PR as a follow-up to improve these, although to be able to force the release artifacts for all builds like (Jalapeno Debug) which are the builds PRs generate, we will have to keep a few implementations. 😅

Should I refrain from removing dependency and ignore this for now, that is, at least until this work above is done? Or maybe it is already done but I can't connect the dots just yet, not sure? 🤔

Thanking you in advance for your advise here! 🙏

Copy link
Contributor Author

Choose a reason for hiding this comment

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

and the WPAndroid seems to be building/working just fine.

Does the editor open correctly after this change?

Copy link
Contributor

@ParaskP7 ParaskP7 Mar 15, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

👋 @geriux and thanks for the reply!

Does the editor open correctly after this change?

Yes, I am able to open the editor correctly after this change (both the Classic and Block editor). Unless of course, I am missing something obvious here... 🤔

Copy link
Contributor

Choose a reason for hiding this comment

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

FYI: I just pushed this removal commit, feel free to test this PR yourself and let me know if everything works as you would have expected.

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like the dependency org.jsoup:jsoup is used by Aztec-Android (reference).

In fact, we set the version of that dependency here in Gutenberg. I'm not sure whether it can be removed or not, in any case, this could be tested by checking if the editor can be built and works as expected.


implementation "org.wordpress:utils:$wordPressUtilsVersion"

implementation "androidx.appcompat:appcompat:$androidxAppcompatVersion"
Expand Down