Skip to content

perf(ui): defer my-data shell widgets#29177

Open
shah-harshit wants to merge 25 commits into
feat/dashboard-lcp-parentfrom
feat/dashboard-lcp-01-my-data-shell
Open

perf(ui): defer my-data shell widgets#29177
shah-harshit wants to merge 25 commits into
feat/dashboard-lcp-parentfrom
feat/dashboard-lcp-01-my-data-shell

Conversation

@shah-harshit

@shah-harshit shah-harshit commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Defers /my-data widget rendering behind the shell and adds dashboard widget caching primitives for faster first paint.
  • Split from the dashboard LCP optimization work so it can be reviewed independently.

Testing

  • Not run; tests will be fixed separately for this PR.

Ref: https://github.com/open-metadata/openmetadata-collate/issues/4230

Greptile Summary

This PR defers all landing-page widget rendering behind DeferredWidget (IntersectionObserver) and breaks CustomizeMyDataPageClassBase into focused utility files (CustomizeMyDataPageWidgetUtils, CustomizeMyDataPageImageUtils, DataAssetServiceUtils, LandingPageWidgetIconUtils) to shrink the /my-data shell bundle. It also lazy-loads Suggestions/SearchOptions in the search bar and fixes several useEffect dependency arrays.

  • Bundle splitting: widget registry, widget preview images, service icons, and carousel arrows each moved to separate modules so they are not part of the initial shell parse.
  • Deferred rendering: all grid widgets (not just below-fold ones) are now wrapped in DeferredWidget, deferring data-fetch effects until the IO callback fires; AdvanceSearchProvider is removed from the page wrapper since CuratedAssetsWidget nests its own.
  • Utilities extracted: getEntityLinkFromType, getDataAssetExploreTab, getFormattedDataAssetServiceType, and getEntityIcon replace corresponding class-method calls, eliminating runtime class lookups on the hot render path.

Confidence Score: 5/5

Safe to merge; all functional paths for standard entity types and widget rendering are covered, and the widget/image/service splits correctly preserve the class extension points introduced in the previous fix commit.

The core deferred-rendering path is well-reasoned and the IO/fallback logic handles test, SSR, and Lighthouse environments explicitly. The two observations flagged are confined to uncommon extension-point scenarios.

DataAssetServiceUtils.tsx — the security-category to TABLES tab mapping warrants a second look; FollowingWidget.tsx (and CuratedAssetsWidget, CustomiseLandingPageHeader) — getEntityLinkFromType bypasses the entityUtilClassBase override point.

Important Files Changed

Filename Overview
openmetadata-ui/src/main/resources/ui/src/pages/MyDataPage/MyDataPage.component.tsx All widgets now wrapped in DeferredWidget (not just below-fold); AdvanceSearchProvider removed; layout initialised with default to paint immediately; widgets memo deps correctly simplified; fetchAnnouncements dep-array fixed.
openmetadata-ui/src/main/resources/ui/src/components/common/DeferredWidget/DeferredWidget.component.tsx Removes navigator.webdriver eager-render path (Lighthouse concern) and replaces it with jsdom UA sniff; fallbackInView:true and the useEffect-driven hasBeenVisible pattern look correct.
openmetadata-ui/src/main/resources/ui/src/utils/DataAssetServiceUtils.tsx New file: service icon loading and service-type categorisation extracted here; DataAssetServiceLogo initialises with default icon URL so no blank slot on first render; security category maps to TABLES tab which may be a placeholder.
openmetadata-ui/src/main/resources/ui/src/utils/CustomizeMyDataPageClassBase.ts Heavily trimmed; widget registry and image utils moved out; defaultLayout delegates to new constants; getWidgetsFromKey still present as a public override point routing to getMyDataWidgetFromKey.
openmetadata-ui/src/main/resources/ui/src/utils/CustomizeMyDataPageWidgetUtils.tsx New file: lazy-import registry for all landing-page widgets; correctly isolated so widget chunks are only reachable through the deferred render path.
openmetadata-ui/src/main/resources/ui/src/utils/LandingPageWidgetIconUtils.tsx New file: getEntityIcon consolidates entity-type icon and service-logo lookup; falls back to LANDING_WIDGET_DEFAULT_ICON_URL for unknown types.
openmetadata-ui/src/main/resources/ui/src/components/MyData/CustomizableComponents/CustomiseLandingPageHeader/CustomiseSearchBar.tsx Suggestions/SearchOptions now lazy-loaded; popoverContent returns null when search box is closed preventing unnecessary mounts; SEARCH_INDEX_PATH_MAP replaces searchClassBase.getTabsInfo() lookup.
openmetadata-ui/src/main/resources/ui/src/components/MyData/RightSidebar/FollowingWidget.tsx fetchUserFollowedData correctly wrapped in useCallback; now handles no-user case by clearing state; getEntityIcon replaces inline serviceUtilClassBase call; entityUtilClassBase.getEntityLink replaced by getEntityLinkFromType.
openmetadata-ui/src/main/resources/ui/src/utils/CustomizableLandingPagePureUtils.ts Class-level constants replaced with imported named constants; getAddWidgetHandler now accepts an injected widgetHeightResolver defaulting to the local getWidgetHeight, preserving enterprise override ability.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant Browser
    participant MyDataPage
    participant DeferredWidget
    participant IO as IntersectionObserver
    participant Widget as Widget (lazy chunk)

    Browser->>MyDataPage: navigate to /my-data
    MyDataPage->>MyDataPage: useState(getDefaultLandingPageLayout)
    Note over MyDataPage: Shell + skeleton render
    MyDataPage->>MyDataPage: fetchDocument()
    MyDataPage-->>Browser: Paint skeleton immediately

    par fetch persona layout
        MyDataPage->>MyDataPage: getDocumentByFQN(persona)
        MyDataPage->>MyDataPage: setLayout + setIsLoading(false)
    and IO setup
        MyDataPage->>DeferredWidget: "mount all DeferredWidgets (shouldRender=false)"
        DeferredWidget->>IO: observe each widget wrapper
    end

    MyDataPage-->>Browser: Replace skeleton with empty grid placeholders

    IO-->>DeferredWidget: "inView=true (above-fold first)"
    DeferredWidget->>DeferredWidget: setHasBeenVisible(true)
    DeferredWidget->>Widget: import() lazy chunk
    Widget-->>Browser: Render widget + fire data fetch

    IO-->>DeferredWidget: "inView=true (on scroll)"
    DeferredWidget->>Widget: mount + fetch
Loading
%%{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"}}}%%
sequenceDiagram
    participant Browser
    participant MyDataPage
    participant DeferredWidget
    participant IO as IntersectionObserver
    participant Widget as Widget (lazy chunk)

    Browser->>MyDataPage: navigate to /my-data
    MyDataPage->>MyDataPage: useState(getDefaultLandingPageLayout)
    Note over MyDataPage: Shell + skeleton render
    MyDataPage->>MyDataPage: fetchDocument()
    MyDataPage-->>Browser: Paint skeleton immediately

    par fetch persona layout
        MyDataPage->>MyDataPage: getDocumentByFQN(persona)
        MyDataPage->>MyDataPage: setLayout + setIsLoading(false)
    and IO setup
        MyDataPage->>DeferredWidget: "mount all DeferredWidgets (shouldRender=false)"
        DeferredWidget->>IO: observe each widget wrapper
    end

    MyDataPage-->>Browser: Replace skeleton with empty grid placeholders

    IO-->>DeferredWidget: "inView=true (above-fold first)"
    DeferredWidget->>DeferredWidget: setHasBeenVisible(true)
    DeferredWidget->>Widget: import() lazy chunk
    Widget-->>Browser: Render widget + fire data fetch

    IO-->>DeferredWidget: "inView=true (on scroll)"
    DeferredWidget->>Widget: mount + fetch
Loading

Reviews (17): Last reviewed commit: "fix(ui): guard NLP suggestions fetch" | Re-trigger Greptile

@shah-harshit shah-harshit requested a review from a team as a code owner June 18, 2026 10:31
@shah-harshit shah-harshit added UI UI specific issues safe to test Add this label to run secure Github workflows on PRs skip-pr-checks Bypass PR metadata validation check labels Jun 18, 2026
@shah-harshit shah-harshit self-assigned this Jun 18, 2026
Comment thread openmetadata-ui/src/main/resources/ui/src/utils/DataAssetServiceUtils.tsx Outdated
Comment thread openmetadata-ui/src/main/resources/ui/src/hooks/useDashboardWidgetData.ts Outdated
Comment thread openmetadata-ui/src/main/resources/ui/src/utils/DataAssetServiceUtils.tsx Outdated
Comment thread openmetadata-ui/src/main/resources/ui/src/hooks/useDashboardWidgetData.ts Outdated
Comment thread openmetadata-ui/src/main/resources/ui/src/utils/DataAssetServiceUtils.tsx Outdated
@sonarqubecloud

Copy link
Copy Markdown

Comment thread openmetadata-ui/src/main/resources/ui/src/components/AppBar/Suggestions.tsx Outdated
@gitar-bot

gitar-bot Bot commented Jun 19, 2026

Copy link
Copy Markdown
Code Review ✅ Approved 9 resolved / 9 findings

Optimizes dashboard loading by deferring landing page widgets behind IntersectionObserver and refactoring the shell into lightweight utilities. Resolved all identified loading edge cases and state management issues.

✅ 9 resolved
Edge Case: Stale data set into state after cache epoch change

📄 openmetadata-ui/src/main/resources/ui/src/hooks/useDashboardWidgetData.ts:88-100
In refresh, after the in-flight request resolves the epoch guard correctly prevents writing the result to the shared cache when clearDashboardWidgetCache() ran mid-flight (e.g. logout / user switch). However setData(response) is still called unconditionally, so a component that is still mounted across the clear (or that re-fired a fetch for a new user with a different cacheKey) can momentarily render the previous user's data even though the cache was intentionally invalidated. Consider also gating the setData call on epoch === dashboardWidgetCacheEpoch so the discarded response never reaches component state.

Bug: Widget stuck in loading when currentUser is undefined

📄 openmetadata-ui/src/main/resources/ui/src/components/MyData/MyDataWidget/MyDataWidget.component.tsx:167-170
The refactor wraps the entire fetch body in if (!isUndefined(currentUser)) but removed the previous else/early-return branch that called setData([]) and setIsLoading(false). isLoading is initialized to true (line 68). Now, when currentUser is undefined (e.g. before the application store hydrates, or for an unauthenticated/incomplete session), fetchMyDataAssets does nothing: it never enters the try/finally, so setIsLoading(false) is never called. The widget then renders its loading skeleton indefinitely instead of showing an empty state. The prior version handled this explicitly. Restore an early-out that clears loading state when there is no current user.

Bug: ownedAssets.map can throw when hits is undefined

📄 openmetadata-ui/src/main/resources/ui/src/components/MyData/MyDataWidget/MyDataWidget.component.tsx:148-149
The previous code used const ownedAssets = res?.hits?.hits ?? []; to guard against an absent hits array. The refactor dropped the ?? [] fallback, so const ownedAssets = res?.hits?.hits; followed by ownedAssets.map(...) will throw a TypeError if res.hits.hits is undefined. This is caught by the surrounding catch (which sets data to []), so it is not user-visible, but it relies on an exception path for normal flow and is a robustness regression. Re-add the ?? [] default.

Edge Case: NLP-active mount can leave Suggestions stuck on Loader

📄 openmetadata-ui/src/main/resources/ui/src/components/AppBar/Suggestions.tsx:386-399 📄 openmetadata-ui/src/main/resources/ui/src/components/AppBar/Suggestions.tsx:436-438
isLoading is initialized to true, and the new effect now calls fetchSearchData() whenever searchText && !isTourOpen. But fetchSearchData() returns early when isNLPActive is true, before setIsLoading(true)/finally setIsLoading(false) run — so it never clears the initial loading state. If the component first mounts with isNLPActive=true and a non-empty searchText (in GlobalSearchBar the dropdown renders Suggestions whenever searchValue || isNLPActive), the effect calls fetchSearchData, it early-returns, and isLoading stays true forever, so the component renders <Loader/> indefinitely.

The removed isMounting ref previously avoided this: on the first render it skipped fetchSearchData and hit the else { setIsLoading(false) } branch, clearing the loader. Removing it reintroduces the stuck-loader path for the NLP case.

Fix: short-circuit the effect when NLP is active (so the else-branch clears loading), or guard the fetch call.

Bug: WidgetCard test mocks named export instead of default singleton

📄 openmetadata-ui/src/main/resources/ui/src/components/MyData/CustomizableComponents/WidgetCard/WidgetCard.test.tsx:44-46 📄 openmetadata-ui/src/main/resources/ui/src/utils/CustomizeMyDataPageClassBase.ts:124-126
CustomizeMyDataPageClassBase exports a default singleton (export default customizeMyDataPageClassBase), and WidgetCard.tsx consumes it as a default import: import customizePageClassBase from '...' then customizePageClassBase.getWidgetImageFromKey(...) (LandingPage branch).

The new mock replaces the module with { getWidgetImageFromKey: jest.fn() } — a named export with no default. As a result the default import resolves to undefined, and the mock never actually intercepts getWidgetImageFromKey. The tests only pass today because useCustomizeStore().currentPageType is not PageType.LandingPage, so the switch falls to the default branch which calls the unmocked customizeDetailPageClassBase instead. If the LandingPage path were ever exercised, the test would throw Cannot read properties of undefined (reading 'getWidgetImageFromKey').

Mock the default export shape instead so it matches how the component imports the module.

...and 4 more resolved from earlier reviews

Options

Display: compact → Showing less information.

Comment with these commands to change:

Compact
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

safe to test Add this label to run secure Github workflows on PRs skip-pr-checks Bypass PR metadata validation check UI UI specific issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants