-
-
Notifications
You must be signed in to change notification settings - Fork 354
feat: Add Sentry Babel Transformer #3916
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
feat: Add Sentry Babel Transformer #3916
Conversation
|
iOS (legacy) Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 063bfce+dirty | 1224.27 ms | 1219.66 ms | -4.61 ms |
| 41db11d+dirty | 1207.36 ms | 1210.32 ms | 2.96 ms |
| 376301c+dirty | 1215.73 ms | 1219.80 ms | 4.06 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 063bfce+dirty | 2.36 MiB | 3.05 MiB | 702.78 KiB |
| 41db11d+dirty | 2.36 MiB | 3.04 MiB | 698.69 KiB |
| 376301c+dirty | 2.36 MiB | 3.05 MiB | 702.83 KiB |
Previous results on branch: kw/add-sentry-babel-transaformer
Startup times
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| fd77856+dirty | 1230.60 ms | 1222.46 ms | -8.15 ms |
| 3ae09f5+dirty | 1219.96 ms | 1217.64 ms | -2.32 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| fd77856+dirty | 2.36 MiB | 3.05 MiB | 702.80 KiB |
| 3ae09f5+dirty | 2.36 MiB | 3.05 MiB | 702.85 KiB |
iOS (new) Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 063bfce+dirty | 1225.38 ms | 1218.06 ms | -7.31 ms |
| 41db11d+dirty | 1208.60 ms | 1210.47 ms | 1.87 ms |
| 376301c+dirty | 1224.74 ms | 1227.00 ms | 2.26 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 063bfce+dirty | 2.92 MiB | 3.61 MiB | 710.22 KiB |
| 41db11d+dirty | 2.92 MiB | 3.61 MiB | 705.84 KiB |
| 376301c+dirty | 2.92 MiB | 3.61 MiB | 709.95 KiB |
Previous results on branch: kw/add-sentry-babel-transaformer
Startup times
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| fd77856+dirty | 1245.06 ms | 1254.37 ms | 9.30 ms |
| 3ae09f5+dirty | 1228.45 ms | 1236.63 ms | 8.18 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| fd77856+dirty | 2.92 MiB | 3.61 MiB | 710.01 KiB |
| 3ae09f5+dirty | 2.92 MiB | 3.61 MiB | 709.96 KiB |
Android (new) Performance metrics 🚀
|
Android (legacy) Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 063bfce | 469.96 ms | 516.38 ms | 46.42 ms |
| 41db11d | 429.33 ms | 451.24 ms | 21.91 ms |
| 376301c | 445.52 ms | 474.70 ms | 29.18 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 063bfce | 17.73 MiB | 20.04 MiB | 2.30 MiB |
| 41db11d | 17.73 MiB | 20.04 MiB | 2.30 MiB |
| 376301c | 17.73 MiB | 20.04 MiB | 2.30 MiB |
vaind
left a comment
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.
This goes well beyond my understanding of the ecosystem but the I approve of the idea, because it's exactly what I wanted to do at first, I just didn't know how.
3c0b4ba to
d31895e
Compare
lforst
left a comment
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.
Take with a grain of salt because I am no expert but nothing in this PR looks completely off to me.
| */ | ||
| export function withSentryBabelTransformer(config: MetroConfig): MetroConfig { | ||
| const defaultBabelTransformerPath = config.transformer && config.transformer.babelTransformerPath; | ||
| logger.debug('Default Babel transformer path from `config.transformer`:', defaultBabelTransformerPath); |
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 Sentry SDK logger is mostly intended to be used when the SDK is initialized. For build tooling we usually don't rely on it. I would do a good ol' console.log instead.
lucas-zimerman
left a comment
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.
Looks good to me from my part, LGTM!
|
Thanks for expanding support to react native! This looks great |
* feat(replay): Add Mobile Replay Alpha (#3714) * feat(sample): add running indicator (animation overlay) (#3903) * feat(replay): Add breadcrumbs mapping from RN to RRWeb format (#3846) * feat(replay): Add network breadcrumbs (#3912) * fix(replay): Add tests for touch events (#3924) * feat(replay): Filter Sentry event breadcrumbs (#3925) * fix(changelog): Add latest native SDKs details * release: 5.25.0-alpha.2 * misc(samples): Add console anything examples for replay testing (#3928) * feat: Add Sentry Babel Transformer (#3916) * fix(replay): Add app lifecycle breadcrumbs conversion tests (#3932) * chore(deps): bump sentry-android to 7.12.0-alpha.3 * chore(deps): bump sentry-android to 7.12.0-alpha.4 * fix(replay): Mask SVGs from `react-native-svg` when `maskAllVectors=true` (#3930) * fix(replay): Add missing properties to android nav breadcrumbs (#3942) * release: 5.26.0-alpha.3 * misc(replay): Add Mobile Replay Public Beta changelog (#3943) --------- Co-authored-by: Ivan Dlugos <[email protected]> Co-authored-by: Ivan Dlugos <[email protected]> Co-authored-by: getsentry-bot <[email protected]> Co-authored-by: getsentry-bot <[email protected]> Co-authored-by: Roman Zavarnitsyn <[email protected]> Co-authored-by: Bruno Garcia <[email protected]>
📢 Type of change
📜 Description
This PR adds
@sentry/babel-plugin-component-annotateto@sentry/react-native/metro.Example use RN 0.72 and newer:
For RN 0.65 to 0.71 (these version did not have default config in metro.config.js but applied it later, that changed in 0.72):
💡 Motivation and Context
@sentry/babel-plugin-component-annotateusingwithSentry#3913💚 How did you test it?
sample apps, e2e tests, unit tests
📝 Checklist
sendDefaultPIIis enabled🔮 Next steps