Skip to content

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

Merged
chinesedfan merged 5 commits intogitpoint:masterfrom
machour:better-upgrade-rn
Jan 27, 2018
Merged

chore: Upgrade react, react-dom, react-native to latests versions#698
chinesedfan merged 5 commits intogitpoint:masterfrom
machour:better-upgrade-rn

Conversation

@machour
Copy link
Copy Markdown
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

@coveralls
Copy link
Copy Markdown

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
Copy Markdown
Member

@housseindjirdeh housseindjirdeh left a comment

Choose a reason for hiding this comment

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

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
Copy Markdown
Member Author

machour commented Jan 17, 2018

Please do \o/

Copy link
Copy Markdown
Member

@chinesedfan chinesedfan left a comment

Choose a reason for hiding this comment

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

@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.

jsCodeLocation = [[RCTBundleURLProvider sharedSettings] jsBundleURLForBundleRoot:@"index" fallbackResource:nil];
#else
jsCodeLocation = [CodePush bundleURL];
jsCodeLocation = [CodePush bundleURL];
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Shall we keep the pure empty line and old indentations?

@machour
Copy link
Copy Markdown
Member Author

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

NSURL *jsCodeLocation;


Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

the pure empty line

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

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

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
@machour machour deleted the better-upgrade-rn branch January 29, 2018 06:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants