[No QA] Remove Performance.tsx (replaced by Sentry)#83114
[No QA] Remove Performance.tsx (replaced by Sentry)#83114roryabraham wants to merge 7 commits intomainfrom
Conversation
The custom Performance module built on react-native-performance has been fully replaced by Sentry spans for telemetry. This removes the module and all its consumers: - Delete src/libs/Performance.tsx, src/libs/Metrics/, and the react-native-performance type declarations - Remove all Performance.markStart/markEnd/withRenderTrace/measureTTI calls from ~20 consumer files - Uninstall the react-native-performance npm package - Remove all dead CONST.TIMING string constants that were only used as performance mark identifiers (keep numeric debounce/delay values) Co-authored-by: Cursor <cursoragent@cursor.com>
The jest.mock('@libs/Performance', ...) in OnyxUpdatesSpanTest.ts
references the now-deleted Performance.tsx, causing the test suite
to fail with "Cannot find module".
Co-authored-by: Cursor <cursoragent@cursor.com>
The react-native-performance package was uninstalled, but MainActivity.kt and MainApplication.kt still imported and used RNPerformance, breaking the Android build. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
index.native.ts and index.ts were both no-ops after removing Performance. Move the web implementation from index.website.ts into index.ts and delete the .website variant. Co-authored-by: Cursor <cursoragent@cursor.com>
|
@codex review |
|
🚧 @roryabraham has triggered a test Expensify/App build. You can view the workflow run here. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2158bbeaaf
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| AppRegistry.runApplication(Config.APP_NAME, { | ||
| rootTag: document.getElementById('root'), | ||
| }); |
There was a problem hiding this comment.
Keep browser bootstrap out of shared platformSetup entry
Moving the web bootstrap logic into platformSetup/index.ts means every consumer that resolves the default .ts file now runs AppRegistry.runApplication(...) and DateUtils.startCurrentDateUpdater() instead of the previous no-op; this is a regression for non-website contexts (notably test setup paths that call @src/setup) and can crash or leak timers when no real web root is mounted. The web-only implementation was previously isolated in index.website.ts, so this change introduces cross-environment side effects rather than a pure telemetry cleanup.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
I think if tests pass it's all good. Common convention is that when there's a web and native implementation web gets index.ts and native gets index.native.ts
|
@DylanDylann Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
Explanation of Change
The custom
Performancemodule (built onreact-native-performance) has been fully replaced by Sentry spans for telemetry. This PR removes the module and all its consumers:src/libs/Performance.tsx,src/libs/Metrics/(web + native variants + types),src/types/modules/react-native-performance.d.tsPerformance.*calls (markStart,markEnd,withRenderTrace,measureTTI,enableMonitoring) from ~20 consumer files. Sentry spans (startSpan/endSpan) that already cover these measurements are left in place.react-native-performancenpm packageCONST.TIMINGstring constants that were only used as performance mark identifiers (numeric debounce/delay values are kept)Fixed Issues
$ #80358
Tests
There's not really any manual test steps here since this just removes dead code. Just did a basic smoke test.
Offline tests
N/A - No behavioral changes; this only removes dead telemetry code.
QA Steps
[No QA] - Pure dead code removal with no behavioral changes. Sentry telemetry is unaffected.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
N/A - No UI changes
Android: mWeb Chrome
N/A - No UI changes
iOS: Native
N/A - No UI changes
iOS: mWeb Safari
N/A - No UI changes
MacOS: Chrome / Safari
Basic smoke tests pass: