[Gutenberg] Force using release variants for editor libraries

[SiobhyB]

Description

We noticed in the latest Android installable build, as part of the RN upgrade, that the editor takes a bit longer to load. After checking the latest upgrade we made (version 0.69.4), the team involved in that effort pointed out that we experienced a similar issue (p1688741473102169-slack-CSYKQSY8G). The culprit, in that case, was the fact that WP-Android is using the debug variant of React Native modules.

As such, we want to attempt to enforce the release variant for all scenarios.

Testing

TBD

Regression Notes

1. Potential unintended areas of impact

TBD

2. What I did to test those areas of impact (or what existing automated tests I relied on)

TBD

3. What automated tests I added (or what prevented me from doing so)

TBD

---

PR submission checklist:

• I have completed the Regression Notes.
• I have considered adding accessibility improvements for my changes.
• I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

[wpmobilebot]

📲 You can test the changes from this Pull Request in Jetpack by scanning the QR code below to install the corresponding build.

	App Name	Jetpack
	Flavor	Jalapeno
	Build Type	Debug
	Version	pr18760-5e46a8b
	Commit	5e46a8b
	Direct Download	jetpack-prototype-build-pr18760-5e46a8b.apk

Note: Google Login is not supported on these builds.

[wpmobilebot]

📲 You can test the changes from this Pull Request in WordPress by scanning the QR code below to install the corresponding build.

	App Name	WordPress
	Flavor	Jalapeno
	Build Type	Debug
	Version	pr18760-5e46a8b
	Commit	5e46a8b
	Direct Download	wordpress-prototype-build-pr18760-5e46a8b.apk

Note: Google Login is not supported on these builds.

[SiobhyB]

@ParaskP7, 👋 , I wonder if I might be able to pick your brain here. We're attempting to enforce the release variants of the react-android and hermes-android libraries, so that the editor isn't as slow to load in PR builds. More context can be found in p1688741473102169-slack-CSYKQSY8G.

I attempted to see if changing the debug buildType in the editor's build-gradle file here (so that it matches the release buildType) could be one hacky way to achieve this, but it doesn't appear to work.

Do you happen to know if there are any valid ways to enforce that the editor module is always built in the release variant, even for local and PR builds? Thanks in advance for any insights!

[ParaskP7]

Just and FYI and as per the documentation when minifyEnabled is set to true then this enables code shrinking, obfuscation, and optimization for only your project's release build type. Make sure to use a build variant with 'isDebuggable=false'.

PS: I am not sure if that's what you want and whether this will help the editor to load faster. 🤔

[SiobhyB]

Thanks Petros, this code was a hacky experiment to see if I could "trick" the debug variant into matching the main app's release variant. 😅 It didn't appear to have any impact on the editor's load time, and from your comments it does seem like it isn't a viable approach to continue experimenting with. Really appreciate your expertise here. 🙇‍♀️

[ParaskP7]

Just and FYI and as per the documentation when proguardFiles is configured then this includes the default ProGuard rules files that are packaged with the Android Gradle plugin.

PS: I am not sure this will work as there is no proguard.cfg file for the editor module, we only got that on the WordPress module. 🤔

[ParaskP7]

👋 @SiobhyB and thanks for the ping!

Do you happen to know if there are any valid ways to enforce that the editor module is always built in the release variant, even for local and PR builds? Thanks in advance for any insights!

I am looking into that and trying to think what can we do about that. I will share some insights once I have something concrete for you.

PS: However, and no matter our looking into that, or whether something like that is feasible or not, IMHO enforcing the editor module to always be built in the release build type might not be the best idea since this would mean that we might have some unexpected behaviour while running the app in debug mode, due to a mixture of build types, and that, maybe, might even create some false positive crashes that will become hard to reason about, or anything of that matter. I am just thinking out-loud here for the moment, not sure... 🤔

[ParaskP7]

Following this p1688741473102169-slack-CSYKQSY8G and its associated previous p1688725502239169-slack-CSYKQSY8G thread, plus this last statement below:

...because it will always be an issue. I wonder if we could make the editor module be built in release in any variant the WordPress app...

I now wonder if that is indeed the correct way of dealing with this problem, that is, even if that ends-up being possible. It is another thing to filter the published RN debug artifacts, and thus only use the release ones as a binary dependency, and, it is another thing to enforce the release build type for the editor module, to be used as such on compile time.

---

One might try the following on the editor related build.gradle file and see the build failing:

android {
    ...
    variantFilter {
        // Exclude debug builds from APKs
        if (buildType.name == 'debug') {
            setIgnore(true)
        }
    }
}

