Skip to content
This repository has been archived by the owner on Mar 12, 2020. It is now read-only.

React Native Upgrade for 0.59 #1553

Merged
merged 11 commits into from
Apr 9, 2019
Merged

React Native Upgrade for 0.59 #1553

merged 11 commits into from
Apr 9, 2019

Conversation

orta
Copy link
Contributor

@orta orta commented Mar 21, 2019

OK, now that #1551 is in.

Notable upgrade points:

Blocked on

  • yarn type-check
  • yarn jest
  • Running all screens
    • Shows
    • Fair
    • Map
    • Artist
    • Homepage
    • Gene
    • Gene Refined
    • My Profile
    • Start Consignment Flow
    • Inbox
    • Inquiry
    • Favorites
    • Bid Flow

#skip_new_tests

@orta
Copy link
Contributor Author

orta commented Mar 22, 2019

Looks like since we started piping the TS output into tee - then fails don't matter, normally not a problem because Danger would run on it - but while this PR is a draft, Travis doesn't make pr-based CI runs, so no danger output it seems.

Copy link
Contributor

@ashfurrow ashfurrow left a comment

Choose a reason for hiding this comment

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

So exciting!

@@ -9,8 +9,5 @@ gem 'cocoapods-check'
# To manage our secret keys
gem "cocoapods-keys"

# To apply fixes to the React Native code
gem 'cocoapods-fix-react-native'
Copy link
Contributor

Choose a reason for hiding this comment

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

😻

@orta
Copy link
Contributor Author

orta commented Mar 25, 2019

@alloy - I believe you have commits laying around for this

@alloy
Copy link
Contributor

alloy commented Mar 26, 2019

Yup, continuing on this today 👍

@orta
Copy link
Contributor Author

orta commented Apr 2, 2019

I made sure that TypeScript can fail CI now 👍

@alloy
Copy link
Contributor

alloy commented Apr 2, 2019

Thanks!

@artsy artsy deleted a comment from peril-staging bot Apr 2, 2019
@orta orta marked this pull request as ready for review April 2, 2019 13:27
@alloy
Copy link
Contributor

alloy commented Apr 2, 2019

This PR updates tipsi-stripe from 5.2.4 to 7.4.0 because of RN API changes that were fixed in 7.2.0. Here’s the CHANGELOG https://github.com/tipsi/tipsi-stripe/blob/master/CHANGELOG.md#530---2018-07-23

We can probably fix the issue with a patch-package patch, if this update is deemed too dangerous.

Who is the person that knows most about how this should all work?

@alloy
Copy link
Contributor

alloy commented Apr 2, 2019

There’s a few deprecation warnings about libraries that have moved out of core RN, but I didn’t want to pile that on in this PR, we should follow up with that.

  • AsyncStorage
  • ViewPagerAndroid (not sure why we’re getting messages about this one)

@orta
Copy link
Contributor Author

orta commented Apr 2, 2019

I just realized, we should be bumping major versions with these react native updates

@alloy
Copy link
Contributor

alloy commented Apr 2, 2019

Should we? Are we changing Emission’s API?

@artsy artsy deleted a comment from peril-staging bot Apr 2, 2019
@artsy artsy deleted a comment from DangerCI Apr 2, 2019
@artsy artsy deleted a comment from peril-staging bot Apr 2, 2019
@alloy
Copy link
Contributor

alloy commented Apr 2, 2019

UPDATE: Fixed

Seeing the following error in some of the Consignment Flow forms.

Simulator Screen Shot - iPhone 8 - 2019-04-02 at 19 16 14

@artsy artsy deleted a comment from peril-staging bot Apr 2, 2019
@artsy artsy deleted a comment from peril-staging bot Apr 2, 2019
@artsy artsy deleted a comment from peril-staging bot Apr 2, 2019
@artsy artsy deleted a comment from peril-staging bot Apr 2, 2019
@artsy artsy deleted a comment from peril-staging bot Apr 2, 2019
@artsy artsy deleted a comment from peril-staging bot Apr 2, 2019
@alloy
Copy link
Contributor

alloy commented Apr 2, 2019

@orta Ok, I believe you and @sweir27 are testing the tipsi-stripe upgrade, but otherwise I think this is good to go.

@orta
Copy link
Contributor Author

orta commented Apr 2, 2019

I mean, yeah, it's a full breaking change the emission API right? You can't use this with an existing client at all

@@ -10,7 +10,7 @@ patch-package
- }
+ // if (name.length > 1 && name[0] === '_' && name[1] === '_') {
+ // return new _GraphQLError.GraphQLError('Name "' + name + '" must not begin with "__", which is reserved by ' + 'GraphQL introspection.', node);
+ // }
+ // }x
Copy link
Contributor Author

Choose a reason for hiding this comment

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

^

Copy link
Contributor

Choose a reason for hiding this comment

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

Hah

@alloy
Copy link
Contributor

alloy commented Apr 2, 2019

I’m not sure I follow. Eigen doesn’t need to change any API usage (of Emission), right?

@artsy artsy deleted a comment from DangerCI Apr 2, 2019
@DangerCI
Copy link

DangerCI commented Apr 2, 2019


Warnings
⚠️ Missing Test Files:
  • src/lib/Components/Bidding/Components/__tests__/BackButton-tests.tsx
  • src/lib/Components/__tests__/DottedLine-tests.tsx
  • src/lib/Components/__tests__/LoadFailureView-tests.tsx
  • src/lib/Components/__tests__/OpaqueImageView-tests.tsx
  • src/lib/Components/__tests__/Spinner-tests.tsx
  • src/lib/Components/__tests__/SwitchView-tests.tsx
  • src/lib/Components/__tests__/Video-tests.tsx

If these files are supposed to not exist, please update your PR body to include "#skip_new_tests".

Generated by 🚫 dangerJS

@alloy alloy mentioned this pull request Apr 3, 2019
@orta
Copy link
Contributor Author

orta commented Apr 9, 2019

OK, I've given the stripe code a runtime audit and everything looks to be working just fine. Merging!

@orta orta merged commit 843d0ee into master Apr 9, 2019
@orta
Copy link
Contributor Author

orta commented Apr 9, 2019

Thanks for putting the time in @alloy !

@alloy alloy deleted the one_rn_60_ branch April 10, 2019 11:29
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.

4 participants