CAT-1335: tree shake phosphor icons#16338
Conversation
|
🔴 Meticulous spotted visual differences in 255 of 1652 screens tested: view and approve differences detected. Meticulous evaluated ~9 hours of user flows against your PR. Last updated for commit d9df3a9. This comment will update as new commits are pushed. |
Bundle ReportChanges will decrease total bundle size by 5.83MB (-19.7%) ⬇️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: datahub-react-web-esmAssets Changed:
Files in
|
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
|
@benjiaming I scanned through meticulous and here are the classes of errors I found
Claude can probably audit based on some of the surfaces mentioned below: |
| return FilePdf; | ||
| case 'doc': | ||
| case 'docx': | ||
| return 'FileWord'; |
There was a problem hiding this comment.
I think you find a legit case of missing icons
8ee3a46 to
4fabdbf
Compare
4fabdbf to
67c42a8
Compare
annadoesdesign
left a comment
There was a problem hiding this comment.
approving with the caveat carets will be updated sizing
|
Take a peek at the meticulous run for what Anna reject and please address before merging |
Addressing those as fast follow. |
Align with tree-shaking changes from #16338 — Icon component now accepts React.ComponentType instead of string names with source prop. Made-with: Cursor
Update Phosphor icon usage in group identity components to pass direct component references instead of string literals, aligning with the new Icon API from #16338. Made-with: Cursor
Update Phosphor icon usage in ownership components to pass direct component references instead of string literals, aligning with the new Icon API from #16338. Made-with: Cursor
…r dependency Merge master (includes tree-shaking PR #16338). Resolve conflict in ActorsSearchSelect by keeping dataTestId prop and adopting master's DEFAULT_ACTOR_TYPES rename. Fix FileIcon.tsx circular dependency that caused GlossaryTermHeader tests to fail — replace Icon wrapper with direct Phosphor component usage to avoid barrel import cycle. Made-with: Cursor













Summary: Phase 1 of FE performance optimizations
The DataHub frontend currently produces around 20MB of minified JavaScript across 5 chunks, compressing to approximately 5 MB when gzipped. This is a substantial bundle size that impacts initial load time and performance, particularly on slower networks. Phase 1 addresses some of the issues:
vite.config.tsantdto chunksResults
Before
index-B3Suu7YI.js= 19,395 KB (~19 MB)After
vendor-yJ7kOkOV.js: 8.6 MB (8,999.72 KB) [KEY CHUNK]
source-CbhsHxku.js: 3.9 MB (4,042.65 KB)
phosphor-vendor-D8fS90Jx.js: 372 KB (408.51 KB) [OPTIMIZED]
index-CxYYPpoy.js: 4 KB (0.74 KB)
Total Size:
Bundle Analyzer
rollup-plugin-visualizeinstalled as a dev dependencyyarn build:analyzeandopen dist/stats.htmlRisks
→ Mitigation:
npm run validate-bundle→ Mitigation: added
datahub-web-react/src/__tests__/bundle-optimization.test.tsReference
https://www.notion.so/acryldata/DataHub-Frontend-Performance-Tech-Plan-246fc6a64277803e9976fdde565602d8