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.

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

[expo-updates] Fetch asset manifest through programmatic CLI interface instead of depending on a running React Native CLI server #9372

Merged
merged 2 commits into from
Jul 24, 2020

Conversation

brentvatne
Copy link
Member

Why

Fixes this issue: #8374

Basically you want to be able to run ./gradlew :app:assembleRelease without running a React Native CLI server beforehand, and this makes it possible to do so.

How

  • Use the Metro Server API that is used by react-native bundle in the previous build phase.
  • Use @expo/metro-config to configure Metro, because we don't want to depend on the private configuration code that is internal to @react-native-community/cli.

Test Plan

I patched these changes into an existing bare project and it worked properly on iOS and Android. It was able to unblock me to build on the build service.

…pending on a running React Native CLI server
@brentvatne brentvatne changed the title Fetch asset manifest through programmatic CLI interface instead of depending on a running React Native CLI server [expo-updates] Fetch asset manifest through programmatic CLI interface instead of depending on a running React Native CLI server Jul 24, 2020
Copy link
Contributor

@esamelson esamelson left a comment

Choose a reason for hiding this comment

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

lgtm! this should also fix #8691.

@brentvatne brentvatne merged commit db13c28 into master Jul 24, 2020
@brentvatne brentvatne deleted the @brent/expo-updates-programmatic-assets branch July 24, 2020 19:27
brentvatne added a commit that referenced this pull request Jul 24, 2020
…e instead of depending on a running React Native CLI server (#9372)

* Fetch asset manifest through programmatic CLI interface instead of depending on a running React Native CLI server

* Update CHANGELOG
@todorone
Copy link

@brentvatne This PR broke building iOS project on RN 0.63 because of this commit - facebook/react-native#28354

On RN 0.63.x due to above mentioned commit createManifest.js is trying to find package.json one level higher than it is situated.

@brentvatne
Copy link
Member Author

brentvatne commented Jul 28, 2020

i think you can override the PROJECT_ROOT option in your build phases to make it whatever you like to resolve the issue.

can you suggest an alternative that will work on 0.62 and 0.63?

@todorone
Copy link

todorone commented Jul 28, 2020

@brentvatne I was able to resolve issue by adjusting PROJECT_ROOT variable and adjusting run path in Bundle React Native code and images build phase of the project. Sorry for ping...

EDIT: Actually i was not able, i just downgraded to the previous expo-updates version... My bash-scripting-fu is not enough to resolve the issue... :)

@brentvatne
Copy link
Member Author

we could possibly add a conditional to handle this for 0.63+

brentvatne added a commit that referenced this pull request Aug 1, 2020
…anifest

- #9372 (comment)
- I was unable to reproduce it but if the reported cause is correct then this
  should resolve it.
brentvatne added a commit that referenced this pull request Aug 1, 2020
…anifest

- #9372 (comment)
- I was unable to reproduce it but if the reported cause is correct then this
  should resolve it.
@brentvatne
Copy link
Member Author

@todorone - i tried to reproduce this on a new 0.63 project and i was unable to. i created a new project and installed expo-updates on it and the release build worked as expected.

given your description of the problem i attempted a fix: a566b2a

expo-updates@0.2.14 includes this code. please let me know if it resolves your issue

@todorone
Copy link

todorone commented Aug 1, 2020

@brentvatne New version works perfect for me, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants