[ENG-8790][eas-cli] Fix updates synchronization of native files to include strings.xml

[wschurman]

Why

expo/expo@166385e added this to the config plugin.

This code in eas-cli is supposed to emulate the config plugin, but for bare projects.

How

Make the config plugin emulation also do the new synchronization of strings.xml.

Test Plan

• npx create expo-app
• npx expo prebuild
• add runtime version to app.json
• install expo-updates
• commit everything
• eas build (current prod version)
  • it added <meta-data android:name="expo.modules.updates.EXPO_RUNTIME_VERSION" android:value="@string/expo_runtime_version"
  • but there were no changes in strings.xml
• neas build (with this PR)
  • it added <meta-data android:name="expo.modules.updates.EXPO_RUNTIME_VERSION" android:value="@string/expo_runtime_version"
  • it added <string name="expo_runtime_version">1.0</string> to strings.xml

[wschurman]

After figuring out why this code is necessary (i.e. why config plugins aren't being run), my next questions will be:
a) How can we make them run? Why don't they run? How do we expect people to do this?
b) Can we just call the config plugin here? Why is that necessary?

I think I'm missing a ton of context about why this sync even needs to happen in the first place.

[github-actions]

Size Change: -959 B (0%)

Total Size: 41.5 MB

Filename	Size	Change
./packages/eas-cli/dist/eas-linux-x64.tar.gz	41.5 MB	-959 B (0%)

_{compressed-size-action}

[codecov]

Codecov Report

Merging #1865 (871b3fe) into main (4ace7d2) will decrease coverage by 0.01%.
The diff coverage is 20.00%.

❗ Current head 871b3fe differs from pull request most recent head e6d3e19. Consider uploading reports for the commit e6d3e19 to get more accurate results

@@            Coverage Diff             @@
##             main    #1865      +/-   ##
==========================================
- Coverage   52.73%   52.72%   -0.01%     
==========================================
  Files         474      474              
  Lines       17110    17114       +4     
  Branches     3423     3423              
==========================================
  Hits         9021     9021              
- Misses       8072     8076       +4     
  Partials       17       17              

Impacted Files	Coverage Δ
...ckages/eas-cli/src/update/android/UpdatesModule.ts	50.00% <20.00%> (-6.66%)	⬇️

[wkozyra95]

This code in eas-cli I guess is supposed to emulate the config plugin?

It's intended to sync values with native code in a bare project.

After figuring out why this code is necessary (i.e. why config plugins aren't being run), my next questions will be:
a) How can we make them run? Why don't they run? How do we expect people to do this?
b) Can we just call the config plugin here? Why is that necessary?

I think answer for all of this is because this is used in bare projects, so we can't expect that prebuild will or even could be run.

Can we just call the config plugin here? Why is that necessary?

And, specifically to that point. Maybe we can here, but not in general. Code in eas-cli needs to handle most (ideally all) native projects that EAS supports, so it's a higher standard than is expected in prebuild.

e.g. prebuild assumes that there will be no variant/flavors and when we interact with any with projects we need to be aware of that. For example instead of writing to main strings.xml you could write to runtime version to resource specific to a variant that is going to be built. (just an example I don't think it's need here)

[quinlanj]

Thinking about this problem a bit more, how about colocating all the workflow agnostic 'modding code' in @expo/config-plugins? The problem is, we have 4 places which we 'mod' the Updates code:

• expo prebuild, managed workflow. Code in @expo/config-plugins, called in npx expo prebuild and Turtle Builders
• eas updates:configure, bare workflow. Calls out to a subroutine in @expo/config-plugins
• eas build, bare workflow. Calls out to a subroutine in @expo/config-plugins
• Turtle Builders, workflow agnostic. Code in build-tools

When doug added bug fixes to @expo/config-plugins, all the other call sites for the bare/agnostic workflows fell out of sync. They either broke completely by running a subset of the methods, or were flawed because they never picked up the bug fixes. What if we export a method called modUpdatesBareOrAgnosticWorkflow in @expo/config-plugins that mods a bare/agnostic workflow, and have it be called by eas-cli and build-tools? That way, if we make any bug changes in the future, all our fixes can be made in @expo/config-plugins and we won't have to reimplement the same thing in 4 different places.

@wkozyra95 mentioned there is a higher standard in eas-cli and build-tools, where we cannot make assumptions about the variant/flavour of a project. However, if someone were to install expo-updates, we are requiring them to have the correct keys in AndroidManifest.xml and strings.xml. We can assert all our preconditions in the @expo/config-plugins for the bare and agnostic workflows if they are different from managed. So something like this:

export const withUpdates = [...]

// run by build-tools and eas-cli
export function modUpdatesBareOrAgnosticWorkflow () {

// assert preconditions for string.xml, if any

// run the same subroutines as withUpdates

}

[wkozyra95]

What if we export a method called modUpdatesBareOrAgnosticWorkflow in @expo/config-plugins that mods a bare/agnostic workflow, and have it be called by eas-cli and build-tools?

The problem is that at different places we are doing slightly different things. if we had some method to sync configuration only for updates then we could use it in eas-cli and prebuild, but in turtle code does not sync values with app.json. It syncs one value passed as an argument.

We could write this method (e.g. setUpdatedConfiguration) in a way that takes everything as an argument and only updates stuff based on the passed object, but in that case, it wouldn't be full solution, just a helper method, so there still would be a risk that someone adds code to plugin and not this helper method. At this point, we are getting back to a problem what is a public API of config-plugins?

where we cannot make assumptions about the variant/flavour of a project

That was just an example, there other alternatives could be

The other approaches that we might consider:

• Using only well-defined utils and reimplementing top level stuff. It sounds like a lot of work, but after switch to versioned cli using config-plugins code becomes a lot more complicated, so overall it could be beneficial to just avoid reusing that code. In the current workflow when fixing sth in config-plugins you need to make a change there, publish it (which is complicated in itself) and update it everywhere. With flow like that, I think fixing sth in 2-3 places might be less work.
• Tagging with some comments about which functions are used in cli and turtle.

[quinlanj]

@wkozyra95 makes sense. At the very least, let's add a comment in config-plugins for anyone who tries to make changes in the future. That way, we can do our best to avoid this type of situation from happening again and it's not a lot of extra work.

[linear]

ENG-8790 Fix updates synchronization of native files to include strings.xml

https://exponent-internal.slack.com/archives/C013ZK4SA12/p1684967716760549

[wschurman]

Still need to test this before landing. I'm assuming the repro is something like: create bare project using old CLIs, prebuild using old CLI, build using the CLI in this PR?

[wkozyra95]

Still need to test this before landing. I'm assuming the repro is something like: create bare project using old CLIs, prebuild using old CLI, build using the CLI in this PR?

yes, but you also need to specify some runtimeVersion (or policy) so when you run build there is anything to sync. Plus expo-updates need to be installed, otherwise version is not synced
I listed my reporo here https://exponent-internal.slack.com/archives/C013ZK4SA12/p1685522560312919?thread_ts=1684967716.760549&cid=C013ZK4SA12

[wschurman]

/changelog-entry bug-fix Fix updates synchronization of native files to include strings.xml for bare projects

[github-actions]

✅ Thank you for adding the changelog entry!

[sparga]

Since eas-cli 3.13.3, which include this PR, the XML prolog of my strings.xml is removed every time I start a build with eas build --no-wait --non-interactive -p all --auto-submit.

And then it fails because I'm using --non-interactive and the git state is not clean anymore. Is it expected?

Here is the diff after trying to build:

diff --git a/android/app/src/main/res/values/strings.xml b/android/app/src/main/res/values/strings.xml
--- a/android/app/src/main/res/values/strings.xml
+++ b/android/app/src/main/res/values/strings.xml
@@ -1,4 +0,3 @@
-<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
 <resources>
   <string name="app_name">MyApp</string>
   <string name="expo_runtime_version">10</string>