perf(ui): lazy load entity detail components#29179
Conversation
shah-harshit
added
UI
![gitar-bot[bot]](https://avatars.githubusercontent.com/in/827041?s=60&v=4){kind=small}
| import { CurrentState } from 'Models'; | ||
| import { lazy, ReactNode, Suspense } from 'react'; | ||
| import { EntityType, FqnPart } from '../enums/entity.enum'; | ||
| import { isNull } from 'lodash'; |
There was a problem hiding this comment.
💡 Quality: Unused isNull import in EntityDisplayUtils.tsx
EntityDisplayUtils.tsx adds import { isNull } from 'lodash'; but after the refactor the file only contains getServiceLogo, which does not use isNull. This unused import will trigger lint/build warnings (and the project's no-unused rules). Remove it if getServiceLogo does not reference isNull.
Drop the unused lodash import.:
// Remove the unused import if getServiceLogo does not use isNull
-import { isNull } from 'lodash';
- Apply fix
Check the box to apply the fix or reply for a change | Was this helpful? React with 👍 / 👎
Code Review 🚫 Blocked 0 resolved / 2 findingsLazy loading implementation for entity detail components is blocked because the removal of multiple utility functions from EntityDisplayUtils breaks the build. Additionally, remove the unused isNull import in EntityDisplayUtils.tsx. 🚨 Bug: Moved functions still imported from EntityDisplayUtils break build📄 openmetadata-ui/src/main/resources/ui/src/utils/EntityDisplayUtils.tsx This PR removed Confirmed stale imports of removed functions include:
These must be repointed to Re-export the relocated functions from EntityDisplayUtils so existing import sites keep working (callers can be migrated later). Alternatively, update every stale import path to point at EntityDisplayPureUtils.💡 Quality: Unused isNull import in EntityDisplayUtils.tsx📄 openmetadata-ui/src/main/resources/ui/src/utils/EntityDisplayUtils.tsx:14
Drop the unused lodash import.🤖 Prompt for agentsOptionsDisplay: compact → Showing less information. Comment with these commands to change:
Was this helpful? React with 👍 / 👎 | Gitar |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| getTaskDetailPathFromTask, | ||
| getTaskDisplayId, | ||
| GLOSSARY_TASK_ACTION_LIST, | ||
| INCIDENT_TASK_ACTION_LIST, | ||
| isTaskPendingFurtherApproval, | ||
| isTaskTerminalStatus, | ||
| } from '../../../../utils/TaskNavigationUtils'; | ||
| import { | ||
| GLOSSARY_TASK_ACTION_LIST, | ||
| INCIDENT_TASK_ACTION_LIST, | ||
| TASK_ACTION_COMMON_ITEM, | ||
| TASK_ACTION_LIST, | ||
| } from '../../../../utils/TasksUtils'; | ||
| } from '../../../../utils/TaskActionUtils'; | ||
| import { | ||
| fetchOptions, | ||
| generateOptions, | ||
| } from '../../../../utils/TaskAssigneeUtils'; | ||
| import { getNormalizedTaskPayload } from '../../../../utils/TaskPayloadUtils'; |
There was a problem hiding this comment.
TaskTabNew.component.tsx imports from four utility modules (TaskNavigationUtils, TaskActionUtils, TaskAssigneeUtils, TaskPayloadUtils) that do not exist anywhere in the repository. All of the exported symbols being consumed (getTaskDetailPathFromTask, GLOSSARY_TASK_ACTION_LIST, fetchOptions, getNormalizedTaskPayload, etc.) currently live in TasksUtils.ts. The compiler will reject the module resolution immediately, so the entire UI build is broken by this change as written. These new utility files need to be created (or the imports reverted to TasksUtils) before this PR can be merged.
| export const VersionButton = forwardRef< | ||
| HTMLDivElement, | ||
| EntityVersionButtonProps | ||
| >( | ||
| ( | ||
| { version, onVersionSelect, selected, isMajorVersion, className, summary }, | ||
| ref | ||
| ) => { |
There was a problem hiding this comment.
displayName to the extracted forwardRef component so it shows as VersionButton in React DevTools and error stack traces rather than the generic ForwardRef.
| export const VersionButton = forwardRef< | |
| HTMLDivElement, | |
| EntityVersionButtonProps | |
| >( | |
| ( | |
| { version, onVersionSelect, selected, isMajorVersion, className, summary }, | |
| ref | |
| ) => { | |
| export const VersionButton = forwardRef< | |
| HTMLDivElement, | |
| EntityVersionButtonProps | |
| >( | |
| ( | |
| { version, onVersionSelect, selected, isMajorVersion, className, summary }, | |
| ref | |
| ) => { | |
| VersionButton.displayName = 'VersionButton'; |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
1 participant
Summary
Testing
Ref: https://github.com/open-metadata/openmetadata-collate/issues/4230
Greptile Summary
This PR reduces startup bundle size by converting eager imports of heavy entity-detail page components and shared UI building blocks into
React.lazy+withSuspenseFallbackwrappers across a wide set of utility files. It also splits display-only helpers fromEntityDisplayUtils.tsxinto a newEntityDisplayPureUtils.tsxso lightweight consumers stop pulling in the heavier module graph.EntityDetailComponentUtils.tsx): replaces the large switch-case inEntityUtilClassBasethat eagerly imported every page component; each page is now code-split behind aSuspenseboundary.getCountBadge,errorMsg,getEntityMissingError, and related helpers moved toEntityDisplayPureUtils.tsx;EntityDisplayUtils.tsxretains onlygetServiceLogo; all import sites updated.TabsLabel,GenericTab,CommonWidgets,ActivityFeedTab,ErrorPlaceHolder, and others converted from static imports tolazy()wrappers inTableTabsUtils,DashboardDetailsUtils,MetricUtils,TopicDetailsUtils,APICollectionUtils, andAPIEndpointUtils.VersionButtonextracted into its own file with a newsummaryprop soEntityVersionTimeLine.tsxno longer depends ongetSummary/useUserProfiletransitively; import sites and tests updated accordingly.Confidence Score: 2/5
Not safe to merge — the UI build will fail as written.
TaskTabNew.component.tsx imports four utility modules (TaskNavigationUtils, TaskActionUtils, TaskAssigneeUtils, TaskPayloadUtils) that do not exist anywhere in the repository; all the symbols they are expected to export currently live in TasksUtils.ts. The TypeScript compiler will reject these module resolutions outright, preventing a successful build. Separately, EntityDetailComponentUtils allocates a new wrapper function on every call to getEntityDetailComponent, which causes React to treat the component as a new type on each render in EntityVersionPage (the one call site that lacks memoization), triggering full unmount/remount cycles for the default entity-version view.
TaskTabNew.component.tsx (missing utility file targets) and EntityDetailComponentUtils.tsx (unstable component references).
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD ER[EntityRouter\nuseMemo] -->|calls| EUC[EntityUtilClassBase\ngetEntityDetailComponent] EVP[EntityVersionPage\nno memo] -->|calls| EVC[EntityVersionClassBase\ngetEntityDetailComponent] EVC --> EUC EUC -->|delegates| EDCU[EntityDetailComponentUtils\ngetEntityDetailComponent] EDCU -->|looks up| LCM[lazyComponentMap\nstable module-level map] LCM -->|lazy import| Pages["Detail Pages\n(DatabaseDetailsPage, TableDetailsPageV1, ...)"] EDCU -->|creates NEW fn each call| WC[WrappedComponent\nnew reference on every call] WC -->|wraps with| SB[Suspense boundary\nfallback=null] subgraph Split [EntityDisplayUtils split] EDU[EntityDisplayUtils\ngetServiceLogo only] EDPU[EntityDisplayPureUtils\ngetCountBadge, errorMsg, getEntityMissingError, ...] end subgraph LazyUtils [Lazified util files] TTU[TableTabsUtils] DDU[DashboardDetailsUtils] MU[MetricUtils] TDU[TopicDetailsUtils] ACU[APICollectionUtils] AEU[APIEndpointUtils] end LazyUtils -->|lazy + withSuspenseFallback| HeavyComponents[TabsLabel, GenericTab, CommonWidgets, ActivityFeedTab, ErrorPlaceHolder, ...]%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% flowchart TD ER[EntityRouter\nuseMemo] -->|calls| EUC[EntityUtilClassBase\ngetEntityDetailComponent] EVP[EntityVersionPage\nno memo] -->|calls| EVC[EntityVersionClassBase\ngetEntityDetailComponent] EVC --> EUC EUC -->|delegates| EDCU[EntityDetailComponentUtils\ngetEntityDetailComponent] EDCU -->|looks up| LCM[lazyComponentMap\nstable module-level map] LCM -->|lazy import| Pages["Detail Pages\n(DatabaseDetailsPage, TableDetailsPageV1, ...)"] EDCU -->|creates NEW fn each call| WC[WrappedComponent\nnew reference on every call] WC -->|wraps with| SB[Suspense boundary\nfallback=null] subgraph Split [EntityDisplayUtils split] EDU[EntityDisplayUtils\ngetServiceLogo only] EDPU[EntityDisplayPureUtils\ngetCountBadge, errorMsg, getEntityMissingError, ...] end subgraph LazyUtils [Lazified util files] TTU[TableTabsUtils] DDU[DashboardDetailsUtils] MU[MetricUtils] TDU[TopicDetailsUtils] ACU[APICollectionUtils] AEU[APIEndpointUtils] end LazyUtils -->|lazy + withSuspenseFallback| HeavyComponents[TabsLabel, GenericTab, CommonWidgets, ActivityFeedTab, ErrorPlaceHolder, ...]Reviews (1): Last reviewed commit: "perf(ui): lazy load entity detail compon..." | Re-trigger Greptile