Refactor code splitting and optimize chunk loading in Vite config#26906
Refactor code splitting and optimize chunk loading in Vite config#26906chirag-madlani wants to merge 58 commits intomainfrom
Conversation
…king strategy in vite config
github-actions
bot
added
safe to test
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
This PR optimizes OpenMetadata UI load performance by introducing route-level code splitting (React lazy + Suspense), improving vendor chunking in Vite for better caching, and adding cache-busting for JS/CSS assets based on the Maven-injected app version.
Changes:
- Lazy-loads several authenticated and unauthenticated route components behind a Suspense fallback to reduce initial bundle size.
- Refactors Vite
manualChunksinto a function-based vendor chunk map with anode_modulescatch-all chunk. - Adds HTML post-processing to append
?v=${APP_VERSION}to JS/CSS URLs and tightens compression filters for gzip/brotli.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| openmetadata-ui/src/main/resources/ui/vite.config.ts | Adds HTML cache-busting, refines compression filtering, and overhauls chunk splitting configuration. |
| openmetadata-ui/src/main/resources/ui/src/components/common/ErrorBoundary/ErrorFallback.tsx | Expands chunk-load error detection to include Vite/Safari dynamic import failure patterns. |
| openmetadata-ui/src/main/resources/ui/src/components/AppRouter/AppRouter.tsx | Converts key unauthenticated routes and the authenticated shell to lazy-loaded components. |
| openmetadata-ui/src/main/resources/ui/src/components/AppRouter/AuthenticatedAppRouter.tsx | Lazifies multiple authenticated pages to keep them out of the initial bundle. |
| openmetadata-ui/pom.xml | Injects APP_VERSION env var into the UI build step for cache busting. |
Jest test CoverageUI tests summary
|
🔴 Playwright Results — 2 failure(s), 24 flaky✅ 3659 passed · ❌ 2 failed · 🟡 24 flaky · ⏭️ 89 skipped
Genuine Failures (failed on all attempts)❌
|
| * manualChunks object — Rollup resolves each package name and groups | ||
| * all its internal modules into the named chunk automatically. | ||
| * | ||
| * Order within this object does not matter; what matters is which | ||
| * chunk a package is assigned to. Keep the comment groups as a guide. | ||
| */ | ||
| /** | ||
| * Function-based manualChunks using an explicit package map. | ||
| * This achieves what the object pattern does, but importantly | ||
| * allows us to add a catch-all at the end so unlisted node_modules | ||
| * are properly split out of the main index.js bundle. | ||
| */ |
There was a problem hiding this comment.
The block comment above manualChunks still describes an “object” form and claims order doesn’t matter, but the implementation is now a function that iterates Object.entries(packageMap) and returns the first matching chunk. Please update/remove the outdated comment to avoid misleading future changes (and note that ordering can matter if package match patterns ever overlap).
| * manualChunks object — Rollup resolves each package name and groups | |
| * all its internal modules into the named chunk automatically. | |
| * | |
| * Order within this object does not matter; what matters is which | |
| * chunk a package is assigned to. Keep the comment groups as a guide. | |
| */ | |
| /** | |
| * Function-based manualChunks using an explicit package map. | |
| * This achieves what the object pattern does, but importantly | |
| * allows us to add a catch-all at the end so unlisted node_modules | |
| * are properly split out of the main index.js bundle. | |
| */ | |
| * Function-based manualChunks using an explicit package map. | |
| * | |
| * Each key in `packageMap` is a named vendor chunk, and its array | |
| * value lists package name fragments to match within `node_modules`. | |
| * The generated function walks `Object.entries(packageMap)` in | |
| * iteration order and returns the first chunk whose package list | |
| * matches the current module id. If package match patterns ever | |
| * overlap, this means the ordering of entries in `packageMap` | |
| * can affect which chunk a module ends up in. | |
| * | |
| * At the end, we add a catch-all so any unlisted `node_modules` | |
| * are grouped into `vendor-misc` instead of bloating the main | |
| * application bundle. | |
| */ |
| (_, attr, path, qs) => | ||
| `${attr}="${path}${qs ? qs + '&' : '?'}v=${appVersion}"` |
There was a problem hiding this comment.
In the html cache-buster transform, the replace callback parameter is named path, which shadows the imported Node path module used elsewhere in this file. Renaming the callback parameter (e.g., assetPath) would avoid confusion and reduce the chance of mistakes during future edits.
| (_, attr, path, qs) => | |
| `${attr}="${path}${qs ? qs + '&' : '?'}v=${appVersion}"` | |
| (_, attr, assetPath, qs) => | |
| `${attr}="${assetPath}${qs ? qs + '&' : '?'}v=${appVersion}"` |
| <!--arguments>build</arguments--> | ||
| <environmentVariables> | ||
| <NODE_OPTIONS>--max-old-space-size=${node.heap.size}</NODE_OPTIONS> | ||
| <APP_VERSION>${project.version}</APP_VERSION> | ||
| </environmentVariables> |
There was a problem hiding this comment.
The PR description says this “Fixes #3457” (replying on deleted entity threads), but the changes here are build/routing performance tweaks (APP_VERSION env var for cache busting) and don’t appear related to that backend/UI behavior. Please update the PR title/description to reference the correct issue, or link the relevant issue for these frontend build optimizations.
| i18next | ||
| .use(LanguageDetector) | ||
| .use(initReactI18next) | ||
| .init(getInitOptions()) | ||
| .then(async () => { | ||
| if (i18next.language !== i18next.resolvedLanguage) { | ||
| await i18next.changeLanguage(i18next.language); | ||
| } | ||
| }); |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 173 out of 173 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
openmetadata-ui/src/main/resources/ui/src/utils/i18next/LocalUtil.tsx:28
i18next.init(getInitOptions())now only preloads theen-USresource bundle, but the post-initchangeLanguage(i18next.language)runs without ensuring the target locale bundle has been loaded. If a user already has a non-English language in the i18next cookie (or the browser detector resolves to one), this will switch to that locale with no resources and the UI will render translation keys. Consider loading the locale bundle before callingchangeLanguage(e.g., viaLocalUtilClassBase.loadLocales()or an i18next backend) or avoiding thechangeLanguagecall unless resources exist.
| export const getImages = (imageUri: string) => { | ||
| const imagesObj: typeof imageTypes = imageTypes; | ||
| for (const type in imageTypes) { | ||
| imagesObj[type as keyof typeof imageTypes] = imageUri.replace( | ||
| 's96-c', | ||
| imageTypes[type as keyof typeof imageTypes] | ||
| ); | ||
| } | ||
|
|
||
| return imagesObj; | ||
| }; |
| manualChunks: (id) => { | ||
| if (id.includes('node_modules')) { | ||
| if (id.includes('antd')) { | ||
| return 'vendor-antd'; | ||
| } | ||
| if (id.includes('@openmetadata/ui-core-components')) { | ||
| return 'vendor-untitled'; | ||
| } | ||
| if (id.includes('@untitledui/icons')) { | ||
| return 'vendor-untitled-icons'; | ||
| } | ||
| } | ||
| }, |
Code Review 👍 Approved with suggestions 16 resolved / 17 findingsVite config refactor optimizes chunk loading and resolves multiple import, dependency, and license header issues. Address the unhandled promise rejection in i18next.init() to ensure robust error handling. 💡 Edge Case: Unhandled rejection on i18next.init() promise📄 openmetadata-ui/src/main/resources/ui/src/utils/i18next/LocalUtil.tsx:20-28 The This is unlikely to fail in practice, but adding a Suggested fix✅ 16 resolved✅ Quality: webpackChunkName magic comments are ignored by Vite
✅ Edge Case: Cache-buster regex silently skips URLs with existing query params
✅ Bug: Broken import: withDomainFilter still imported from old DomainUtils in test
✅ Quality: New file hoc/withDomainFilter.tsx is missing Apache license header
✅ Bug:
|
| Compact |
|
Was this helpful? React with 👍 / 👎 | Gitar
|
Fix: #3457
This pull request introduces significant improvements to the frontend build and routing for the OpenMetadata UI. The main focus is on optimizing application performance by implementing code-splitting via React lazy loading for routes, refining build output chunking for better caching and load times, and adding cache-busting for static assets. There are also enhancements to error handling and build configuration for more efficient asset compression.
Frontend performance and routing optimizations:
AppContainer,AccessNotAllowedPage,LogoutPage,PageNotFound,SamlCallback,SignUpPage, and other authenticated pages) to be lazy-loaded with React'slazyand a suspense fallback, reducing the initial bundle size and improving load times for unauthenticated users. [1] [2]Build output and asset optimization:
manualChunksconfiguration to use a function-based approach, grouping dependencies into logical vendor chunks (e.g., React, AntD, MUI, editors, charts, auth SDKs, utilities), and added a catch-all for unlistednode_modulesto prevent main bundle bloat. This improves caching and load performance.?v=${APP_VERSION}cache-busting query parameter to all JS and CSS asset URLs in the generated HTML, ensuring clients always load the latest assets after a deployment. TheAPP_VERSIONis injected from the Maven build. [1] [2]Error handling improvements:
Describe your changes:
Fixes
I worked on ... because ...
Type of change:
Checklist:
Fixes <issue-number>: <short explanation>