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

[expo] Upgrade vendored modules #5514

Merged
merged 9 commits into from Sep 9, 2019

Conversation

@sjchmiela
Copy link
Contributor

commented Aug 31, 2019

Why

In preparation for SDK 35.

How

Went through the list of outdated vendored modules and:

  • updated to latest: react-native-{webview, appearance, safe-area-context, screens, reanimated, maps}
  • updated to not latest: react-native-svg (9.8.[0,1] still has some bugs)
  • not updated: react-native-{branch, gesture-handler, view-shot, netinfo} due to required AndroidX upgrade; lottie-react-native due to required Swift integration

We will need to maintain our own fork of react-native-webview for one release cycle until we upgrade to AndroidX. We needed changes in v7 (removal of UIWebView) and v6 introduced AndroidX. I cloned upstream to expo/react-native-webview and reverted AndroidX change.

Due to this we have to decide whether to put https://…/expo/react-native-webview under react-native-webview to bundledNativeDependencies (will make sure that when people eject their Android projects will build, otherwise they will get the official release which won't work due to AndroidX support) or leave it as ~7.0.0 (and put somewhere the instructions for ExpoKit?)

Test Plan

Projects build, went quickly through NCL, looked more or less ok.

@sjchmiela sjchmiela requested review from ide and tsapeta as code owners Aug 31, 2019
@sjchmiela sjchmiela force-pushed the @sjchmiela/upgrade_vendored_modules branch from 7f10683 to f25082b Aug 31, 2019
@ide

This comment has been minimized.

Copy link
Member

commented Aug 31, 2019

I think pointing to https://…/expo/react-native-webview is better. People who don't want this can use 7.0.0 from npm and patch it themselves, but to me, "making my app work" seems more important than "use the npm package".

@ide
ide approved these changes Aug 31, 2019
@brentvatne

This comment has been minimized.

Copy link
Member

commented Sep 5, 2019

i think it's worth updating screens to 2.0.0-alpha1 that @kmagiera released today

@sjchmiela

This comment has been minimized.

Copy link
Contributor Author

commented Sep 5, 2019

True, but doesn't v2 require RN0.60? According to the documentation it does and I don't know whether it's just for AndroidX or some more changes?

@brentvatne

This comment has been minimized.

Copy link
Member

commented Sep 5, 2019

cc @kmagiera halp

@kmagiera

This comment has been minimized.

Copy link

commented Sep 6, 2019

I actually put 0.60 as required version but only thing that’s needed is androidx appcompat@1.1.0

@sjchmiela

This comment has been minimized.

Copy link
Contributor Author

commented Sep 6, 2019

If so, I'll update it and just replace the AndroidX import with a Support one 🙂

@kmagiera

This comment has been minimized.

Copy link

commented Sep 6, 2019

This won't work as I am using features that are only added in androidX appcompat 1.1.0

@kmagiera

This comment has been minimized.

Copy link

commented Sep 6, 2019

latest support library (28) is only compatible with androidx 1.0.0

@sjchmiela

This comment has been minimized.

Copy link
Contributor Author

commented Sep 6, 2019

Ok, thanks for clarifying! Then either we'll merge #5521 in and then update screens to latest or… not.

@kmagiera

This comment has been minimized.

Copy link

commented Sep 6, 2019

I'm using this thing for back button support which has been added to AppCompatActivity only in 1.1.0: https://developer.android.com/reference/androidx/activity/OnBackPressedCallback

@esamelson esamelson referenced this pull request Sep 7, 2019
28 of 28 tasks complete
@esamelson

This comment has been minimized.

Copy link
Member

commented Sep 7, 2019

would prefer to hold off on merging the big AndroidX PR for this release and hold off on updating RN screens if that's necessary. I'd rather not merge something that big this late in the process and increase our QA load.

@tsapeta tsapeta added todo and removed todo labels Sep 9, 2019
@tsapeta tsapeta merged commit 42d9060 into master Sep 9, 2019
3 checks passed
3 checks passed
client Workflow: client
Details
docs Workflow: docs
Details
sdk Workflow: sdk
Details
@tsapeta tsapeta deleted the @sjchmiela/upgrade_vendored_modules branch Sep 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.