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

[Snyk] Upgrade react-native-web from 0.14.10 to 0.16.1 #2939

Closed
wants to merge 1 commit into from

Conversation

snyk-bot
Copy link
Contributor

Snyk has created this PR to upgrade react-native-web from 0.14.10 to 0.16.1.

merge advice
ℹ️ Keep your dependencies up-to-date. This makes it easier to fix existing vulnerabilities and to more quickly identify and fix newly disclosed vulnerabilities when they affect your project.


  • The recommended version is 13 versions ahead of your current version.
  • The recommended version was released 21 days ago, on 2021-04-23.
Release notes
Package name: react-native-web
  • 0.16.1 - 2021-04-23

    0.16.1

  • 0.16.0 - 2021-04-20

    0.16 preview release

    This release includes Flow type exports, updates vendored modules, and miscellaneous bug fixes.

    Breaking changes

    • Animated & VirtualizedList have been updated from React Native.
    • Dimensions has changed the source of window dimensions and works more reliably in Safari.
    • NativeEventEmitter no longer inherits from EventEmitter and does not include the removeSubscription method.

    New features

    • Flow types are now exported. Thanks @ comp615
  • 0.15.7 - 2021-04-13

    0.15.7

  • 0.15.6 - 2021-04-07

    0.15.6

  • 0.15.5 - 2021-03-31

    0.15.5

  • 0.15.4 - 2021-03-30

    0.15.4

  • 0.15.3 - 2021-03-29

    0.15.3

  • 0.15.2 - 2021-03-29

    0.15.2

  • 0.15.1 - 2021-03-26
  • 0.15.0 - 2021-02-12
  • 0.14.13 - 2021-02-05
  • 0.14.12 - 2021-02-04
  • 0.14.11 - 2021-01-29
  • 0.14.10 - 2020-12-17
from react-native-web GitHub release notes

Note: You are seeing this because you or someone else with access to this repository has authorized Snyk to open upgrade PRs.

For more information:

🧐 View latest project report

🛠 Adjust upgrade PR settings

🔕 Ignore this dependency or unsubscribe from future upgrade PRs

@snyk-bot snyk-bot requested a review from a team as a code owner May 14, 2021 20:49
@MelvinBot MelvinBot requested review from sketchydroide and removed request for a team May 14, 2021 20:49
@github-actions
Copy link
Contributor

github-actions bot commented May 14, 2021

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@marcaaron
Copy link
Contributor

Animated & VirtualizedList have been updated from React Native

Definitely we should make sure this doesn't screw anything up too badly 🙃

@Luke9389
Copy link
Contributor

👋 Hey, I'm making a pr that should hopefully allow snyk-bot to pass the CLA check after it gets rerun. Once my PR is merged I'll try rerunning this to see if it passes.

@sketchydroide
Copy link
Contributor

will wait for that then @Luke9389

@Luke9389
Copy link
Contributor

Hey @sketchydroide, looks like re-running it doesn't work. It seems to be referencing the commit hash of whatever the most recent master branch had when it was created. You'll likely have to just merge with the failing CLA and then leave a message explaining that (as long as you know this is actually safe to merge in the first place 😄).

The PR I made will prevent this from happening in the future.

@sketchydroide
Copy link
Contributor

@roryabraham and @marcaaron do you think this is safe to merge, what @Luke9389 says makes some sense, but wanted to get a third opinion.

@roryabraham
Copy link
Contributor

@sketchydroide I think it might be worth doing, but we should definitely be careful and do extensive testing. We could always merge it and see if it causes regressions during QA? We could also ask Applause to do some extra exploratory testing for the release including this PR too?

@sketchydroide
Copy link
Contributor

I think asking Applause to do some exploratory testing sounds great.
I think I'll ask QA first if they have the time to do it, or when they will have it, and then schedule the merge for a time where they can do it.

@marcaaron
Copy link
Contributor

I think the merge it and track regressions approach makes sense for a minor version bumps. In this case, we are jumping 2 major versions forward so I think we can afford to be a bit cautious and...

  1. First go through the changelogs and verify we are not introducing any obvious breaking changes from the past major versions. I took a quick look and noticed one or two things that could impact us. It shouldn't take long to cover.

15.0 - https://github.com/necolas/react-native-web/releases/tag/0.15.0
16.0 - https://github.com/necolas/react-native-web/releases/tag/0.16.0

  1. Test locally for a bit
  2. Have Applause test on staging

Thoughts?

@roryabraham
Copy link
Contributor

Oh, this is a big one that I glossed over the first time:

React 17 is a peer dependency.

That means we also would have to bump React up a major version too?

@marcaaron
Copy link
Contributor

Just gonna start dropping things here that I'm finding so we can make an action list...

https://github.com/Expensify/Expensify.cash/blob/0c8dd5a5430c8b97950c79644ec5481084e48f14/src/libs/openURLInNewTab/index.js#L2-L3

The Linking.openURL(url) API now opens the url in new tab.

from v 15.0 notes

@marcaaron
Copy link
Contributor

Pressable has added onHoverIn and onHoverOut props.

This could probably be a follow up but maybe this can simplify our use of the custom Hoverable component ?

@marcaaron
Copy link
Contributor

The accessible prop is deprecated and will be removed in the next minor release. React Native for Web will follow React Native for Windows/macOS in removing this prop. Use focusable instead.

Found one usage of this.

@marcaaron
Copy link
Contributor

Everything else looks pretty much OK and testing well.

@roryabraham
Copy link
Contributor

This could probably be a follow up but maybe this can simplify our use of the custom Hoverable component

I looked into this a bit, and I'm not sure it will. Some context for why I created the Hoverable component is here. I checked the sandbox that reproduces the error with react-native-web 16 and found that the issue wasn't solved.

@marcaaron
Copy link
Contributor

NativeEventEmitter no longer inherits from EventEmitter and does not include the removeSubscription method.

⬆️

change in v 16

Seems like there are some errors related to the EventEmitter changes

Screen Shot 2021-05-28 at 7 49 35 AM

Not too sure what the source is yet looks like react-navigation.

@marcaaron
Copy link
Contributor

Does just seem like the subscription will be removed anyway but the method is deprecated...

https://github.com/necolas/react-native-web/blob/2d9dca42fa67d09de8ff2de323c622b47a820354/packages/react-native-web/src/vendor/react-native/emitter/_EventEmitter.js#L169-L183

but the error will show in the console 🙃

@marcaaron
Copy link
Contributor

Ok I traced this back to here -> https://github.com/react-navigation/react-navigation/blob/a184ce24b3d7a8a37667d3f43ac043fef0231db4/packages/stack/src/views/Stack/CardContainer.tsx#L183

And I think there are maybe two options...

  1. Don't upgrade to the latest major version of react-native-web just yet
  2. Ignore this error and upgrade anyway

@marcaaron
Copy link
Contributor

Gonna close this in favor of #3215

I don't think we should upgrade to 0.16.1 yet, but migrating to the previous major version was relatively painless.

@marcaaron marcaaron closed this May 28, 2021
@marcaaron marcaaron deleted the snyk-upgrade-b968ca05a439516ccc87ecfb0867caa5 branch May 28, 2021 21:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants