-
Notifications
You must be signed in to change notification settings - Fork 311
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
Bump "promise" to ^8.0.3 to match React Native #413
base: main
Are you sure you want to change the base?
Conversation
React Native 0.63 (and by extension Expo SDK 39-40) bumped its dependency on "promise" to be ^8.0.3 ¹ This repo depends on promise@^7.1.1 which has no valid overlapping versions. This module "fbjs" is depended on by "metro", which in turn is depended on by RN/Expo projects. This means that on a standard RN/Expo project, two versions of "promise" get installed, making it impossible to obtain a reference to the same version that React Native is using², and thus impossible to detect unhandled promise rejections – and for example report them to Bugsnag. ¹ facebook/react-native@b23efc5 ² https://github.com/bugsnag/bugsnag-js/blob/eb6e05f350f5acf496c9e3dfd341c9061c400c1f/packages/plugin-react-native-unhandled-rejection/rejection-handler.js#L5
Hi @bengourley! Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. If you have received this in error or have any questions, please contact us at [email protected]. Thanks! |
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks! |
could this PR be merged ? |
Any hope of getting this merged? We just ran into this in our project... |
React Native stopped using fbjs a year ago (facebook/react-native@54e19a6), as did metro (facebook/metro@203fb31). I know RN has done a release or 2 since then so I'd recommend upgrading RN as a better approach. Merging and shipping a new current fbjs release doesn't help, so this needs to be applied to a 3 year old release of fbjs, which may have other consequences. |
Great, so it looks like the fix is to update to a version of RN >= 0.64.0 (which transitively includes metro >= 0.64.0). Thanks! |
@zpao |
@jer-sen react-native-dom is using fbjs but it's usage is limited and doesn't import anything that uses the promise module. What problems are you seeing? I'm not against shipping this just the issue people were having when reported is not relevant anymore. |
@zpao I use So, could you merge this PR ? Also |
Best I can tell, |
You are right. In fact, what happens is that But the version resolved is not the one from I see to possible fixes:
The second one seems cleaner to me. I will suggest Sentry to update there installation doc and add a runtime check. Sorry for the mistake in my previous comment and many thanks for invastigating my issue. |
No worries, module dependency resolution and po{l,n}yfills are a huge mess. It's a totally legit issue much of the time. Like I said, I'm not against updating here at all. I do hesitate to do major bumps of dependencies like this without also doing a major version bump of fbjs itself. That said, the diff (then/promise@7.1.1...8.0.3) is essentially just adding typedefs for flow and typescript and updating an error message, so it's probably fine. 8.1.0 has some new support but for this case, wouldn't jump to there in a minor version of fbjs. |
Thoughts on landing this? Just spent a couple of hours debugging a maximum call stack error that in the end was resolved by removing our |
tl;dr out of sync dependencies on "promise" in React Native vs. fbjs mean that two different versions get installed on a project, and unhandled promise rejections do not get reported in Bugsnag on RN 0.63+ and Expo SDK39+
React Native 0.63 (and by extension Expo SDK 39-40) bumped its dependency on "promise" to be ^8.0.3 ¹
This repo depends on promise@^7.1.1 which has no valid overlapping versions. This module "fbjs" is depended on by "metro", which in turn is depended on by RN/Expo projects.
This means that on a standard RN/Expo project, two versions of "promise" get installed, making it impossible to obtain a reference to the same version that React Native is using², and thus impossible to detect unhandled promise rejections – and for example report them to Bugsnag.
¹ facebook/react-native@b23efc5
² https://github.com/bugsnag/bugsnag-js/blob/eb6e05f350f5acf496c9e3dfd341c9061c400c1f/packages/plugin-react-native-unhandled-rejection/rejection-handler.js#L5