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.

Sign up for GitHub

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

Jump to bottom

feat: Add exclude Web Replay option to Metro plugin #4006

Merged
merged 40 commits into from
Sep 10, 2024
Merged

feat: Add exclude Web Replay option to Metro plugin #4006

merged 40 commits into from
Sep 10, 2024

Conversation

lucas-zimerman
Copy link
Collaborator

@lucas-zimerman lucas-zimerman commented Aug 9, 2024 •
edited
Loading

📢 Type of change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring

📜 Description

  • This change will remove the Replay package that is not used by React Native, while allowing users to enable it if they prefer. By default it will be enabled on Web projects, but users can still select to remove Web Replay on it.

  • I also added a .gitignore to .yarn because it is where yarn was caching the packages for me.

💡 Motivation and Context

Close #3998

💚 How did you test it?

  • Creating a new sample.
  • Checking if the Sentry and user resolver is called.
  • Testing if the SDK is behaving normally without Sentry Web Replay.
  • Checking the final package with and without replay to compare the size.

📝 Checklist

  • I reviewed submitted code
  • I added tests to verify changes
  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled
  • All tests passing
  • No breaking changes

🔮 Next steps

Before:
image

After:
image

How to visualize the changse:

On the sample app:
npx react-native bundle --platform android --dev false --entry-file index.js --bundle-output ./build/index.android.bundle --assets-dest ./build/ --sourcemap-output ./build/index.android.bundle.map

and then
npx source-map-explorer ./build/index.android2.bundle ./build/index.android2.bundle.map --html ./build/bundle2-analysis.html

@lucas-zimerman lucas-zimerman changed the title Feat: Allow users to exclude Replay package from Web Fix: Exclude Replay package from Web Aug 9, 2024
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Aug 9, 2024 •
edited
Loading

iOS (legacy) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1219.02 ms 1231.44 ms 12.42 ms
Size 2.36 MiB 3.08 MiB 733.94 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
dadc233+dirty 1223.20 ms 1236.88 ms 13.68 ms
5571a20+dirty 1203.57 ms 1204.57 ms 1.00 ms
70e6261+dirty 1220.09 ms 1230.04 ms 9.95 ms
62a750b+dirty 1216.60 ms 1229.14 ms 12.54 ms
acadc0f+dirty 1264.38 ms 1290.06 ms 25.68 ms
6e8584e+dirty 1274.50 ms 1296.82 ms 22.32 ms
c398f67+dirty 1219.67 ms 1225.66 ms 5.99 ms
86d6d2c+dirty 1267.55 ms 1286.21 ms 18.66 ms
e5c9b8b+dirty 1258.57 ms 1267.32 ms 8.75 ms
31fcca2+dirty 1209.17 ms 1216.21 ms 7.04 ms

App size

Revision Plain With Sentry Diff
dadc233+dirty 2.36 MiB 2.84 MiB 486.85 KiB
5571a20+dirty 2.36 MiB 2.92 MiB 569.93 KiB
70e6261+dirty 2.36 MiB 3.03 MiB 680.42 KiB
62a750b+dirty 2.36 MiB 2.92 MiB 570.00 KiB
acadc0f+dirty 2.36 MiB 2.83 MiB 480.37 KiB
6e8584e+dirty 2.36 MiB 2.88 MiB 533.17 KiB
c398f67+dirty 2.36 MiB 3.04 MiB 696.27 KiB
86d6d2c+dirty 2.36 MiB 2.82 MiB 462.82 KiB
e5c9b8b+dirty 2.36 MiB 2.87 MiB 520.43 KiB
31fcca2+dirty 2.36 MiB 2.90 MiB 552.95 KiB

Previous results on branch: ref/exclude-replay

Startup times

Revision Plain With Sentry Diff
5df1cc4+dirty 1240.57 ms 1238.33 ms -2.24 ms
b200c1f+dirty 1222.66 ms 1232.83 ms 10.17 ms

App size

Revision Plain With Sentry Diff
5df1cc4+dirty 2.36 MiB 3.08 MiB 733.93 KiB
b200c1f+dirty 2.36 MiB 3.08 MiB 733.91 KiB

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Aug 9, 2024 •
edited
Loading

Android (legacy) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 462.29 ms 476.35 ms 14.06 ms
Size 17.73 MiB 20.06 MiB 2.33 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
52a8031+dirty 311.55 ms 321.37 ms 9.82 ms
86d6d2c+dirty 332.90 ms 352.45 ms 19.55 ms
0677344 327.74 ms 337.14 ms 9.40 ms
80b2ce3 385.02 ms 387.36 ms 2.34 ms
dadc233+dirty 333.78 ms 343.94 ms 10.16 ms
d197b5c+dirty 338.94 ms 354.87 ms 15.93 ms
15c80ab+dirty 336.27 ms 350.58 ms 14.31 ms
baa882f 354.93 ms 356.18 ms 1.25 ms
9433f35 347.64 ms 356.22 ms 8.58 ms
5bb8d5f 431.21 ms 459.40 ms 28.19 ms

App size

Revision Plain With Sentry Diff
52a8031+dirty 17.73 MiB 20.04 MiB 2.31 MiB
86d6d2c+dirty 17.73 MiB 20.04 MiB 2.31 MiB
0677344 17.73 MiB 19.81 MiB 2.07 MiB
80b2ce3 17.73 MiB 19.75 MiB 2.02 MiB
dadc233+dirty 17.73 MiB 19.75 MiB 2.02 MiB
d197b5c+dirty 17.73 MiB 20.04 MiB 2.31 MiB
15c80ab+dirty 17.73 MiB 20.04 MiB 2.31 MiB
baa882f 17.73 MiB 20.06 MiB 2.33 MiB
9433f35 17.73 MiB 19.81 MiB 2.08 MiB
5bb8d5f 17.73 MiB 19.93 MiB 2.20 MiB

Previous results on branch: ref/exclude-replay

Startup times

Revision Plain With Sentry Diff
5df1cc4 441.00 ms 447.46 ms 6.46 ms
b200c1f 487.37 ms 492.76 ms 5.39 ms

App size

Revision Plain With Sentry Diff
5df1cc4 17.73 MiB 20.06 MiB 2.33 MiB
b200c1f 17.73 MiB 20.06 MiB 2.33 MiB

krystofwoldrich
krystofwoldrich reviewed Aug 9, 2024
View reviewed changes
CHANGELOG.md Outdated Show resolved Hide resolved
@krystofwoldrich
Copy link
Member

Awesome size reduction, thank you!

I left some small suggestions/nits to changelog. And one comment about behaviour for rn-web.

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Aug 15, 2024 •
edited
Loading

iOS (new) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1239.09 ms 1237.54 ms -1.54 ms
Size 2.92 MiB 3.64 MiB 739.97 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
dadc233+dirty 1266.52 ms 1282.55 ms 16.03 ms
5571a20+dirty 1228.09 ms 1233.45 ms 5.36 ms
70e6261+dirty 1224.90 ms 1231.02 ms 6.12 ms
62a750b+dirty 1228.12 ms 1230.53 ms 2.41 ms
acadc0f+dirty 1271.12 ms 1272.28 ms 1.16 ms
6e8584e+dirty 1271.71 ms 1281.26 ms 9.55 ms
c398f67+dirty 1227.31 ms 1230.00 ms 2.69 ms
86d6d2c+dirty 1291.62 ms 1296.80 ms 5.18 ms
e5c9b8b+dirty 1276.90 ms 1280.92 ms 4.02 ms
31fcca2+dirty 1222.04 ms 1226.51 ms 4.47 ms

App size

Revision Plain With Sentry Diff
dadc233+dirty 2.92 MiB 3.40 MiB 492.53 KiB
5571a20+dirty 2.92 MiB 3.48 MiB 575.54 KiB
70e6261+dirty 2.92 MiB 3.59 MiB 686.11 KiB
62a750b+dirty 2.92 MiB 3.48 MiB 575.59 KiB
acadc0f+dirty 2.92 MiB 3.39 MiB 487.34 KiB
6e8584e+dirty 2.92 MiB 3.44 MiB 536.52 KiB
c398f67+dirty 2.92 MiB 3.60 MiB 701.89 KiB
86d6d2c+dirty 2.92 MiB 3.37 MiB 464.31 KiB
e5c9b8b+dirty 2.92 MiB 3.43 MiB 524.50 KiB
31fcca2+dirty 2.92 MiB 3.46 MiB 557.31 KiB

Previous results on branch: ref/exclude-replay

Startup times

Revision Plain With Sentry Diff
5df1cc4+dirty 1239.27 ms 1240.08 ms 0.82 ms
b200c1f+dirty 1233.02 ms 1236.43 ms 3.41 ms

App size

Revision Plain With Sentry Diff
5df1cc4+dirty 2.92 MiB 3.64 MiB 739.99 KiB
b200c1f+dirty 2.92 MiB 3.64 MiB 740.03 KiB

@lucas-zimerman lucas-zimerman marked this pull request as ready for review August 15, 2024 02:54
@lucas-zimerman lucas-zimerman changed the title Fix: Exclude Replay package from Web Fix: Exclude Web Replay package from Mobile by default Aug 15, 2024
@krystofwoldrich krystofwoldrich mentioned this pull request Sep 5, 2024
Open
@krystofwoldrich krystofwoldrich changed the title feature: Exclude Web Replay package from Mobile by default feat: Add exclude Web Replay option to Metro plugin Sep 6, 2024
krystofwoldrich
krystofwoldrich approved these changes Sep 6, 2024
View reviewed changes
Copy link
Member

@krystofwoldrich krystofwoldrich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, this was more complex than expected, due to the older Metro versions.

Looks great now!

@lucas-zimerman lucas-zimerman merged commit 37edf26 into main Sep 10, 2024
62 checks passed
@lucas-zimerman lucas-zimerman deleted the ref/exclude-replay branch September 10, 2024 15:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@krystofwoldrich krystofwoldrich krystofwoldrich approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@lucas-zimerman @krystofwoldrich