-
-
Notifications
You must be signed in to change notification settings - Fork 344
chore(rn-sample): Upgrade to React Native 0.79.2 #4806
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
Conversation
Android (legacy) Performance metrics 🚀
|
Revision | Plain | With Sentry | Diff |
---|---|---|---|
e540498 | 436.26 ms | 433.00 ms | -3.26 ms |
00354a7 | 416.40 ms | 433.44 ms | 17.04 ms |
4ed9c54 | 446.27 ms | 433.56 ms | -12.71 ms |
9cab16b | 463.30 ms | 455.06 ms | -8.24 ms |
8ae23a7 | 526.83 ms | 513.38 ms | -13.45 ms |
d2c32bb | 448.85 ms | 450.19 ms | 1.34 ms |
6e8584e | 447.10 ms | 474.71 ms | 27.61 ms |
e1e6bc7 | 479.96 ms | 482.58 ms | 2.62 ms |
d16beca | 448.87 ms | 447.20 ms | -1.67 ms |
5dff5ee | 462.32 ms | 456.26 ms | -6.06 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
e540498 | 17.73 MiB | 20.11 MiB | 2.37 MiB |
00354a7 | 17.75 MiB | 20.13 MiB | 2.38 MiB |
4ed9c54 | 17.75 MiB | 20.12 MiB | 2.37 MiB |
9cab16b | 17.74 MiB | 20.08 MiB | 2.34 MiB |
8ae23a7 | 17.74 MiB | 20.07 MiB | 2.34 MiB |
d2c32bb | 17.74 MiB | 20.08 MiB | 2.34 MiB |
6e8584e | 17.73 MiB | 19.86 MiB | 2.12 MiB |
e1e6bc7 | 17.75 MiB | 20.13 MiB | 2.38 MiB |
d16beca | 17.74 MiB | 20.10 MiB | 2.36 MiB |
5dff5ee | 17.75 MiB | 20.11 MiB | 2.36 MiB |
Previous results on branch: kw-upgrade-to-rn-79
Startup times
Revision | Plain | With Sentry | Diff |
---|---|---|---|
a975711 | 442.43 ms | 458.50 ms | 16.07 ms |
b5c9051 | 466.34 ms | 533.13 ms | 66.78 ms |
e9746f2 | 528.04 ms | 530.57 ms | 2.52 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
a975711 | 17.75 MiB | 20.13 MiB | 2.38 MiB |
b5c9051 | 17.75 MiB | 20.13 MiB | 2.38 MiB |
e9746f2 | 17.75 MiB | 20.13 MiB | 2.38 MiB |
iOS (legacy) Performance metrics 🚀
|
Revision | Plain | With Sentry | Diff |
---|---|---|---|
ad6c299+dirty | 1244.76 ms | 1260.10 ms | 15.34 ms |
34aba08+dirty | 1276.78 ms | 1308.52 ms | 31.74 ms |
ac41368+dirty | 1226.65 ms | 1237.90 ms | 11.24 ms |
f54118b+dirty | 1232.98 ms | 1235.73 ms | 2.76 ms |
db3192f+dirty | 1214.63 ms | 1214.60 ms | -0.03 ms |
ae2f6bc+dirty | 1226.13 ms | 1227.71 ms | 1.58 ms |
80b2ce3+dirty | 1265.92 ms | 1268.60 ms | 2.69 ms |
2dc8f22+dirty | 1224.88 ms | 1212.40 ms | -12.47 ms |
49ea1eb+dirty | 1215.20 ms | 1218.10 ms | 2.90 ms |
1332acb+dirty | 1230.53 ms | 1234.54 ms | 4.01 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
ad6c299+dirty | 2.36 MiB | 2.84 MiB | 488.85 KiB |
34aba08+dirty | 2.36 MiB | 2.85 MiB | 495.32 KiB |
ac41368+dirty | 2.36 MiB | 3.14 MiB | 793.46 KiB |
f54118b+dirty | 2.36 MiB | 3.10 MiB | 757.51 KiB |
db3192f+dirty | 2.63 MiB | 3.76 MiB | 1.13 MiB |
ae2f6bc+dirty | 2.63 MiB | 3.70 MiB | 1.06 MiB |
80b2ce3+dirty | 2.36 MiB | 2.84 MiB | 486.98 KiB |
2dc8f22+dirty | 2.63 MiB | 3.69 MiB | 1.05 MiB |
49ea1eb+dirty | 2.63 MiB | 3.76 MiB | 1.13 MiB |
1332acb+dirty | 2.36 MiB | 3.11 MiB | 759.86 KiB |
Previous results on branch: kw-upgrade-to-rn-79
Startup times
Revision | Plain | With Sentry | Diff |
---|---|---|---|
b5c9051+dirty | 1232.47 ms | 1242.88 ms | 10.41 ms |
a975711+dirty | 1228.02 ms | 1236.17 ms | 8.15 ms |
e9746f2+dirty | 1223.41 ms | 1224.96 ms | 1.55 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
b5c9051+dirty | 2.63 MiB | 3.79 MiB | 1.16 MiB |
a975711+dirty | 2.63 MiB | 3.79 MiB | 1.16 MiB |
e9746f2+dirty | 2.63 MiB | 3.78 MiB | 1.15 MiB |
Android (new) Performance metrics 🚀
|
Revision | Plain | With Sentry | Diff |
---|---|---|---|
9c48b2c+dirty | 270.82 ms | 321.12 ms | 50.30 ms |
ae2f6bc+dirty | 371.94 ms | 390.02 ms | 18.08 ms |
94fc925+dirty | 365.44 ms | 422.74 ms | 57.30 ms |
9f0f6c8+dirty | 365.53 ms | 419.16 ms | 53.63 ms |
3853f43+dirty | 278.12 ms | 338.72 ms | 60.60 ms |
e73d82f+dirty | 377.67 ms | 407.06 ms | 29.39 ms |
e2aa97e+dirty | 375.20 ms | 400.02 ms | 24.82 ms |
80b2ce3+dirty | 271.29 ms | 316.47 ms | 45.18 ms |
d361d38+dirty | 257.72 ms | 318.76 ms | 61.04 ms |
728164b+dirty | 335.93 ms | 342.94 ms | 7.01 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
9c48b2c+dirty | 7.15 MiB | 8.07 MiB | 947.16 KiB |
ae2f6bc+dirty | 7.15 MiB | 8.38 MiB | 1.23 MiB |
94fc925+dirty | 7.15 MiB | 8.38 MiB | 1.23 MiB |
9f0f6c8+dirty | 7.15 MiB | 8.37 MiB | 1.22 MiB |
3853f43+dirty | 7.15 MiB | 8.08 MiB | 959.34 KiB |
e73d82f+dirty | 7.15 MiB | 8.34 MiB | 1.19 MiB |
e2aa97e+dirty | 7.15 MiB | 8.39 MiB | 1.23 MiB |
80b2ce3+dirty | 7.15 MiB | 8.04 MiB | 911.02 KiB |
d361d38+dirty | 7.15 MiB | 8.08 MiB | 959.34 KiB |
728164b+dirty | 7.15 MiB | 8.12 MiB | 997.71 KiB |
Previous results on branch: kw-upgrade-to-rn-79
Startup times
Revision | Plain | With Sentry | Diff |
---|---|---|---|
b5c9051+dirty | 433.11 ms | 446.45 ms | 13.34 ms |
e9746f2+dirty | 392.72 ms | 409.18 ms | 16.46 ms |
a975711+dirty | 450.24 ms | 466.48 ms | 16.23 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
b5c9051+dirty | 7.15 MiB | 8.40 MiB | 1.25 MiB |
e9746f2+dirty | 7.15 MiB | 8.40 MiB | 1.25 MiB |
a975711+dirty | 7.15 MiB | 8.40 MiB | 1.25 MiB |
iOS (new) Performance metrics 🚀
|
Revision | Plain | With Sentry | Diff |
---|---|---|---|
ad6c299+dirty | 1248.50 ms | 1248.88 ms | 0.38 ms |
34aba08+dirty | 1268.58 ms | 1276.80 ms | 8.22 ms |
ac41368+dirty | 1226.69 ms | 1229.96 ms | 3.27 ms |
f54118b+dirty | 1235.42 ms | 1236.80 ms | 1.38 ms |
db3192f+dirty | 1221.33 ms | 1217.61 ms | -3.71 ms |
ae2f6bc+dirty | 1220.82 ms | 1217.93 ms | -2.88 ms |
80b2ce3+dirty | 1245.12 ms | 1262.04 ms | 16.92 ms |
2dc8f22+dirty | 1215.43 ms | 1206.68 ms | -8.75 ms |
49ea1eb+dirty | 1225.63 ms | 1228.55 ms | 2.92 ms |
1332acb+dirty | 1243.98 ms | 1241.12 ms | -2.86 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
ad6c299+dirty | 2.92 MiB | 3.40 MiB | 494.12 KiB |
34aba08+dirty | 2.92 MiB | 3.41 MiB | 499.03 KiB |
ac41368+dirty | 2.92 MiB | 3.69 MiB | 794.29 KiB |
f54118b+dirty | 2.92 MiB | 3.67 MiB | 770.05 KiB |
db3192f+dirty | 3.19 MiB | 4.33 MiB | 1.14 MiB |
ae2f6bc+dirty | 3.19 MiB | 4.26 MiB | 1.08 MiB |
80b2ce3+dirty | 2.92 MiB | 3.40 MiB | 492.75 KiB |
2dc8f22+dirty | 3.19 MiB | 4.25 MiB | 1.06 MiB |
49ea1eb+dirty | 3.19 MiB | 4.33 MiB | 1.14 MiB |
1332acb+dirty | 2.92 MiB | 3.67 MiB | 772.45 KiB |
Previous results on branch: kw-upgrade-to-rn-79
Startup times
Revision | Plain | With Sentry | Diff |
---|---|---|---|
b5c9051+dirty | 1219.27 ms | 1217.79 ms | -1.47 ms |
a975711+dirty | 1215.44 ms | 1235.02 ms | 19.58 ms |
e9746f2+dirty | 1228.36 ms | 1233.96 ms | 5.60 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
b5c9051+dirty | 3.19 MiB | 4.36 MiB | 1.17 MiB |
a975711+dirty | 3.19 MiB | 4.36 MiB | 1.17 MiB |
e9746f2+dirty | 3.19 MiB | 4.35 MiB | 1.16 MiB |
c89ccf1
to
4c4daeb
Compare
I've removed the bump to the RN 0.79 from the core package as it causes issues with out current tests. |
Note that this PR also adds This is required to fix react-native-webview/react-native-webview#3767 in the |
.yarnrc.yml
Outdated
plugins: | ||
# The plugin source file has to be present in the repository, otherwise it would have to be install | ||
# before running yarn install by the user/ci... | ||
- .yarn/plugins/plugin-after-install.js |
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.
Should we install the plugin in the workflow or commit it to avoid the CI failure?
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.
Ahh, my bad, I've not push the plugin file, when the file is present it should be picked up automatically without installation, which was my goal, as having to install plugin before using yarn install
would be terrible developer experience.
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.
The post install script is working just fine
Running `afterInstall` hook...
lerna notice cli v8.1.8
> sentry-react-native-sample:postinstall
patch-package 8.0.0
Applying patches...
[email protected] ✔
——————————————————————————————————————————————————————————————————————————————————————————————
Lerna (powered by Nx) Successfully ran target postinstall for project sentry-react-native-sample (739ms)
But in regard to the script, we should add a memo in the plugin-after-install script that it is only there due to react-native-webview/react-native-webview#3767
Or we should make an issue to keep track of it and remove it later once the fix is implemented on that library.
Other than that, LGTM!
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.
I noticed a runtime issue while running locally the iOS and Android React Native sample apps.
ERROR Error: Unable to resolve module use-latest-callback from /Users/antonis/git/sentry-react-native/samples/react-native/node_modules/@react-navigation/native/lib/module/NavigationContainer.js: use-latest-callback could not be found within the project or in these directories:
node_modules
../../node_modules
node_modules
../../packages/core/node_modules
4 | import * as React from 'react';
5 | import { I18nManager } from 'react-native';
> 6 | import useLatestCallback from 'use-latest-callback';
| ^
7 | import { LinkingContext } from "./LinkingContext.js";
8 | import { LocaleDirContext } from "./LocaleDirContext.js";
9 | import { DefaultTheme } from "./theming/DefaultTheme.js";
at ModuleResolver.resolveDependency (/Users/antonis/git/sentry-react-native/samples/react-native/node_modules/@react-native/community-cli-plugin/node_modules/metro/src/node-haste/DependencyGraph/ModuleResolution.js:150:15)
at DependencyGraph.resolveDependency (/Users/antonis/git/sentry-react-native/samples/react-native/node_modules/@react-native/community-cli-plugin/node_modules/metro/src/node-haste/DependencyGraph.js:239:43)
at /Users/antonis/git/sentry-react-native/samples/react-native/node_modules/@react-native/community-cli-plugin/node_modules/metro/src/lib/transformHelpers.js:165:21
at resolveDependencies (/Users/antonis/git/sentry-react-native/samples/react-native/node_modules/@react-native/community-cli-plugin/node_modules/metro/src/DeltaBundler/buildSubgraph.js:42:25)
at visit (/Users/antonis/git/sentry-react-native/samples/react-native/node_modules/@react-native/community-cli-plugin/node_modules/metro/src/DeltaBundler/buildSubgraph.js:83:30)
at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
at async Promise.all (index 5)
at async visit (/Users/antonis/git/sentry-react-native/samples/react-native/node_modules/@react-native/community-cli-plugin/node_modules/metro/src/DeltaBundler/buildSubgraph.js:92:5)
at async Promise.all (index 28)
at async visit (/Users/antonis/git/sentry-react-native/samples/react-native/node_modules/@react-native/community-cli-plugin/node_modules/metro/src/DeltaBundler/buildSubgraph.js:92:5)
I'll retry in case it has to do with my local configuration.
Does it happen on any specific action right away when the app is loading? I've not experience this, might be something wrong with the local node_modules as you mentioned. |
It happens right away. I cleaned up everything with |
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.
It happens right away. I cleaned up everything with git clean -fdx and the problem persisted 👀
@krystofwoldrich I retried and it worked 🤷
📢 Type of change
📜 Description
Upgrade the RN Sample app to the latest RN version 0.79.2.
Also upgraded the dependencies which were not compatible with the latest version.
Notable
setimmediate
,RNScreen
,RNSVG
.The AppDelegate changes are taken from RN/RNTester https://github.com/facebook/react-native/blob/8d08845cdabfa32d10311c52c46463fbac085b10/packages/rn-tester/RNTester/AppDelegate.mm#L49 which still uses
.mm
instead ofswift
.The
react-native
inpackages/core
is pinned to version0.78.2
due to issues with global types in^0.79.0
.After this issue facebook/react-native#51104 is fixed we can upgrade to the lates RN package.
💚 How did you test it?
built and ran the sample app locally
ci builds and e2e tests
📝 Checklist
sendDefaultPII
is enabled#skip-changelog