-
Notifications
You must be signed in to change notification settings - Fork 16
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
Fix: Bump react-native version to fix build failures #1040
Conversation
652fc4c
to
a80b874
Compare
Update: using [email protected] seems to fix this and the app runs, @achou could you also test, and we'll see if CI works ok. |
@gmaclennan just tried this on my device and things seemed to work as expected. noticed CI is still failing though (not sure if due to flakiness or something more legitimate) |
Looks like the app is not running/opening on CI at all, so need to fix that before we merge this. |
version we tried to upgrade v8 to was causing issues with Detox tests
Summary of changes I added:
|
New issue to arise: Intl support for relative time. Example error: [Error: [React Intl Error FORMAT_ERROR] Error formatting relative time.
Locale: TypeError: undefined is not an object (evaluating 'RelativeTimeFormat.bind')
MessageID: undefined
Default Message: undefined
Description: undefined
] Affected components:
Solution: Need to polyfill via https://formatjs.io/docs/polyfills/intl-relativetimeformat/ |
for posterity, can't believe i didn't find this sooner but this is why detox wasn't working with the v8 version we tried to upgrade to: |
Created a script to create the polyfill file for Startup time is notably slower, and not sure about bundle size increase, but let's evaluate and discuss as needed |
const languages = require("../src/frontend/languages.json"); | ||
|
||
// For @formatjs/intl-relativetimeformat and @formatjs/intl-locale, which are needed for jsc-intl runtime | ||
buildIntlRelativeTimeFormat(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
at the time of writing this, the output file looks like this:
// This file is automatically generated through scripts/build-intl-polyfills.js. Do not edit this directly!
// If you need to rebuild it, you can run `npm run build:intl-polyfills`
import "@formatjs/intl-locale/polyfill";
import "@formatjs/intl-relativetimeformat/polyfill";
import "@formatjs/intl-relativetimeformat/locale-data/de";
import "@formatjs/intl-relativetimeformat/locale-data/es";
import "@formatjs/intl-relativetimeformat/locale-data/fr";
import "@formatjs/intl-relativetimeformat/locale-data/id";
import "@formatjs/intl-relativetimeformat/locale-data/ja";
import "@formatjs/intl-relativetimeformat/locale-data/km";
import "@formatjs/intl-relativetimeformat/locale-data/my";
import "@formatjs/intl-relativetimeformat/locale-data/ne";
import "@formatjs/intl-relativetimeformat/locale-data/nl";
import "@formatjs/intl-relativetimeformat/locale-data/en";
import "@formatjs/intl-relativetimeformat/locale-data/pt";
import "@formatjs/intl-relativetimeformat/locale-data/si";
import "@formatjs/intl-relativetimeformat/locale-data/sw";
import "@formatjs/intl-relativetimeformat/locale-data/ta";
import "@formatjs/intl-relativetimeformat/locale-data/th";
import "@formatjs/intl-relativetimeformat/locale-data/vi";
Not sure how accurate this is, but |
DO NOT MERGE
On Nov 4th 2022 Facebook pushed a new version of react-native that broke react-native builds for everyone else. The suggested fix is to bump react-native from v0.66.4 to v0.66.5, which is what this PR does.
Fixes: #1039
However, this patch also seems to break react-native-v8, since running the app with this PR results in the following crash: