Skip to content

Conversation

@logonoff
Copy link
Member

@logonoff logonoff commented Jan 16, 2026

Cherrypick from #14869

The LazyDynamicRoutePage component was calling React.lazy() inside a useMemo that depended on the component prop reference. During in-app navigation, if extension references changed, this caused:

  1. React.lazy() to be called again, creating a new lazy component
  2. React to unmount the old component and mount the new one
  3. Suspense to trigger loading state and re-resolve the async import
  4. Cascading re-mounts causing 100% CPU usage and page freeze

Now, we cache the resolved components by extension UID in a module-level Map. The lazy component is created once per extension and reused on subsequent renders, preventing unnecessary remounting during navigation.

Summary by CodeRabbit

  • Refactor
    • Optimized extension route loading with improved caching mechanism for lazy-loaded plugin components.

✏️ Tip: You can customize this high-level summary in your review settings.

The LazyDynamicRoutePage component was calling React.lazy() inside a
useMemo that depended on the component prop reference. During in-app
navigation, if extension references changed, this caused:

1. React.lazy() to be called again, creating a new lazy component
2. React to unmount the old component and mount the new one
3. Suspense to trigger loading state and re-resolve the async import
4. Cascading re-mounts causing 100% CPU usage and page freeze

Fix: Cache lazy components by extension UID in a module-level Map.
The lazy component is created once per extension and reused on
subsequent renders, preventing unnecessary remounting during navigation.
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 16, 2026
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jan 16, 2026

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Jan 16, 2026

@logonoff: This pull request references CONSOLE-5035 which is a valid jira issue.

Details

In response to this:

Cherrypick from #14869

The LazyDynamicRoutePage component was calling React.lazy() inside a useMemo that depended on the component prop reference. During in-app navigation, if extension references changed, this caused:

  1. React.lazy() to be called again, creating a new lazy component
  2. React to unmount the old component and mount the new one
  3. Suspense to trigger loading state and re-resolve the async import
  4. Cascading re-mounts causing 100% CPU usage and page freeze

Now, we cache the resolved components by extension UID in a module-level Map. The lazy component is created once per extension and reused on subsequent renders, preventing unnecessary remounting during navigation.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Jan 16, 2026
@coderabbitai
Copy link

coderabbitai bot commented Jan 16, 2026

Walkthrough

This change optimizes the plugin routing hook by introducing a memoized, per-extension lazy component cache keyed by extension UID. The refactor replaces a dynamic component wrapper approach with a cacheable lazy loader pattern while preserving existing activation/inactivation routing logic.

Changes

Cohort / File(s) Summary
Plugin routing component caching
frontend/packages/console-app/src/hooks/usePluginRoutes.tsx
Introduced lazyComponentCache Map and memoized LazyComponent instances per extension UID to prevent lazy loader recreation on re-renders. Removed LazyDynamicRoutePage wrapper; LazyRoutePage now computes or retrieves cached lazy components. Added ComponentType to React imports. Routing behavior for active/inactive pages unchanged.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~18 minutes

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately captures the main change: introducing a caching mechanism for lazy route components to resolve a navigation freeze issue.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

🧹 Recent nitpick comments
frontend/packages/console-app/src/hooks/usePluginRoutes.tsx (1)

18-31: Address potential type unsafety in lazy component caching pattern.

While TypeScript's strict: false config prevents errors, lazyComponentCache.get(uid) on line 28 technically returns LazyExoticComponent | undefined. The subsequent JSX render on line 31 could fail if the type weren't loosely checked. The if/has/set/get pattern is runtime-safe, but restructuring eliminates the type ambiguity and improves readability:

const LazyComponent = useMemo(() => {
  let cached = lazyComponentCache.get(uid);
  if (!cached) {
    cached = lazy(async () => {
      const Component = await component();
      return { default: Component };
    });
    lazyComponentCache.set(uid, cached);
  }
  return cached;
}, [uid, component]);

Additionally, the component dependency in the useMemo array is now a no-op since the cache key depends only on uid. Consider a brief inline comment explaining why component changes don't invalidate the cache—this clarifies the intentional design choice for future reviewers.


📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Cache: Disabled due to data retention organization setting

Knowledge base: Disabled due to data retention organization setting

📥 Commits