The above filters out the debug build type so that only the release build type is used for this module. However, this is not allowed and the overall build will complain on not finding the debug build type for the editor module.

---

Another way to do it is to duplicate the debug build type of the release build type, something, which I guess this PR is about. However, I am not sure if that is even feasible and even if it is, what the implications of doing so might be thereafter. 🤔

PS: The debug and release build types are not just one more variant for us to fine tune as we like, those are the core Android variants that every other custom variants (productFlavors + dimension) are build on top, like vanilla/wasabi/jalapeno and wordpress/jetpack.

---

To my understanding, we are just trying to overcome a problem on filtering out published debug RN artifacts from the editor module, something you already done in the past, successfully, but on the RN level, not within WPAndroid.

If that is still the case and you can do that again to overcome the slow startup time on Android, I suggest doing that instead of trying to enforce the release on a debug build for WPAndroid.

I understand that you would like to solve this issue on the WPAndroid level, once and for all, so that to avoid having to go through that painful process of analysing, investigating and solving this issue, again and again, but I wonder if the solution lies on the RN level, not the editor module on the WPAndroid level. 🤔

Let me know your thoughts on all the above and I can further investigate this.

[ParaskP7]

Let me shamelessly plug @oguzkocer into this discussion too, I am keen to hear his thoughts on it (🙏), him being an expert on this kind of RN + Gradle + Publish setup (😊).

[SiobhyB]

@ParaskP7, thank you again for all the insights here. It's super useful. Here's where my understanding is currently at:

• In the PR we made for the 0.69.4 upgrade, we specified the release version in the React Native and Hermes dependencies (reference) by appending :release@aar. Doing similar for the 0.71.11 upgrade causes a duplicate class error.
• Given the above, we wanted to see if it was possible to always build the entire editor module in the release variant. However, from the discussions here, it appears that is not possible.
• To workaround this error we should therefore go back to figuring out if it's possible to specify the release version for the React Native and Hermes dependencies for 0.71.11.

Does that sound right to you?

[ParaskP7]

👋 @SiobhyB and thank you for the reply, your understanding is spot-on! 💯

In #17066, we specified the release version in the React Native and Hermes dependencies (reference) by appending :release@aar. Doing similar for the 0.71.11 upgrade causes a duplicate class error.

👍

Given the above, we wanted to see if it was possible to always build the entire editor module in the release variant. However, from the discussions here, it appears that is not possible.

That's right, it does "appears" so, but I want to be clear here that didn't invest a whole bunch of time in really digging deeper into that as IMHO we shouldn't even consider trying to do this change, that is, unless we have exhausted every other "alternative".

To workaround this error we should therefore go back to figuring out if it's possible to specify the release version for the React Native and Hermes dependencies for 0.71.11.

Yes, this is what I understand as well. Also, to my understanding, on past RN upgrades, some of them, this is what was done. Please someone correct me if I am wrong.

[SiobhyB]

Following the latest discussion at p1689171765012609/1688741473.102169-slack-CSYKQSY8G, we've decided to leave this issue for now. There doesn't appear to be any straightforward way to enforce the release variants of the react-android and hermes-android libraries, and the delay it's causing isn't currently noticeable enough to warrant further work on this.

[oguzkocer]

@SiobhyB We used to publish our own artifacts for react-native and as part of it, we were publishing both release & debug variants. This is not a common practice, however both of these artifacts were available in node_modules, so we wanted to make sure that developers had the option to use to debug variant if they would like. Here is that configuration for 0.69.4.

Facebook recently started publishing react-native, now renamed as react-android, to mavenCentral. The configuration for that would be different and seems to be following the common approach. So, the -release@aar is probably not available and even if it was, I don't think it's necessary. I suggest letting Gradle and Android Gradle Plugin to do the correct thing here and don't force the release version unless there is a documented flag for it.

Also worth noting that duplicate class errors are generally not blockers, so if you come across them in any other instance, please feel free to reach out as there is likely a way to resolve them.

[ParaskP7]

Also worth noting that duplicate class errors are generally not blockers, so if you come across them in any other instance, please feel free to reach out as there is likely a way to resolve them.

💯 ⬆️ 💯

[SiobhyB]

@oguzkocer, thanks for your insight. I appreciate we can generally work to resolve errors, but I noted the error in this case as an obstacle to consider when weighing up our next steps. It sounds like we're on the same page with keeping the small delay for debug builds, and not spending further time working on ways to enforce release variants, though not let me know if I'm missing anything.

Thanks again for the help, all!