CONSOLE-4402: React 18 upgrade - phase 1 (WIP)

[logonoff]

Depends on #15893, #15854, and #15875

This PR aims to bump our React dependency to 18. We will NOT be using the new createRoot method here, this will be done in a follow-up. Why?

• So we can earlier gain the benefits of upgrading our @types/react quicker (namely making React.FCC redundant)
• So plugins can prepare by upgrading to react 18 as well
• So we can make a smaller PR
• I still need to figure out how to get us working with the new createRoot render function

Changes:

• Bumped react, react-dom, @types/react, @types/react-dom to 18.x
• Enable i18next allowObjectInHTMLChildren -- see Cannot pass objects to <Trans> with React 18 type definitions i18next/react-i18next#1483
• Fix LazyRoutePage hanging multiple pages within our app (especially in the developer perspective), like +Add, Topology

[TDBank007]

can we check the conflicts with base branch

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

frontend/packages/console-app/src/hooks/usePluginRoutes.tsx (1)

12-32: Verify cache invalidation logic aligns with useMemo dependencies.

The caching approach is sound for preventing component recreation on re-renders. However, there's a subtle inconsistency:

• The useMemo dependency array includes both uid and component (line 29)
• The cache lookup and storage use only uid as the key (lines 19-28)

If the component function reference changes while uid remains the same, useMemo will re-execute but still return the cached component created with the old component function. This inconsistency could lead to stale components being rendered if extensions support hot-reloading or dynamic updates.

Recommendations:

1. If component references are stable for a given uid (typical for static plugin systems), remove component from the dependency array: }, [uid]);
2. If components can change, either use a composite cache key or invalidate the cache entry when detecting a change

In practice, this is likely not an issue if extensions are loaded once at startup with stable references.

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Cache: Disabled due to data retention organization setting

Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting

📥 Commits

Reviewing files that changed from the base of the PR and between e674450 and 8b94bf1.

📒 Files selected for processing (1)

• frontend/packages/console-app/src/hooks/usePluginRoutes.tsx

🧰 Additional context used

📓 Path-based instructions (1)

**

⚙️ CodeRabbit configuration file

-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.

Files:

• frontend/packages/console-app/src/hooks/usePluginRoutes.tsx

🔇 Additional comments (2)

frontend/packages/console-app/src/hooks/usePluginRoutes.tsx (2)

1-1: LGTM!

Adding ComponentType to the imports is necessary for the type annotation of the lazy component cache.

---

39-41: LGTM!

Correctly adds missing dependencies (extension.properties.perspective, path, setActivePerspective) to the useEffect dependency array, ensuring the effect runs when these values change.

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

frontend/packages/console-app/src/hooks/usePluginRoutes.tsx (1)

12-31: Cache invalidation doesn't account for component prop changes.

The useMemo dependency array includes component, but the cache key is only uid. If component changes while uid remains the same, the stale cached lazy component is returned because the cache-hit path doesn't verify the component reference.

Additionally, lazyComponentCache.get(uid) returns T | undefined, so LazyComponent may be typed as potentially undefined.

🔎 Proposed fix using a composite cache key or non-null assertion

Option 1: Use non-null assertion since the logic guarantees existence:

-    return lazyComponentCache.get(uid);
-  }, [uid, component]);
+    return lazyComponentCache.get(uid)!;
+  }, [uid, component]);

Option 2: If component can realistically change, consider keying by both uid and a stable component identifier, or clear the cache entry when the component changes:

 const LazyComponent = useMemo(() => {
-  if (!lazyComponentCache.has(uid)) {
+  const cached = lazyComponentCache.get(uid);
+  // Clear cache if component reference changed (rare but possible)
+  if (cached === undefined) {
     lazyComponentCache.set(
       uid,
       lazy(async () => {
         const Component = await component();
         return { default: Component };
       }),
     );
   }
-  return lazyComponentCache.get(uid);
+  return lazyComponentCache.get(uid)!;
 }, [uid, component]);

If extensions are guaranteed to have stable component props, document this assumption and remove component from the dependency array to avoid confusion.

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Cache: Disabled due to data retention organization setting

Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting

📥 Commits

Reviewing files that changed from the base of the PR and between 8b94bf1 and a64eb5c.

📒 Files selected for processing (1)

• frontend/packages/console-app/src/hooks/usePluginRoutes.tsx

🧰 Additional context used

📓 Path-based instructions (1)

**

⚙️ CodeRabbit configuration file

-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.

Files:

• frontend/packages/console-app/src/hooks/usePluginRoutes.tsx

🔇 Additional comments (2)

frontend/packages/console-app/src/hooks/usePluginRoutes.tsx (2)

1-1: LGTM!

Import addition is necessary for the lazyComponentCache type annotation.

---

39-41: LGTM!

Correct exhaustive dependency array for the useEffect. All values referenced in the effect body are now properly listed.

[openshift-ci-robot]

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

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.22.0" version, but no target version was set.

Details

In response to this:

Depends on #15893, #15854, and #15875

This PR aims to bump our React dependency to 18. We will NOT be using the new createRoot method here, this will be done in a follow-up. This is so that
a) we can gain the benefits of upgrading our @types/react quicker (namely making React.FCC redundant)
b) I want to make smaller PRs
c) there is still some investigation needed as to how to prepare console to work properly with the new createRoot render function

Changes:

• Bumped react, react-dom, @types/react, @types/react-dom to 18.x
• Enable i18next allowObjectInHTMLChildren -- see Cannot pass objects to <Trans> with React 18 type definitions i18next/react-i18next#1483
• Fix LazyRoutePage hanging multiple pages within our app (especially in the developer perspective), like +Add, Topology

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]

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

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "4.22.0" version, but no target version was set.

Details

In response to this:

Depends on #15893, #15854, and #15875

This PR aims to bump our React dependency to 18. We will NOT be using the new createRoot method here, this will be done in a follow-up. Why?

• So we can earlier gain the benefits of upgrading our @types/react quicker (namely making React.FCC redundant)
• So plugins can prepare by upgrading to react 18 as well
• So we can make a smaller PR
• I still need to figure out how to get us working with the new createRoot render function

Changes:

• Bumped react, react-dom, @types/react, @types/react-dom to 18.x
• Enable i18next allowObjectInHTMLChildren -- see Cannot pass objects to <Trans> with React 18 type definitions i18next/react-i18next#1483
• Fix LazyRoutePage hanging multiple pages within our app (especially in the developer perspective), like +Add, Topology

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]

@logonoff: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/frontend	21126dd	link	true	/test frontend
ci/prow/backend	21126dd	link	true	/test backend
ci/prow/e2e-gcp-console	21126dd	link	true	/test e2e-gcp-console

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.