Repository navigation
[NoQA] prevent SVG definitions from re-rendering on every commit - #99669
Conversation
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
@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] |
|
@codex review |
|
Codex Review: Didn't find any major issues. Chef's kiss. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Reviewer Checklist
Screenshots/VideosMain BranchScreen.Recording.2026-09-02.at.14.13.35.movCurrent BranchScreen.Recording.2026-09-02.at.14.18.24.mov |
|
@BartekObudzinski Hmmm, I think old code was already memoized by React Compiler automatically??? But just to be sure, could you tell me how you verified the performance impact?
|
|
I run devtools profiler record and measure sentry span on navigate to ManualNavigateToInboxTab flow @DylanDylann |
|
@BartekObudzinski This is the step I used for testing
I tried these steps on both the main branch and the current PR branch (already merged the latest main), but unfortunately I don't see any significant improvement Main BranchScreen.Recording.2026-09-02.at.14.13.35.movCurrent BranchScreen.Recording.2026-09-02.at.14.18.24.mov |
|
Right, it’s not a big perf win. It’s more of a cleanup, it cuts the noise in profiler recordings and shaves a bit off render time @DylanDylann |
|
🚧 JS00001 has triggered a test Expensify/App build. You can view the workflow run here. |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/JS00001 in version: 9.4.69-0 🚀
|
|
🤖 No help site changes required. I did not create a docs PR. Why: This PR is an internal React rendering optimization with no user-facing change. It moves the hidden Nothing in the diff adds, removes, or renames a feature, tab, setting, label, or button, so there is no behavior for What I checked
Help site updates are warranted when a change alters what someone sees or does in the product. This one deliberately keeps that identical, so the existing articles remain accurate as written. @BartekObudzinski, please confirm you agree no help site update is needed here. If you believe some user-facing behavior did change that I missed, reply with the detail and I will draft the docs PR. |
|
🚀 Deployed to production by https://github.com/francoisl in version: 9.4.69-1 🚀
Bundle Size Analysis (Sentry): |
Explanation of Change
SVGDefinitionsProviderwas rendering the hidden<svg><defs>element and the entire appchildrenin a single component. Because the provider sits near the root and wraps the whole app, every root re-render rebuilt the<defs>subtree, including the twoLinearGradientEmptyStateBackgroundgradients. Profiler showed those gradients re-rendering on every commit across the app.This change extracts the
<svg><defs>markup into a separateSVGDefinitionscomponent with no props. React Compiler memoizes it automatically, so the gradients render once and are skipped on subsequent commits whileSVGDefinitionsProvidercontinues to wrapchildrenas before.Performance impact: measured over 5 runs, average commit time dropped from ~612ms to ~527ms (about 14% improvement).

LinearGradientEmptyStateBackgroundno longer re-renders on every commit.Fixed Issues
$ #99844
PROPOSAL:
Tests
Offline tests
N/A
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
Same as tests.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, 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.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
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari