X Tutup
The Wayback Machine - https://web.archive.org/web/20201218001852/https://github.com/gitpoint/git-point/pull/698
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

chore: Upgrade react, react-dom, react-native to latests versions #698

Merged
merged 5 commits into from Jan 27, 2018

Conversation

@machour
Copy link
Member

@machour machour commented Jan 16, 2018

Description

Resurrecting #523, I updated react, react-native to their latests versions using react-native-git-upgrade. I then proceeded to fix conflicts and get rid of index.ios.js & index.android.js in favor of index.js 🎉

Tested the application on both iOS and Android simulator, nothing seems to brake.

The app startup is quicker, it becomes responsive as soon as it's launched, overall, it seems to be on steroïds ... WOW.

I was only able to validate this on Android, as I never was able to run the app on iOS without hardcoding my IP in AppDelegate.m. @chinesedfan could you give it a try on iOS ?

For this PR, the best thing is to rm -rf node_modules and then yarn && yarn run link && yarn start --reset-cache

PS: Upgrading react-native-linear-gradient is mandatory as it fixes a compatibility problem: react-native-linear-gradient/react-native-linear-gradient#235

machour added 2 commits Jan 16, 2018
Upgrading react-native-linear-gradient is mandatory as it fixes a compatibility problem: react-native-linear-gradient/react-native-linear-gradient#235
@machour machour requested a review from chinesedfan Jan 16, 2018
@coveralls
Copy link

@coveralls coveralls commented Jan 16, 2018

Coverage Status

Coverage increased (+0.07%) to 44.074% when pulling 70739b0 on machour:better-upgrade-rn into 0d694bf on gitpoint:master.

Copy link
Member

@housseindjirdeh housseindjirdeh left a comment

Genuinely excited to see how much it approves perf :)

Can test on my iPhone when I'm home tonight if @chinesedfan doesn't get to it in time.

@machour
Copy link
Member Author

@machour machour commented Jan 17, 2018

Please do \o/

Copy link
Member

@chinesedfan chinesedfan left a comment

@machour Great improvement. I just see the logo in a flash then events list is there! Except a tiny indentation question and yarn.lock conflicts, we can merge it.

#else
jsCodeLocation = [CodePush bundleURL];
jsCodeLocation = [CodePush bundleURL];

This comment has been minimized.

@chinesedfan

chinesedfan Jan 21, 2018
Member

Shall we keep the pure empty line and old indentations?

@machour
Copy link
Member Author

@machour machour commented Jan 26, 2018

@chinesedfan Fixed the whitespaces changes
@housseindjirdeh I'd love it if you can give this a go and merge this one in <3

@@ -20,9 +20,9 @@ - (BOOL)application:(UIApplication *)application didFinishLaunchingWithOptions:(
{
NSURL *jsCodeLocation;


This comment has been minimized.

@chinesedfan

chinesedfan Jan 27, 2018
Member

the pure empty line

@machour Sorry, I didn't comment very clearly before.

This comment has been minimized.

@machour

machour Jan 27, 2018
Author Member

Originally there was a line with a single space in it. I just removed it all together.
Not sure if I'm following you though :D

@chinesedfan chinesedfan merged commit e0ffe2c into gitpoint:master Jan 27, 2018
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.07%) to 44.074%
Details
@machour machour deleted the machour:better-upgrade-rn branch Jan 29, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.
X Tutup