Skip to content
This repository has been archived by the owner on Jan 16, 2022. It is now read-only.

Upgrades to React Native 64 #397

Merged
merged 25 commits into from
Oct 21, 2021
Merged

Upgrades to React Native 64 #397

merged 25 commits into from
Oct 21, 2021

Conversation

redreceipt
Copy link
Member

@redreceipt redreceipt commented Jun 17, 2021

The upgrade is minor, this puts us way closer to the core react native project without a lot of unnecessary technical debt we've accrued.

Screen Shot 2021-06-16 at 10 49 45 PM

@redreceipt
Copy link
Member Author

pending facebook/react-native#31740

@redreceipt
Copy link
Member Author

redreceipt commented Jun 17, 2021

@vinnyjth I got carried away last night 😬 it's actually A LOT closer to working than I thought on the first pass

@@ -20,31 +24,21 @@ target 'apolloschurchapp' do
# Enables Flipper.
#
# Note that if you have use_frameworks! enabled, Flipper will not work and
# you should disable these next few lines.
# you should disable the next line.
use_flipper!('Flipper' => '0.75.1', 'Flipper-Folly' => '2.5.3', 'Flipper-RSocket' => '1.3.1')
Copy link
Member

Choose a reason for hiding this comment

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

I think we can omit the "custom" flipper versions now, also. that was a patch for our previous version of RN

Copy link
Member Author

Choose a reason for hiding this comment

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

it's still listed in a thread I found somewhere as a problem with 64 too

@redreceipt redreceipt marked this pull request as ready for review July 22, 2021 03:23
@redreceipt
Copy link
Member Author

redreceipt commented Jul 22, 2021

@vinnyjth this one is ready to look at. the bulk of the changes are linter changes. I took out our custom config and just went with react native community config now that they have one, removes a lot of infrastructure

@redreceipt redreceipt enabled auto-merge (squash) August 6, 2021 18:58
@redreceipt redreceipt requested a review from vinnyjth August 6, 2021 18:59
@vinnyjth
Copy link
Member

vinnyjth commented Aug 6, 2021

@redreceipt I LOVE that we are able to eliminate our custom config, but.... this will wreak havoc with the update tool 😬 . It will likely force manual merges for any files that have been modified by clients. Obviously, that's okay! It's what the update tool is for, but does the costbenefitt work out in our favor given that the custom linter config rarely causes issues?

As integration lead, you get to make that call :) I like the idea of waiting until we've got 98% of the code in apps before touching our lint settings.

@redreceipt
Copy link
Member Author

Oh, shoot I didn't think about that. Honestly, I did it because it puts us way closer to a true RN project which makes future react-native-upgrade calls easier. And if I push it off, I'll have to do the hard work again later of manually figuring out which of our eslint deps and config files are necessary and which are custom. I think I'd rather just bite the bullet now. App directories are already barely custom save the older apps. I think it'll be obvious to folks that REMOTE is just linter changes. We should just be able to always choose HEAD and then run eslint --fix, right?

@vinnyjth
Copy link
Member

vinnyjth commented Aug 6, 2021

I think it'll be obvious to folks that REMOTE is just linter changes. We should just be able to always choose HEAD and then run eslint --fix, right?

Ooohh that's a good point. That's assuming they are just moving one version up. If they are updating a bunch of different versions, with multiple template changes, they'll have to keep track of what files need to go to HEAD and what files need to be actually merged. So maybe we tell people to update to 2.29.1 and then from there to 2.30?

What if this is the start of version 3?

@redreceipt
Copy link
Member Author

redreceipt commented Aug 6, 2021 via email

@redreceipt redreceipt disabled auto-merge August 11, 2021 14:28
@redreceipt
Copy link
Member Author

ok @vinnyjth I got rid of most of the linter differences. Maybe we keep curly braces around if statements though?

Screen Shot 2021-08-12 at 7 42 14 PM

@redreceipt
Copy link
Member Author

blocked by https://github.com/ApollosProject/apollos-apps/pull/2063 and #430 so the events tests and mocking the timezone will be a non-issue

@redreceipt redreceipt requested a review from conrad-vanl August 17, 2021 03:02
@codecov
Copy link

codecov bot commented Aug 18, 2021

Codecov Report

Merging #397 (635ae38) into master (3d65272) will increase coverage by 0.10%.
The diff coverage is 75.00%.

❗ Current head 635ae38 differs from pull request most recent head cbaa6b6. Consider uploading reports for the commit cbaa6b6 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master     #397      +/-   ##
==========================================
+ Coverage   45.16%   45.26%   +0.10%     
==========================================
  Files           3        3              
  Lines          93       95       +2     
  Branches       17       17              
==========================================
+ Hits           42       43       +1     
- Misses         49       50       +1     
  Partials        2        2              
Impacted Files Coverage Δ
apolloschurchapp/src/user-settings/UserSettings.js 43.90% <75.00%> (+0.31%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3d65272...cbaa6b6. Read the comment docs.

@redreceipt
Copy link
Member Author

@vinnyjth if I get you screenshots, could you look at this?

@vinnyjth
Copy link
Member

yah sure @redreceipt

@redreceipt
Copy link
Member Author

Screen Shot 2021-10-11 at 9 38 01 AM

Screen Shot 2021-10-11 at 9 40 41 AM

@redreceipt redreceipt enabled auto-merge (squash) October 21, 2021 20:20
@redreceipt redreceipt merged commit 6ef4f7e into master Oct 21, 2021
@redreceipt redreceipt deleted the rn-64 branch October 21, 2021 20:23
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants