fbjs
fbjs copied to clipboard
facebook
Bump "promise" to ^8.0.3 to match React Native
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.
¹ https://github.com/facebook/react-native/commit/b23efc526446cad53adc3de7465fd8be7309d84e ² 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!
React Native stopped using fbjs a year ago (https://github.com/facebook/react-native/commit/54e19a6b7f217ffc0611e660f2a6b1a8ad14775b), as did metro (https://github.com/facebook/metro/commit/203fb31ec028f9f147bd0a1b689b587a396d9b37). 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 react-native-web still relies on fbjs which cause me trouble. Why can't you merge this PR?
@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 sentry-expo for bug tracking (as a LOT of people using Expo since its the recommanded and only supported bug tracker for managed workflow: https://docs.expo.dev/guides/using-sentry/). It needs to hook on Promise global object to report failed or unhandled promised. To do so it relies on promise module but it needs to use the same version than the one that hooked on Promise global object, otherwise Sentry do not received events. React Native depends on promise: "^8.0.3" but 2 other modules depend inderectly on an other version of promise through fbjs: react-native-web#fbjs#promise & expo#fbemitter#fbjs#promise. If fbjs dependency on promise were bumped to ^8.0.3 there would be only one version in my project and everything would work fine (as it does with this addition to my package.json: "resolutions": { "react-native-web/**/promise": "^8.0.3", "expo/**/promise": "^8.0.3" },).
So, could you merge this PR ? Also expo will have to stop relying on the archived project fbemitter which depends on an old version of fbjs.
Best I can tell, promise is never actually required in any paths used by fbemitter nor react-native-web. Just because the old version is a dependency somewhere doesn't mean it's actually used. Do you have a stack trace or something indicating it is somehow a problem.
You are right. In fact, what happens is that sentry-expo requires promise module to register an event. Cf https://github.com/getsentry/sentry-react-native/blob/e3d11a1b657f1864af41414dced73394f03e9b23/src/js/integrations/reactnativeerrorhandlers.ts#L83
But the version resolved is not the one from react-native at ^8.0.3, it's an other hoisted for react-native-web#fbjs#promise and expo#fbemitter#fbjs#promise at 7.3.1. So the event is not registered on the global Promise object installed by react-native.
I see to possible fixes:
- bumping
promiseversion offbjs=> all modules will then use the same version that will be hoisted - adding a dependency to my project at the same version than
react-native=>react-nativewill use this hoisted version which will be the one resolved bysentry-expo's require
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 (https://github.com/then/promise/compare/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 core-js polyfill of Promise.allSettled and adding a resolutions for promise to force v8 (and disabling Sentry's polyfilling). As with above, v7 from fbjs was the one hoisted and the one @sentry/react-native ended up pulling in.
https://github.com/getsentry/sentry-react-native/pull/1984
