-
Notifications
You must be signed in to change notification settings - Fork 3k
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
[No QA] Upgrade react native CLI to 6.2.0 #7117
Conversation
Tagging in @Jag96 for another review because we chatted about this 1:1 |
Curious why there is a diff in the compiled GitHub actions? I am too. I'm not sure I have a 100% crystal clear understanding of exactly what happened here, but my hand-wavy explanation is:
|
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.
I've got a Podfile.lock diff but the changes look unrelated, just confirming that there wasn't anything that had to be pushed there.
The GH actions diff is interesting, I built the actions locally and confirmed there isn't a diff so not sure what the cause is.
Anyways, one of the dependencies used in our GitHub Actions must have somehow been wrapped up with the fbjs module such that ncc was including fbjs (or some subset thereof) in the compiled GitHub Actions output. But now that fbjs is included as its own separate dependency, ncc knows that it's not imported by our GitHub Actions. So all that code is no longer in the compiled ncc output.
👍 Idk if there's a better way to confirm that, but I'm not sure we need to.
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.
I followed the test steps and got it running on all platforms. Thanks for doing the upgrade.
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
🚀 Deployed to staging by @neil-marcellini in version: 1.1.27-2 🚀
|
🚀 Deployed to staging by @neil-marcellini in version: 1.1.27-3 🚀
|
Regarding IMO we only had We can remove it from
Which it seems you already did, so we can just remove it from our Finally, We do use App/config/checkMetroBundlerPort.js Line 1 in 7e50a8d
It should be listed in our |
🚀 Deployed to staging by @neil-marcellini in version: 1.1.27-3 🚀
|
Details
Fixes a local build error I'm seeing on Android: https://expensify.slack.com/archives/C01GTK53T8Q/p1641859243262700
Note: I followed the custom upgrade instructions here.
Note: I had to manually install fbjs wonday/react-native-pdf#592
Fixed Issues
$ n/a
Tests
rm -rf node_modules
npm i
react-native start --reset-cache
QA Steps
None.
Tested On
Mobile WebThis is the same build as web, the client makes no difference wrt to the CLIScreenshots
Web
Desktop
iOS
Android