Reviewing files that changed from the base of the PR and between bf86037 and 6adaf27.

📒 Files selected for processing (1)
  • frontend/packages/console-app/src/hooks/usePluginRoutes.tsx
🔇 Additional comments (2)
frontend/packages/console-app/src/hooks/usePluginRoutes.tsx (2)

1-1: LGTM on import addition.

The ComponentType import is necessary for properly typing the lazy component cache. Clean and minimal change.


12-13: Solid caching strategy for extension lazy components.

Module-level cache keyed by uid is the right approach here—extensions are effectively static during a session, so memory growth is bounded and the perf win (avoiding repeated React.lazy() instantiation) is worth it.

One minor consideration: during development with HMR, cached components won't hot-reload if the underlying component changes while the UID stays constant. Likely acceptable trade-off for production stability, but worth documenting in a comment if this causes confusion during plugin development.

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

@logonoff logonoff marked this pull request as ready for review January 16, 2026 01:00
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 16, 2026
@logonoff
Copy link
Member Author

logonoff commented Jan 16, 2026

/label px-approved
/label docs-approved

/assign @yapei @vojtechszocs

Note we can't really test this patch in a React 18 scenario at the moment, but we can verify that this doesn't cause problems in React 17 (e.g., console.page/route continues to work fine, when accessed via react-router in the UI, after a hard reload, etc.).

@openshift-ci openshift-ci bot added the component/core Related to console core functionality label Jan 16, 2026
@openshift-ci openshift-ci bot added approved Indicates a PR has been approved by an approver from all required OWNERS files. px-approved Signifies that Product Support has signed off on this PR docs-approved Signifies that Docs has signed off on this PR labels Jan 16, 2026
@openshift-ci openshift-ci bot requested review from Leo6Leo and TheRealJon January 16, 2026 01:00
@openshift-ci-robot
Copy link
Contributor

openshift-ci-robot commented Jan 16, 2026

@logonoff: This pull request references CONSOLE-5035 which is a valid jira issue.

Details

In response to this:

Cherrypick from #14869

The LazyDynamicRoutePage component was calling React.lazy() inside a useMemo that depended on the component prop reference. During in-app navigation, if extension references changed, this caused:

  1. React.lazy() to be called again, creating a new lazy component
  2. React to unmount the old component and mount the new one
  3. Suspense to trigger loading state and re-resolve the async import
  4. Cascading re-mounts causing 100% CPU usage and page freeze

Now, we cache the resolved components by extension UID in a module-level Map. The lazy component is created once per extension and reused on subsequent renders, preventing unnecessary remounting during navigation.

Summary by CodeRabbit

  • Refactor
  • Optimized extension route loading with improved caching mechanism for lazy-loaded plugin components.

✏️ Tip: You can customize this high-level summary in your review settings.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

@yapei
Copy link
Contributor

yapei commented Jan 16, 2026

deploy console-demo-plugin, visit exposed paths defined as "type": "console.page/route" ,verify pages work well after hard refresh. No regression issues found
/verified by @yapei

@openshift-ci-robot openshift-ci-robot added the verified Signifies that the PR passed pre-merge verification criteria label Jan 16, 2026
@openshift-ci-robot
Copy link
Contributor

@yapei: This PR has been marked as verified by @yapei.

Details

In response to this:

deploy console-demo-plugin, visit exposed paths defined as "type": "console.page/route" ,verify pages work well after hard refresh. No regression issues found
/verified by @yapei

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository.

Copy link
Member

@TheRealJon TheRealJon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jan 16, 2026
@logonoff
Copy link
Member Author

/retest

@vojtechszocs
Copy link
Contributor

/lgtm

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jan 16, 2026

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: logonoff, TheRealJon, vojtechszocs

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jan 17, 2026

@logonoff: all tests passed!

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

@openshift-merge-bot openshift-merge-bot bot merged commit 5ee7eca into openshift:main Jan 17, 2026
8 checks passed
@logonoff logonoff deleted the CONSOLE-5035-lazyroutepage branch January 17, 2026 01:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. component/core Related to console core functionality docs-approved Signifies that Docs has signed off on this PR jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged. px-approved Signifies that Product Support has signed off on this PR verified Signifies that the PR passed pre-merge verification criteria

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants