CONSOLE-4439: Upgrade to React Router 7#14957
CONSOLE-4439: Upgrade to React Router 7#14957vojtechszocs wants to merge 4 commits intoopenshift:mainfrom
Conversation
openshift-ci
bot
added
the
do-not-merge/work-in-progress
openshift-merge-robot
added
the
needs-rebase
openshift-ci
bot
added
component/core
|
/label tide/merge-method-squash |
openshift-ci
bot
added
the
tide/merge-method-squash
openshift-merge-robot
removed
the
needs-rebase
|
/retest NO-JIRA: Drop support of React Router 5 (wip) |
|
@logonoff: The The following commands are available to trigger optional jobs: Use DetailsIn response to this:
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. |
|
/test all |
logonoff
force-pushed
the
drop-react-router-v5
branch
from
7827ca9 to
b53853f
Compare
openshift-ci
bot
removed
the
lifecycle/rotten
logonoff
force-pushed
the
drop-react-router-v5
branch
from
1d70080 to
ac55799
Compare
openshift-merge-robot
removed
the
needs-rebase
|
@vojtechszocs: This pull request references CONSOLE-4439 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. DetailsIn response to this:
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
bot
added
the
approved
logonoff
force-pushed
the
drop-react-router-v5
branch
from
ac55799 to
63e9560
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
frontend/public/components/utils/router.ts (1)
100-100:⚠️ Potential issue | 🟡 MinorTrailing
?when all query params are removed.The URL template
`${loc.pathname}?${updated.toString()}${loc.hash}`will produce a trailing?(e.g.,/foo?#hashor/foo?) whenupdatedis empty. This is a cosmetic issue but can cause URL normalization mismatches in tests or analytics.Suggested fix
- navigate(`${loc.pathname}?${updated.toString()}${loc.hash}`, { + const search = updated.toString(); + navigate(`${loc.pathname}${search ? `?${search}` : ''}${loc.hash}`, {Apply the same pattern to lines 121, 143, 157, and 178.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/public/components/utils/router.ts` at line 100, The navigate URL template currently always injects a "?" even when updated.toString() is empty, producing trailing question marks; change each navigate call (the one at the shown line and the other calls at lines 121, 143, 157, 178) to build the URL conditionally: compute const q = updated.toString(); then use const url = q ? `${loc.pathname}?${q}${loc.hash}` : `${loc.pathname}${loc.hash}` and pass url to navigate(...). Replace the inline `${loc.pathname}?${updated.toString()}${loc.hash}` usage in those navigate calls with this conditional URL construction.frontend/packages/console-dynamic-plugin-sdk/src/extensions/pages.ts (1)
73-82:⚠️ Potential issue | 🟡 MinorStale React Router v5 reference in
StandaloneRoutePagedoc.The
StandaloneRoutePageJSDoc (lines 76-77) still says "Under the hood we use React Router. See https://v5.reactrouter.com/". Since this PR drops v5 support, this should be updated to reference v6 (consistent with theRoutePagedoc at line 30 which correctly links to v6).📝 Proposed fix
/** * Adds new standalone page (rendered outside the common page layout) to Console router. * - * Under the hood we use React Router. - * See https://v5.reactrouter.com/ + * Under the hood we use [React Router v6](https://reactrouter.com/docs/en/v6). */🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/packages/console-dynamic-plugin-sdk/src/extensions/pages.ts` around lines 73 - 82, The JSDoc for StandaloneRoutePage still references React Router v5; update its comment to reference React Router v6 to match RoutePage's doc. Edit the comment block above the StandaloneRoutePage type (the Extension declaration using 'console.page/route/standalone' and Omit<RoutePageProperties, 'perspective'>) and replace the v5 URL/text with the v6 documentation URL and wording consistent with RoutePage's doc.
♻️ Duplicate comments (1)
frontend/packages/console-dynamic-plugin-sdk/docs/console-extensions.md (1)
819-832: Stale v5 link inconsole.page/route/standalonesummary — downstream of source file.Line 823 still reads "See https://v5.reactrouter.com/" which is inconsistent with the v6 migration. This appears to be auto-generated from the
StandaloneRoutePageJSDoc inpages.ts— fixing the source (as noted in that file's review) should resolve this on the next doc generation pass.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/packages/console-dynamic-plugin-sdk/docs/console-extensions.md` around lines 819 - 832, Update the JSDoc for the StandaloneRoutePage (StandaloneRoutePage or StandaloneRoutePageProps) in pages.ts to reference React Router v6 instead of the v5 URL shown in generated docs; modify the JSDoc summary or See link text to point to the v6 docs (https://reactrouter.com/) or remove the versioned URL so the generated docs for console.page/route/standalone no longer include "See https://v5.reactrouter.com/". Ensure the JSDoc change is in the StandaloneRoutePage declaration so the next docs generation picks up the corrected link.
🧹 Nitpick comments (5)
frontend/packages/console-dynamic-plugin-sdk/release-notes/4.22.md (1)
15-16: Add a forward migration pointer for "deprecated" users.The note tells plugin authors to "continue to use this deprecated shared module as is" but offers no guidance on what the eventual migration target is. The intent was always to change all
react-router-dom-v5-compatimports toreact-router-domonce the app is fully on v6. Pointing to either the official React Router v6 migration guide or a Console-specific migration document would make this release note actionable, especially given theplugin-api-changedlabel on the PR.✏️ Suggested addition
compatible with the previous version of `react-router-dom-v5-compat`. Plugins that need to support versions of OpenShift starting from `4.14.x` can continue to use this deprecated shared module as is. + To migrate off this deprecated module, replace all `react-router-dom-v5-compat` imports with + `react-router-dom` and refer to the [React Router v6 upgrade guide](https://reactrouter.com/6.30.0/upgrading/v5).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/packages/console-dynamic-plugin-sdk/release-notes/4.22.md` around lines 15 - 16, The release note tells authors to keep using the deprecated shared module but lacks guidance on migrating; update the text in release-notes/4.22.md to include a forward migration pointer instructing plugin authors to replace imports of react-router-dom-v5-compat with react-router-dom once the app is fully on v6, and link to the React Router v6 migration guide (or a Console-specific migration doc) for step-by-step instructions and mention the plugin-api-changed context so consumers know this is the intended long-term change.frontend/packages/console-dynamic-plugin-sdk/src/runtime/plugin-shared-modules.ts (1)
34-37: Clean implementation. Consider a defensive guard for robustness.The aliasing logic is correct and the JSDoc is excellent. One minor thought: if
scope['react-router-dom']were ever missing (e.g., due to a future refactor removing it from shared modules), this would silently set the compat entry toundefined, which would surface as a confusing runtime error in plugins.A lightweight guard could help future-proof this:
Optional defensive check
export const patchSharedScope = () => { const scope = getSharedScope(); + if (!scope['react-router-dom']) { + // eslint-disable-next-line no-console + console.warn('react-router-dom not found in shared scope; skipping v5-compat patch'); + return; + } scope['react-router-dom-v5-compat'] = scope['react-router-dom']; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/packages/console-dynamic-plugin-sdk/src/runtime/plugin-shared-modules.ts` around lines 34 - 37, In patchSharedScope(), guard the alias assignment so you only set scope['react-router-dom-v5-compat'] when scope['react-router-dom'] exists: retrieve the scope via getSharedScope(), check for the presence (non-undefined) of the 'react-router-dom' entry and only then assign it to 'react-router-dom-v5-compat'; if missing, skip the alias (optionally log or leave a TODO comment) to avoid creating an undefined compat binding.frontend/public/components/utils/router.ts (1)
89-107: Setting an empty string deletes the parameter — potentially surprising.Lines 94-95: when
v === ''is passed tosetQueryArgument, the key is deleted rather than set to an empty string. This is an implicit contract that differs fromURLSearchParams.set(k, '')behavior. If any caller genuinely wants to set a key to""(e.g., to clear a filter value while keeping the parameter present), this will silently remove it instead.If this is intentional, a brief inline comment would help. If not:
Suggested clarification
if (v === '') { + // Intentionally remove the param rather than setting it to empty string updated.delete(k); } else {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/public/components/utils/router.ts` around lines 89 - 107, The setQueryArgument callback currently deletes the key when v === '' which diverges from URLSearchParams.set(k, '') semantics; update setQueryArgument (the function that builds updated = new URLSearchParams(searchParamsRef.current)) to always call updated.set(k, v) even for empty strings (i.e., remove the if (v === '') { updated.delete(k) } branch) so callers can preserve an empty-valued param, and add a brief inline comment above setQueryArgument explaining that empty string values are intentionally preserved (or, if deletion was intended, instead document that behavior and use a distinct sentinel like null/undefined to mean “remove”).frontend/packages/console-app/src/hooks/usePluginRoutes.tsx (1)
60-81: Well-designed helper — one subtle concern aboutexactsemantics inversion.The implementation is clean and correctly handles the array/string normalization and
/*suffix logic. A few observations:
exactdefault is inverted from v6 native behavior: Whenexactisundefined/false, you append/*(non-exact matching). This is the opposite of React Router v6's native behavior (exact by default). The JSDoc + extension docs cover this, but consider adding a brief inline comment at line 77 to prevent future contributors from being confused by the inversion.
getElementparametercurrentPathis unused by all current callers (both here at line 96 and inapp.tsx). If no caller needs it now, you could simplify the signature togetElement: () => RouteProps['element']. But keeping it is reasonable for future extensibility.💡 Optional inline comment for clarity
return routePaths.map((currentPath) => ( <Route key={`${uid}/${currentPath}`} + // Note: Unlike React Router v6 defaults (exact match), Console retains v5 behavior + // where routes are non-exact unless explicitly opted in via `exact: true`. path={`${currentPath}${exact ? '' : '/*'}`} element={getElement(currentPath)} /> ));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/packages/console-app/src/hooks/usePluginRoutes.tsx` around lines 60 - 81, Add a short inline comment inside mapExtensionToRoutes explaining that the exact parameter is intentionally inverted relative to React Router v6 (i.e., when exact is falsy we append '/*' to allow nested matching because v6 routes are exact by default), so future contributors won’t be confused; locate the comment near where path is suffixed (`path={`${currentPath}${exact ? '' : '/*'}`}`) and keep the current logic, and optionally consider (but do not change now) simplifying the getElement signature from (currentPath: string) => RouteProps['element'] to () => RouteProps['element'] if no callers need the parameter.frontend/packages/console-dynamic-plugin-sdk/scripts/package-definitions.ts (1)
83-83:VIRTUAL_DEPSspread last gives it override precedence over actual root package deps.Since
VIRTUAL_DEPSis intended to satisfy modules not installed in the root package, it should act as a fallback, not a trump card. As written, ifreact-router-dom-v5-compatis later added torootPackage.json, the virtual entry silently wins with a potentially stale/different version.♻️ Proposed fix — spread VIRTUAL_DEPS first
- const srcDeps = { ...pkg.devDependencies, ...pkg.dependencies, ...VIRTUAL_DEPS }; + const srcDeps = { ...VIRTUAL_DEPS, ...pkg.devDependencies, ...pkg.dependencies };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/packages/console-dynamic-plugin-sdk/scripts/package-definitions.ts` at line 83, The spread ordering gives VIRTUAL_DEPS higher precedence than real deps; change the srcDeps construction so VIRTUAL_DEPS are spread first and pkg.devDependencies and pkg.dependencies come after (i.e., build srcDeps from VIRTUAL_DEPS, then ...pkg.devDependencies, then ...pkg.dependencies) so actual root package entries override the virtual fallbacks; update the srcDeps assignment that references VIRTUAL_DEPS, pkg.devDependencies, and pkg.dependencies accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/package.json`:
- Around line 220-222: Remove the incompatible dependency
"react-router-hash-link" from package.json and either implement manual hash
scrolling or swap to a v6-compatible package: if implementing manually, use
React Router's useLocation() in your layout or a scroll handler (e.g., in App or
a ScrollManager component) to detect location.hash and call
document.getElementById(...).scrollIntoView({ behavior: 'smooth' }) (or
element.scrollIntoView) after navigation; if replacing, remove
"react-router-hash-link" and add a v6-compatible alternative such as
"scroll-to-hash-element" and update any Link usage to the new API. Ensure you
update package.json dependencies and any imports/usages referencing
react-router-hash-link.
In `@frontend/packages/console-app/src/hooks/usePluginRoutes.tsx`:
- Around line 87-112: The memo in usePluginRoutes is being invalidated by an
unstable setActivePerspective reference—wrap the setPerspective function created
in useValuesForPerspectiveContext (used by DetectPerspective) in useCallback to
provide referential stability; specifically, wrap setPerspective with
useCallback and use [navigate, fireTelemetryEvent, setLastPerspective] as its
dependency array so the setActivePerspective value passed into usePluginRoutes
remains stable and the useMemo in usePluginRoutes can actually memoize the
routePages reduce.
In `@frontend/packages/console-dynamic-plugin-sdk/release-notes/4.22.md`:
- Around line 14-16: The release note claims backward compatibility but
patchSharedScope in plugin-shared-modules.ts (the line setting
scope['react-router-dom-v5-compat'] = scope['react-router-dom']) only aliases
the module and does not re-export CompatRouter or CompatRoute, so plugins
importing CompatRouter/CompatRoute will get undefined; update the 4.22 release
note text to explicitly state that the shared module exposes only standard React
Router v6 APIs and add the sentence: "Plugins must migrate away from
CompatRouter and CompatRoute before upgrading to 4.22, as only standard React
Router v6 APIs are available in the shared module."
In `@frontend/packages/console-dynamic-plugin-sdk/scripts/package-definitions.ts`:
- Around line 74-76: The VIRTUAL_DEPS entry uses an npm install-time alias
('npm:react-router-dom@^6.30.x') which is not a valid SemVer range and will end
up in peerDependencies via parseSharedModuleDeps; change the value for
'react-router-dom-v5-compat' in VIRTUAL_DEPS to a plain SemVer range (e.g.
'^6.30.x') so parseSharedModuleDeps emits a valid peerDependency, and if you
still need the install-time aliasing, implement that at install/alias resolution
time (not inside VIRTUAL_DEPS) or map the virtual name to the actual package
name elsewhere in the resolver that synthesizes install aliases.
In `@frontend/public/components/app.tsx`:
- Around line 19-21: Consolidate the mixed routing imports by importing all
router symbols from the same package: move the unstable_HistoryRouter
(UnstableHistoryRouter) import so it's grouped with useParams, useLocation,
Routes, Route (or vice versa) instead of importing from both 'react-router-dom'
and 'react-router-dom-v5-compat'; update the import statement(s) so that
UnstableHistoryRouter, useParams, useLocation, Routes, and Route come from a
single source (either 'react-router-dom' or 'react-router-dom-v5-compat') and
keep mapExtensionToRoutes import unchanged.
In `@frontend/public/components/factory/modal.tsx`:
- Around line 74-80: The modal rendering path mounts components outside the app
router so any component using router hooks (e.g., useNavigate/useLocation)
fails; update the render in ModalWrapper/Component branch to wrap the output
with the app's HistoryRouter (the UnstableHistoryRouter or HistoryRouter
instance used in app.tsx) using the shared history instance so router context is
available to launched modals; specifically import the UnstableHistoryRouter (or
HistoryRouter) and the shared history that app.tsx initializes, and wrap the JSX
returned for both the ModalWrapper and direct Component render inside that
router (preserving blocking, className, onClose, cancel, close props) so
createModal-mounted components like DeleteModal and ClusterUpdateModal receive
router context.
In `@frontend/public/components/utils/router.ts`:
- Around line 44-48: The monkey-patched overrides for
history.replace/history.push (history.__replace__/history.__push and the new
history.replace/history.push) drop the optional state parameter and mishandle
non-string locations; update the patched signatures to match the original (to:
To, state?: any) and preserve state when delegating to the originals; inside the
new functions, detect if to is a string and call removeBasePath(to) before
passing it through, otherwise pass the location object unchanged, and forward
the optional state argument to (history as any).__replace__ / __push__; also
update typings/JSDoc for history.replace and history.push to reflect the
original signature.
---
Outside diff comments:
In `@frontend/packages/console-dynamic-plugin-sdk/src/extensions/pages.ts`:
- Around line 73-82: The JSDoc for StandaloneRoutePage still references React
Router v5; update its comment to reference React Router v6 to match RoutePage's
doc. Edit the comment block above the StandaloneRoutePage type (the Extension
declaration using 'console.page/route/standalone' and Omit<RoutePageProperties,
'perspective'>) and replace the v5 URL/text with the v6 documentation URL and
wording consistent with RoutePage's doc.
In `@frontend/public/components/utils/router.ts`:
- Line 100: The navigate URL template currently always injects a "?" even when
updated.toString() is empty, producing trailing question marks; change each
navigate call (the one at the shown line and the other calls at lines 121, 143,
157, 178) to build the URL conditionally: compute const q = updated.toString();
then use const url = q ? `${loc.pathname}?${q}${loc.hash}` :
`${loc.pathname}${loc.hash}` and pass url to navigate(...). Replace the inline
`${loc.pathname}?${updated.toString()}${loc.hash}` usage in those navigate calls
with this conditional URL construction.
---
Duplicate comments:
In `@frontend/packages/console-dynamic-plugin-sdk/docs/console-extensions.md`:
- Around line 819-832: Update the JSDoc for the StandaloneRoutePage
(StandaloneRoutePage or StandaloneRoutePageProps) in pages.ts to reference React
Router v6 instead of the v5 URL shown in generated docs; modify the JSDoc
summary or See link text to point to the v6 docs (https://reactrouter.com/) or
remove the versioned URL so the generated docs for console.page/route/standalone
no longer include "See https://v5.reactrouter.com/". Ensure the JSDoc change is
in the StandaloneRoutePage declaration so the next docs generation picks up the
corrected link.
---
Nitpick comments:
In `@frontend/packages/console-app/src/hooks/usePluginRoutes.tsx`:
- Around line 60-81: Add a short inline comment inside mapExtensionToRoutes
explaining that the exact parameter is intentionally inverted relative to React
Router v6 (i.e., when exact is falsy we append '/*' to allow nested matching
because v6 routes are exact by default), so future contributors won’t be
confused; locate the comment near where path is suffixed
(`path={`${currentPath}${exact ? '' : '/*'}`}`) and keep the current logic, and
optionally consider (but do not change now) simplifying the getElement signature
from (currentPath: string) => RouteProps['element'] to () =>
RouteProps['element'] if no callers need the parameter.
In `@frontend/packages/console-dynamic-plugin-sdk/release-notes/4.22.md`:
- Around line 15-16: The release note tells authors to keep using the deprecated
shared module but lacks guidance on migrating; update the text in
release-notes/4.22.md to include a forward migration pointer instructing plugin
authors to replace imports of react-router-dom-v5-compat with react-router-dom
once the app is fully on v6, and link to the React Router v6 migration guide (or
a Console-specific migration doc) for step-by-step instructions and mention the
plugin-api-changed context so consumers know this is the intended long-term
change.
In `@frontend/packages/console-dynamic-plugin-sdk/scripts/package-definitions.ts`:
- Line 83: The spread ordering gives VIRTUAL_DEPS higher precedence than real
deps; change the srcDeps construction so VIRTUAL_DEPS are spread first and
pkg.devDependencies and pkg.dependencies come after (i.e., build srcDeps from
VIRTUAL_DEPS, then ...pkg.devDependencies, then ...pkg.dependencies) so actual
root package entries override the virtual fallbacks; update the srcDeps
assignment that references VIRTUAL_DEPS, pkg.devDependencies, and
pkg.dependencies accordingly.
In
`@frontend/packages/console-dynamic-plugin-sdk/src/runtime/plugin-shared-modules.ts`:
- Around line 34-37: In patchSharedScope(), guard the alias assignment so you
only set scope['react-router-dom-v5-compat'] when scope['react-router-dom']
exists: retrieve the scope via getSharedScope(), check for the presence
(non-undefined) of the 'react-router-dom' entry and only then assign it to
'react-router-dom-v5-compat'; if missing, skip the alias (optionally log or
leave a TODO comment) to avoid creating an undefined compat binding.
In `@frontend/public/components/utils/router.ts`:
- Around line 89-107: The setQueryArgument callback currently deletes the key
when v === '' which diverges from URLSearchParams.set(k, '') semantics; update
setQueryArgument (the function that builds updated = new
URLSearchParams(searchParamsRef.current)) to always call updated.set(k, v) even
for empty strings (i.e., remove the if (v === '') { updated.delete(k) } branch)
so callers can preserve an empty-valued param, and add a brief inline comment
above setQueryArgument explaining that empty string values are intentionally
preserved (or, if deletion was intended, instead document that behavior and use
a distinct sentinel like null/undefined to mean “remove”).
ℹ️ Review info
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
⛔ Files ignored due to path filters (1)
frontend/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (16)
frontend/.eslintrc.jsfrontend/package.jsonfrontend/packages/console-app/src/hooks/usePluginRoutes.tsxfrontend/packages/console-dynamic-plugin-sdk/docs/console-extensions.mdfrontend/packages/console-dynamic-plugin-sdk/release-notes/4.22.mdfrontend/packages/console-dynamic-plugin-sdk/scripts/package-definitions.tsfrontend/packages/console-dynamic-plugin-sdk/src/extensions/console-types.tsfrontend/packages/console-dynamic-plugin-sdk/src/extensions/pages.tsfrontend/packages/console-dynamic-plugin-sdk/src/runtime/plugin-init.tsfrontend/packages/console-dynamic-plugin-sdk/src/runtime/plugin-shared-modules.tsfrontend/public/components/app.tsxfrontend/public/components/factory/modal.tsxfrontend/public/components/modals/delete-modal.tsxfrontend/public/components/utils/router.tsfrontend/tsconfig.jsonfrontend/webpack.config.ts
frontend/package.json
Outdated
| "react-router": "6.29.x", | ||
| "react-router-dom": "6.29.x", | ||
| "react-router-hash-link": "^2.0.0", |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
react-router-hash-link React Router v6 compatibility
💡 Result:
react-router-hash-link (rafgraph, latest 2.4.3, published May 13, 2021) was built for React Router v4/v5 and hasn’t been updated for React Router v6+; its own package metadata still describes it as “Hash link scroll functionality for React Router v4/5”. [1][2]
In practice, many people hit broken/quirky behavior after upgrading to react-router-dom v6 (hash updates but no scroll / inconsistent scroll), and common guidance is that it “doesn’t appear to be compatible with react-router v6”. [3][4]
What to do on React Router v6:
- Implement hash scrolling yourself (listen to
location.hashfromuseLocation()and callelement.scrollIntoView()), or - Use a v6-friendly alternative (e.g., the “scroll-to-hash-element” approach/package referenced in the v6 hash-links writeup). [3]
Sources:
[1] https://app.unpkg.com/react-router-hash-link%402.3.0/files/package.json
[2] https://classic.yarnpkg.com/en/package/react-router-hash-link
[3] https://ncoughlin.com/posts/react-router-v6-hash-links/
[4] https://stackoverflow.com/questions/70441115/smooth-scroll-not-working-with-react-router-hash-link
Remove or replace react-router-hash-link — incompatible with React Router v6.
react-router-hash-link ^2.0.0 (line 222) is incompatible with React Router v6. The library was built exclusively for v4/v5, and its maintainer has not updated it for v6. In practice, this causes hash scrolling to break or behave inconsistently at runtime.
Options:
- Remove the dependency and implement hash scrolling manually using
useLocation()andelement.scrollIntoView(), or - Replace with a v6-compatible alternative (e.g.,
scroll-to-hash-element).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/package.json` around lines 220 - 222, Remove the incompatible
dependency "react-router-hash-link" from package.json and either implement
manual hash scrolling or swap to a v6-compatible package: if implementing
manually, use React Router's useLocation() in your layout or a scroll handler
(e.g., in App or a ScrollManager component) to detect location.hash and call
document.getElementById(...).scrollIntoView({ behavior: 'smooth' }) (or
element.scrollIntoView) after navigation; if replacing, remove
"react-router-hash-link" and add a v6-compatible alternative such as
"scroll-to-hash-element" and update any Link usage to the new API. Ensure you
update package.json dependencies and any imports/usages referencing
react-router-hash-link.
| return useMemo( | ||
| () => | ||
| routePages.reduce( | ||
| ([activeAcc, inactiveAcc], extension) => { | ||
| const active = isRoutePageExtensionActive(extension, activePerspective); | ||
| const routes = getRoutesForExtension(extension); | ||
| const routes = mapExtensionToRoutes({ | ||
| uid: extension.uid, | ||
| path: extension.properties.path, | ||
| exact: extension.properties.exact, | ||
| getElement: () => ( | ||
| <RoutePage | ||
| extension={extension} | ||
| activePerspective={activePerspective} | ||
| setActivePerspective={setActivePerspective} | ||
| /> | ||
| ), | ||
| }); | ||
|
|
||
| return active | ||
| ? [[...activeAcc, ...routes], inactiveAcc] | ||
| : [activeAcc, [...inactiveAcc, ...routes]]; | ||
| }, | ||
| [[], []], | ||
| ), | ||
| [routePages, getRoutesForExtension, activePerspective], | ||
| [routePages, activePerspective, setActivePerspective], | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for useActivePerspective hook definition
rg -n "useActivePerspective|export.*useActivePerspective" --type=ts --type=tsx -g '!node_modules/**' | head -40Repository: openshift/console
Length of output: 88
🏁 Script executed:
#!/bin/bash
# Get the full useActivePerspective hook implementation
rg -n "const useActivePerspective|function useActivePerspective" --type=ts --type=tsx -A 30 -g '!node_modules/**'Repository: openshift/console
Length of output: 88
🏁 Script executed:
#!/bin/bash
# Search for useActivePerspective using pattern without type flags
rg "useActivePerspective" -g '!node_modules/**' -A 30 | head -100Repository: openshift/console
Length of output: 11108
🏁 Script executed:
#!/bin/bash
# Find files containing useActivePerspective
fd "\.tsx?$" -g '!node_modules/**' -g '!dist/**' -x rg -l "useActivePerspective" {} + 2>/dev/null | head -20Repository: openshift/console
Length of output: 43
🏁 Script executed:
#!/bin/bash
# Find the dynamic-plugin-sdk package
fd -t d "dynamic-plugin-sdk" -g '!node_modules/**' 2>/dev/nullRepository: openshift/console
Length of output: 43
🏁 Script executed:
#!/bin/bash
# Search for useActivePerspective definition in packages
find frontend/packages -name "*.ts" -o -name "*.tsx" | xargs rg -l "export.*useActivePerspective|const useActivePerspective" 2>/dev/null | head -10Repository: openshift/console
Length of output: 360
🏁 Script executed:
#!/bin/bash
# Read the useActivePerspective implementation
cat -n frontend/packages/console-dynamic-plugin-sdk/src/perspective/useActivePerspective.tsRepository: openshift/console
Length of output: 647
🏁 Script executed:
#!/bin/bash
# Check the index.ts export
cat -n frontend/packages/console-dynamic-plugin-sdk/src/perspective/index.tsRepository: openshift/console
Length of output: 187
🏁 Script executed:
#!/bin/bash
# Find the perspective-context file
fd "perspective-context" frontend/packages/console-dynamic-plugin-sdk/Repository: openshift/console
Length of output: 144
🏁 Script executed:
#!/bin/bash
# Read perspective-context implementation
cat -n frontend/packages/console-dynamic-plugin-sdk/src/perspective/perspective-context.tsRepository: openshift/console
Length of output: 785
🏁 Script executed:
#!/bin/bash
# Find where PerspectiveContext.Provider is used
rg "PerspectiveContext\\.Provider|<PerspectiveContext" -A 5 -g '!node_modules/**' | head -80Repository: openshift/console
Length of output: 2259
🏁 Script executed:
#!/bin/bash
# Also search for where the context value is created
rg "activePerspective.*setActivePerspective|setActivePerspective.*activePerspective" -B 3 -A 3 -g '!node_modules/**' | head -80Repository: openshift/console
Length of output: 8312
🏁 Script executed:
#!/bin/bash
# Read the full useValuesForPerspectiveContext implementation
cat -n frontend/packages/console-app/src/components/detect-perspective/useValuesForPerspectiveContext.tsRepository: openshift/console
Length of output: 2416
🏁 Script executed:
#!/bin/bash
# Confirm the usePluginRoutes useMemo dependency array one more time
sed -n '87,112p' frontend/packages/console-app/src/hooks/usePluginRoutes.tsxRepository: openshift/console
Length of output: 912
setActivePerspective in the memoization dependency array breaks optimization — lacks referential stability.
The setPerspective function returned by useValuesForPerspectiveContext (in DetectPerspective) is created inline without useCallback, generating a new reference on every render. This unstable reference flows through context → useActivePerspective() → usePluginRoutes, invalidating the useMemo on every render and defeating the memoization of the route reduce operation.
Wrap setPerspective in useCallback within useValuesForPerspectiveContext with dependencies [navigate, fireTelemetryEvent, setLastPerspective] to stabilize the reference across renders.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/packages/console-app/src/hooks/usePluginRoutes.tsx` around lines 87
- 112, The memo in usePluginRoutes is being invalidated by an unstable
setActivePerspective reference—wrap the setPerspective function created in
useValuesForPerspectiveContext (used by DetectPerspective) in useCallback to
provide referential stability; specifically, wrap setPerspective with
useCallback and use [navigate, fireTelemetryEvent, setLastPerspective] as its
dependency array so the setActivePerspective value passed into usePluginRoutes
remains stable and the useMemo in usePluginRoutes can actually memoize the
routePages reduce.
| - Upgraded `react-router-dom-v5-compat` to version `npm:react-router-dom@^6.30.x`. This change is API- | ||
| compatible with the previous version of `react-router-dom-v5-compat`. Plugins that need to support | ||
| versions of OpenShift starting from `4.14.x` can continue to use this deprecated shared module as is. |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for CompatRouter, CompatRoute, and patchSharedScope in the codebase
echo "=== Searching for CompatRouter, CompatRoute, patchSharedScope ==="
rg -rn 'CompatRouter|CompatRoute|patchSharedScope' --type ts --type tsx -g '!*.test.*' -g '!node_modules'Repository: openshift/console
Length of output: 171
🏁 Script executed:
#!/bin/bash
# Look for the react-router-dom-v5-compat alias/patch configuration
echo "=== Searching for react-router-dom alias/patch configuration ==="
rg -rn 'react-router-dom' --type ts --type tsx --type json -g '!node_modules' -g '!dist' | head -50Repository: openshift/console
Length of output: 170
🏁 Script executed:
#!/bin/bash
# Check the release notes file to understand the full context
echo "=== Checking release notes file ==="
cat -n frontend/packages/console-dynamic-plugin-sdk/release-notes/4.22.mdRepository: openshift/console
Length of output: 3538
🏁 Script executed:
#!/bin/bash
# Search for CompatRouter, CompatRoute, patchSharedScope without type restriction
echo "=== Searching for CompatRouter, CompatRoute, patchSharedScope ==="
rg -rn 'CompatRouter|CompatRoute|patchSharedScope' --max-count=50Repository: openshift/console
Length of output: 393
🏁 Script executed:
#!/bin/bash
# Search for react-router-dom references in config files
echo "=== Searching for react-router-dom in config/webpack ==="
rg -rn 'react-router-dom' -t json -t js | head -80Repository: openshift/console
Length of output: 921
🏁 Script executed:
#!/bin/bash
# Look for the actual plugin SDK package structure
echo "=== Listing console-dynamic-plugin-sdk directory ==="
find frontend/packages/console-dynamic-plugin-sdk -type f -name '*.ts' -o -name '*.js' -o -name '*.json' | grep -E '(webpack|tsconfig|package\.json|shared)' | head -20Repository: openshift/console
Length of output: 1106
🏁 Script executed:
#!/bin/bash
# Check for webpack configuration and aliases
echo "=== Searching for webpack config and aliases ==="
find . -name 'webpack*.js' -o -name 'webpack*.config.*' | head -10 | xargs ls -lahRepository: openshift/console
Length of output: 276
🏁 Script executed:
#!/bin/bash
# Read the plugin-shared-modules.ts file to see what's exported
echo "=== plugin-shared-modules.ts ==="
cat -n frontend/packages/console-dynamic-plugin-sdk/src/runtime/plugin-shared-modules.tsRepository: openshift/console
Length of output: 1677
🏁 Script executed:
#!/bin/bash
# Check the tsconfig.json for the actual path aliases
echo "=== frontend/tsconfig.json path aliases ==="
cat frontend/tsconfig.json | grep -A 20 '"paths"'Repository: openshift/console
Length of output: 1541
🏁 Script executed:
#!/bin/bash
# Search for "react-router-dom-v5-compat" (full name, not abbreviated)
echo "=== Searching for react-router-dom-v5-compat (full name) ==="
rg -rn 'react-router-dom-v5-compat' --max-count=50Repository: openshift/console
Length of output: 42006
🏁 Script executed:
#!/bin/bash
# Check the package.json files for the actual module configuration
echo "=== frontend/package.json react/router entries ==="
cat frontend/package.json | grep -A 5 -B 5 'react-router'Repository: openshift/console
Length of output: 516
Release note is misleading on backward compatibility—clarify that only standard React Router v6 APIs are supported.
The patchSharedScope implementation (in plugin-shared-modules.ts:36) only aliases the module reference: scope['react-router-dom-v5-compat'] = scope['react-router-dom']. It does not re-export CompatRouter or CompatRoute. Since vanilla react-router-dom@^6.29.x does not export these symbols, plugins importing them will receive undefined at runtime—a silent breaking change that contradicts "continue to use this deprecated shared module as is."
Update the note to explicitly state: "Plugins must migrate away from CompatRouter and CompatRoute before upgrading to 4.22, as only standard React Router v6 APIs are available in the shared module."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/packages/console-dynamic-plugin-sdk/release-notes/4.22.md` around
lines 14 - 16, The release note claims backward compatibility but
patchSharedScope in plugin-shared-modules.ts (the line setting
scope['react-router-dom-v5-compat'] = scope['react-router-dom']) only aliases
the module and does not re-export CompatRouter or CompatRoute, so plugins
importing CompatRouter/CompatRoute will get undefined; update the 4.22 release
note text to explicitly state that the shared module exposes only standard React
Router v6 APIs and add the sentence: "Plugins must migrate away from
CompatRouter and CompatRoute before upgrading to 4.22, as only standard React
Router v6 APIs are available in the shared module."
| const VIRTUAL_DEPS: PackageJson['dependencies'] = { | ||
| 'react-router-dom-v5-compat': 'npm:react-router-dom@^6.30.x', | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n frontend/packages/console-dynamic-plugin-sdk/scripts/package-definitions.ts | head -100Repository: openshift/console
Length of output: 4043
🏁 Script executed:
rg -n "sharedPluginModules" frontend/packages/console-dynamic-plugin-sdk/scripts/package-definitions.ts -A 20 -B 5Repository: openshift/console
Length of output: 1900
🏁 Script executed:
rg -n "parseSharedModuleDeps\|getMinDepVersion" frontend/packages/console-dynamic-plugin-sdk/scripts/package-definitions.ts -B 3 -A 3Repository: openshift/console
Length of output: 43
🏁 Script executed:
rg -n "VIRTUAL_DEPS\|srcDeps" frontend/packages/console-dynamic-plugin-sdk/scripts/package-definitions.ts -B 2 -A 2Repository: openshift/console
Length of output: 43
🏁 Script executed:
rg -r "react-router-dom-v5-compat" frontend/packages/console-dynamic-plugin-sdk/ --type ts --type jsonRepository: openshift/console
Length of output: 831
🏁 Script executed:
cat -n frontend/packages/console-dynamic-plugin-sdk/src/shared-modules/shared-modules-meta.ts | head -150Repository: openshift/console
Length of output: 2487
🏁 Script executed:
rg -n "getMinDepVersion" frontend/packages/console-dynamic-plugin-sdk/scripts/package-definitions.ts -A 2 -B 2Repository: openshift/console
Length of output: 396
🏁 Script executed:
grep -r "react-router-dom-v5-compat" frontend/packages/console-dynamic-plugin-sdk/src/ --include="*.ts" --include="*.tsx"Repository: openshift/console
Length of output: 931
🏁 Script executed:
sed -n '115,250p' frontend/packages/console-dynamic-plugin-sdk/scripts/package-definitions.tsRepository: openshift/console
Length of output: 2891
🏁 Script executed:
rg -n "parseSharedModuleDeps\|peerDependencies" frontend/packages/console-dynamic-plugin-sdk/scripts/package-definitions.ts -A 2 -B 2Repository: openshift/console
Length of output: 43
🏁 Script executed:
grep -r "npm:" frontend/packages/console-dynamic-plugin-sdk/package.jsonRepository: openshift/console
Length of output: 43
🏁 Script executed:
cat -n package.json | head -150Repository: openshift/console
Length of output: 105
🏁 Script executed:
ls -la | grep package.jsonRepository: openshift/console
Length of output: 43
🏁 Script executed:
fd package.json -d 1Repository: openshift/console
Length of output: 43
🏁 Script executed:
cat -n frontend/packages/console-dynamic-plugin-sdk/package.jsonRepository: openshift/console
Length of output: 1576
🏁 Script executed:
find . -maxdepth 2 -name "package.json" -exec grep -l "react-router-dom-v5-compat" {} \;Repository: openshift/console
Length of output: 95
🏁 Script executed:
cat frontend/dynamic-demo-plugin/package.json | grep -A 5 -B 5 "react-router-dom-v5-compat"Repository: openshift/console
Length of output: 134
🏁 Script executed:
rg -n "react-router-dom\|react-router-dom-v5-compat" frontend/package.json -A 2 -B 2Repository: openshift/console
Length of output: 43
🏁 Script executed:
find . -maxdepth 3 -name "package.json" -type f | head -20Repository: openshift/console
Length of output: 150
🏁 Script executed:
head -100 ./package.jsonRepository: openshift/console
Length of output: 134
🏁 Script executed:
head -150 ./frontend/package.json | grep -A 10 -B 2 "react-router"Repository: openshift/console
Length of output: 43
🏁 Script executed:
cat ./dynamic-demo-plugin/package.jsonRepository: openshift/console
Length of output: 3784
🏁 Script executed:
web_search
npm: protocol peerDependencies semver validationRepository: openshift/console
Length of output: 154
🌐 Web query:
npm: protocol peerDependencies package.json semver validation
💡 Result:
peerDependenciesvalues are meant to be SemVer ranges (compatibility constraints), not “where to fetch it from.” npm documentspeerDependenciesas expressing compatibility with a host and gives only SemVer-range examples ("2.x","^1.0","1.x",">= 1.5.2 < 2", etc.). (docs.npmjs.com)- npm validates versions/ranges using
node-semver. The npm docs explicitly say theversionfield must be parseable bynode-semver(the same library family used for range parsing/validation throughout npm). (docs.npmjs.com) - Protocol specifiers (eg
file:,git+https:,http(s):,npm:alias) are “package specifiers” that npm parses for installable dependencies, vianpm-package-arg. That parser recognizes types likerange,version,tag,file,directory,git,remote,alias, and throws on unsupported protocols. (npmjs.com) - So if you put a protocol in
peerDependencies, you’re outside the intended model (a peer is provided by the consumer, and npm needs a range to check satisfaction). Practically, this often shows up as parse errors like “Unsupported URL Type …” for protocols npm doesn’t support (notablyworkspace:). (github.com) - Behavior note: since npm v7, peer deps are installed by default and conflicts can become hard install errors (vs warnings in npm v3–v6). (docs.npmjs.com)
How to validate in tooling (what npm effectively does):
- Parse the spec like npm would:
npm-package-arg(rejects unsupported protocols). (npmjs.com) - If it’s a peer dep, additionally require it to be a SemVer range/version (
type === 'range' || type === 'version') and validate withsemver.validRange(...)/semver.valid(...)(fromnode-semver, referenced by npm docs). (docs.npmjs.com)
Citations:
- 1: https://docs.npmjs.com/cli/v9/configuring-npm/package-json
- 2: https://docs.npmjs.com/cli/v9/configuring-npm/package-json
- 3: https://www.npmjs.com/package/npm-package-arg?utm_source=openai
- 4: [RRFC] Support for
workspace:protocol (Yarn 2) npm/rfcs#765 - 5: https://docs.npmjs.com/cli/v9/configuring-npm/package-json
- 6: https://www.npmjs.com/package/npm-package-arg?utm_source=openai
- 7: https://docs.npmjs.com/cli/v9/configuring-npm/package-json
npm: protocol is not a valid SemVer range—will break generated peerDependencies.
The version string 'npm:react-router-dom@^6.30.x' is the npm install-time alias syntax, not a semver range. Since react-router-dom-v5-compat is in sharedPluginModules (line 34 of shared-modules-meta.ts), parseSharedModuleDeps will include it in the generated core package's peerDependencies (lines 135–136) with this alias as-is.
peerDependencies must contain valid SemVer ranges only. The npm: protocol is a package specifier (where-to-fetch), not a version range. npm and pnpm will reject this—pnpm explicitly errors with ERR_PNPM_INVALID_PEER_DEPENDENCY_SPECIFICATION when encountering protocol specifiers in peerDependencies.
🔧 Proposed fix
-const VIRTUAL_DEPS: PackageJson['dependencies'] = {
- 'react-router-dom-v5-compat': 'npm:react-router-dom@^6.30.x',
-};
+const VIRTUAL_DEPS: PackageJson['dependencies'] = {
+ // Semver range only; the npm: alias lives in the consuming package's dependencies.
+ 'react-router-dom-v5-compat': '^6.30.0',
+};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const VIRTUAL_DEPS: PackageJson['dependencies'] = { | |
| 'react-router-dom-v5-compat': 'npm:react-router-dom@^6.30.x', | |
| }; | |
| const VIRTUAL_DEPS: PackageJson['dependencies'] = { | |
| // Semver range only; the npm: alias lives in the consuming package's dependencies. | |
| 'react-router-dom-v5-compat': '^6.30.0', | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/packages/console-dynamic-plugin-sdk/scripts/package-definitions.ts`
around lines 74 - 76, The VIRTUAL_DEPS entry uses an npm install-time alias
('npm:react-router-dom@^6.30.x') which is not a valid SemVer range and will end
up in peerDependencies via parseSharedModuleDeps; change the value for
'react-router-dom-v5-compat' in VIRTUAL_DEPS to a plain SemVer range (e.g.
'^6.30.x') so parseSharedModuleDeps emits a valid peerDependency, and if you
still need the install-time aliasing, implement that at install/alias resolution
time (not inside VIRTUAL_DEPS) or map the virtual name to the actual package
name elsewhere in the resolver that synthesizes install aliases.
frontend/public/components/app.tsx
Outdated
| import { unstable_HistoryRouter as UnstableHistoryRouter } from 'react-router-dom'; | ||
| import { mapExtensionToRoutes } from '@console/app/src/hooks/usePluginRoutes'; | ||
| import { useParams, useLocation, Routes, Route } from 'react-router-dom-v5-compat'; |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
React Router v7 unstable_HistoryRouter removed
💡 Result:
unstable_HistoryRouter is not removed in React Router v7—it’s still exported, but it remains unstable and discouraged for most apps. In v7 it lives on the main react-router package exports. [1][2]
What likely changed for you in v7
- You may have upgraded and (correctly) switched imports from
react-router-domtoreact-router. The v7 upgrade guide recommends importing everything fromreact-router. [3] - The component is still named
unstable_HistoryRouter(there isn’t a stableHistoryRouterexport). [1][2]
How to use it in v7
- Import from
react-router:import { unstable_HistoryRouter } from "react-router";[2]
If you used a custom history to migrate v6 → v7
- React Router v7 added
UNSAFE_createBrowserHistoryplusUNSAFE_createMemoryHistory/UNSAFE_createHashHistoryspecifically to help apps usingunstable_HistoryRoutermigrate while avoiding mismatchedhistoryversions. [4]
Preferred alternative (if your goal is “navigate outside React”)
- Use a data router (
createBrowserRouter) and callrouter.navigate(...)from wherever you need, instead of maintaining your ownhistoryinstance. (This is the direction React Router has been pushing since v6.4+.) [3]
Sources:
[1] https://reactrouter.com/api/declarative-routers/unstable_HistoryRouter
[2] https://api.reactrouter.com/v7/modules/react-router.html
[3] https://reactrouter.com/upgrading/v6
[4] https://reactrouter.com/start/start/changelog
🏁 Script executed:
# Check the actual imports in app.tsx at lines 19-21
sed -n '1,30p' frontend/public/components/app.tsxRepository: openshift/console
Length of output: 1340
🏁 Script executed:
# Check project React Router version
cat package.json | grep -A 5 -B 5 react-routerRepository: openshift/console
Length of output: 105
🏁 Script executed:
# Search for unstable_HistoryRouter usage in the codebase
rg "unstable_HistoryRouter|UnstableHistoryRouter" --type tsx --type tsRepository: openshift/console
Length of output: 88
🏁 Script executed:
# Find package.json in the repo
find . -name "package.json" -type f | head -5Repository: openshift/console
Length of output: 276
🏁 Script executed:
# Search for unstable_HistoryRouter usage with correct syntax
rg "unstable_HistoryRouter|UnstableHistoryRouter" --type typescriptRepository: openshift/console
Length of output: 95
🏁 Script executed:
# Also check what's actually imported from react-router-dom-v5-compat
rg "from 'react-router-dom-v5-compat'" --type typescript -A 2Repository: openshift/console
Length of output: 95
🏁 Script executed:
# Check the main frontend package.json for react-router versions
cat frontend/package.json | grep -E "react-router|react-router-dom"Repository: openshift/console
Length of output: 164
🏁 Script executed:
# Search for unstable_HistoryRouter usage with correct file type
rg "unstable_HistoryRouter|UnstableHistoryRouter" -t ts -t tsxRepository: openshift/console
Length of output: 88
🏁 Script executed:
# Check all imports from react-router-dom and react-router-dom-v5-compat in app.tsx
rg "from ['\"]react-router" frontend/public/components/app.tsxRepository: openshift/console
Length of output: 228
unstable_HistoryRouter is a known trade-off; consolidate mixed imports.
The unstable_HistoryRouter carries an explicit stability warning and is the documented approach for maintaining Console's external history object (needed for programmatic navigation outside the React tree). Understand that this creates a dependency on react-router-dom 6.x as long as external history control is required; React Router v7 still exports it, but it remains unstable and the v7 upgrade guide recommends migrating to data routers with router.navigate(...) instead. This is a longer-term refactoring consideration if you can eliminate the external history dependency.
More immediately: consolidate the mixed imports. Line 19 imports from react-router-dom, line 21 from react-router-dom-v5-compat—pick one source and group all routing imports together.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/public/components/app.tsx` around lines 19 - 21, Consolidate the
mixed routing imports by importing all router symbols from the same package:
move the unstable_HistoryRouter (UnstableHistoryRouter) import so it's grouped
with useParams, useLocation, Routes, Route (or vice versa) instead of importing
from both 'react-router-dom' and 'react-router-dom-v5-compat'; update the import
statement(s) so that UnstableHistoryRouter, useParams, useLocation, Routes, and
Route come from a single source (either 'react-router-dom' or
'react-router-dom-v5-compat') and keep mapExtensionToRoutes import unchanged.
| {modalWrapper ? ( | ||
| <ModalWrapper blocking={blocking} className={modalClassName} onClose={handleClose}> | ||
| <Component {...(props as any)} cancel={handleCancel} close={handleClose} /> | ||
| </ModalWrapper> | ||
| ) : ( | ||
| <Component {...(props as any)} cancel={handleCancel} close={handleClose} /> | ||
| )} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== createModalLauncher call sites =="
rg -nP --type=ts --type=tsx '\bcreateModalLauncher\s*\(' -C2
echo
echo "== Files containing both createModalLauncher and React Router APIs =="
python3 - <<'PY'
from pathlib import Path
import re
launcher = re.compile(r'\bcreateModalLauncher\s*\(')
router = re.compile(
r'\b(useNavigate|useLocation|useParams|useSearchParams|useMatch|useResolvedPath)\b'
r'|<\s*(Link|NavLink|Navigate)\b'
)
matched_files = []
for p in Path('.').rglob('*'):
if p.suffix not in {'.ts', '.tsx'}:
continue
try:
text = p.read_text(encoding='utf-8', errors='ignore')
if launcher.search(text) and router.search(text):
matched_files.append(str(p))
except Exception as e:
pass
if matched_files:
print(f"Found {len(matched_files)} files:")
for f in sorted(matched_files):
print(f" {f}")
else:
print("No files found with both createModalLauncher and Router APIs")
PYRepository: openshift/console
Length of output: 142
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Searching for createModalLauncher in TS/TSX files =="
fd -e ts -e tsx | xargs rg -l 'createModalLauncher' 2>/dev/null | head -20
echo
echo "== Files with both createModalLauncher and Router APIs =="
python3 - <<'PY'
from pathlib import Path
import re
launcher = re.compile(r'\bcreateModalLauncher\s*\(')
router = re.compile(
r'\b(useNavigate|useLocation|useParams|useSearchParams|useMatch|useResolvedPath)\b'
r'|<\s*(Link|NavLink|Navigate)\b'
)
matched_files = []
for p in Path('.').rglob('*.ts'):
try:
text = p.read_text(encoding='utf-8', errors='ignore')
if launcher.search(text) and router.search(text):
matched_files.append(str(p))
except Exception:
pass
for p in Path('.').rglob('*.tsx'):
try:
text = p.read_text(encoding='utf-8', errors='ignore')
if launcher.search(text) and router.search(text):
matched_files.append(str(p))
except Exception:
pass
matched_files = sorted(set(matched_files))
if matched_files:
print(f"Found {len(matched_files)} files with both patterns:")
for f in matched_files:
print(f" {f}")
else:
print("No files found with both createModalLauncher and Router APIs")
PY
echo
echo "== Check the modal.tsx file directly =="
cat -n frontend/public/components/factory/modal.tsx | head -100Repository: openshift/console
Length of output: 1755
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== modal.tsx content (first 120 lines) =="
cat -n frontend/public/components/factory/modal.tsx | head -120
echo
echo "== Check confirm-modal.tsx for Router APIs =="
grep -n -E 'useNavigate|useLocation|useParams|<Link|<NavLink|<Navigate' frontend/public/components/modals/confirm-modal.tsx 2>/dev/null || echo "No Router APIs found"
echo
echo "== Check add-users-modal.tsx for Router APIs =="
grep -n -E 'useNavigate|useLocation|useParams|<Link|<NavLink|<Navigate' frontend/public/components/modals/add-users-modal.tsx 2>/dev/null || echo "No Router APIs found"
echo
echo "== Check all modals for Router APIs - comprehensive search =="
for file in frontend/public/components/modals/*.tsx; do
if grep -q -E 'useNavigate|useLocation|useParams|<Link|<NavLink|<Navigate' "$file" 2>/dev/null; then
echo " $file: USES ROUTER APIs"
fi
doneRepository: openshift/console
Length of output: 5570
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== delete-modal.tsx Router API usage =="
grep -n -E 'useNavigate|useLocation|useParams|<Link|<NavLink|<Navigate' frontend/public/components/modals/delete-modal.tsx
echo
echo "== cluster-update-modal.tsx Router API usage =="
grep -n -E 'useNavigate|useLocation|useParams|<Link|<NavLink|<Navigate' frontend/public/components/modals/cluster-update-modal.tsx
echo
echo "== Sample of delete-modal.tsx showing Router hook usage =="
grep -B3 -A3 'useNavigate\|useLocation' frontend/public/components/modals/delete-modal.tsx | head -30Repository: openshift/console
Length of output: 1140
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Check for Router/BrowserRouter usage in codebase =="
grep -r 'from .react-router' --include='*.ts' --include='*.tsx' | grep -i 'router\|browserrouter' | head -20
echo
echo "== Check App.tsx for router setup =="
find . -name 'App.tsx' -type f | head -5
echo
echo "== Look at main entry point for router context =="
grep -r 'BrowserRouter\|HistoryRouter' --include='*.ts' --include='*.tsx' | head -10Repository: openshift/console
Length of output: 2821
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Check main console app initialization for Router =="
fd -e tsx -e ts | xargs grep -l 'BrowserRouter\|HistoryRouter' | head -10
echo
echo "== Check index or root App file =="
find . -name 'index.tsx' -o -name 'App.tsx' | grep -E 'frontend/public|console-app' | head -5 | xargs ls -la 2>/dev/null
echo
echo "== Search for Router initialization in public folder =="
grep -r 'Router\|BrowserRouter' frontend/public --include='*.tsx' --include='*.ts' | grep -v node_modules | grep -v '.spec' | head -10Repository: openshift/console
Length of output: 291
🏁 Script executed:
#!/bin/bash
set -euoarpipefail
echo "== Check app.tsx Router context =="
grep -n 'BrowserRouter\|HistoryRouter\|Router' frontend/public/components/app.tsx | head -20
echo
echo "== Confirm modal mounting location (separate root) =="
grep -n 'modal-container' frontend/public/components/factory/modal.tsx
echo
echo "== Verify delete-modal is launched via createModalLauncher =="
grep -n 'createModalLauncher\|DeleteModal' frontend/public/components/modals/delete-modal.tsx | tail -5Repository: openshift/console
Length of output: 735
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Check app.tsx Router context =="
grep -n 'BrowserRouter\|HistoryRouter\|Router' frontend/public/components/app.tsx | head -20
echo
echo "== Confirm modal mounting location (separate root) =="
grep -n 'modal-container' frontend/public/components/factory/modal.tsx
echo
echo "== Verify delete-modal is launched via createModalLauncher =="
grep -n 'createModalLauncher\|DeleteModal' frontend/public/components/modals/delete-modal.tsx | tail -5Repository: openshift/console
Length of output: 1052
Router context missing in launcher-mounted modals.
Lines 74–80 render Component without a router provider. Since createModal mounts via ReactDOM.render() to a separate root (line 17), any modal using useNavigate(), useLocation(), or router components (Link, etc.) will fail at runtime with "must be used within a <Router>".
At least 7 modals already depend on Router APIs (e.g., DeleteModal, ClusterUpdateModal), making this a blocker for their use.
Fix: Wrap with HistoryRouter using the app's history instance
The app already initializes UnstableHistoryRouter with a shared history instance in app.tsx. Import and reuse it:
+import { unstable_HistoryRouter as HistoryRouter } from 'react-router-dom';
+import { history } from '../../history'; // shared instance from app.tsx setup
<Provider store={store}>
<PluginStoreProvider store={pluginStore}>
+ <HistoryRouter history={history}>
{modalWrapper ? (
<ModalWrapper blocking={blocking} className={modalClassName} onClose={handleClose}>
<Component {...(props as any)} cancel={handleCancel} close={handleClose} />
</ModalWrapper>
) : (
<Component {...(props as any)} cancel={handleCancel} close={handleClose} />
)}
+ </HistoryRouter>
</PluginStoreProvider>
</Provider>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {modalWrapper ? ( | |
| <ModalWrapper blocking={blocking} className={modalClassName} onClose={handleClose}> | |
| <Component {...(props as any)} cancel={handleCancel} close={handleClose} /> | |
| </ModalWrapper> | |
| ) : ( | |
| <Component {...(props as any)} cancel={handleCancel} close={handleClose} /> | |
| )} | |
| <HistoryRouter history={history}> | |
| {modalWrapper ? ( | |
| <ModalWrapper blocking={blocking} className={modalClassName} onClose={handleClose}> | |
| <Component {...(props as any)} cancel={handleCancel} close={handleClose} /> | |
| </ModalWrapper> | |
| ) : ( | |
| <Component {...(props as any)} cancel={handleCancel} close={handleClose} /> | |
| )} | |
| </HistoryRouter> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/public/components/factory/modal.tsx` around lines 74 - 80, The modal
rendering path mounts components outside the app router so any component using
router hooks (e.g., useNavigate/useLocation) fails; update the render in
ModalWrapper/Component branch to wrap the output with the app's HistoryRouter
(the UnstableHistoryRouter or HistoryRouter instance used in app.tsx) using the
shared history instance so router context is available to launched modals;
specifically import the UnstableHistoryRouter (or HistoryRouter) and the shared
history that app.tsx initializes, and wrap the JSX returned for both the
ModalWrapper and direct Component render inside that router (preserving
blocking, className, onClose, cancel, close props) so createModal-mounted
components like DeleteModal and ClusterUpdateModal receive router context.
| history.__replace__ = history.replace; | ||
| history.replace = (url: string) => (history as any).__replace__(removeBasePath(url)); | ||
|
|
||
| (history as any).__push__ = history.push; | ||
| history.__push__ = history.push; | ||
| history.push = (url: string) => (history as any).__push__(removeBasePath(url)); |
There was a problem hiding this comment.
Monkey-patched push/replace narrow the call signature, dropping state support.
The original history.push and history.replace in @remix-run/router accept (to: To, state?: any). The overrides on lines 45 and 48 only accept (url: string), which means:
- Any caller passing a location object (e.g.,
{ pathname, search, hash }) will silently misbehave —removeBasePathwill receive an object, and_.startsWith(obj, ...)returnsfalse, passing the object through as-is. - Navigation state is silently dropped — there's no way to pass state through the patched methods.
If this is intentional to discourage non-hook usage, the @deprecated JSDoc is good, but callers that do still use history.push(path, state) will lose their state without warning.
Consider at minimum preserving the state parameter:
Proposed fix
-history.__replace__ = history.replace;
-history.replace = (url: string) => (history as any).__replace__(removeBasePath(url));
+history.__replace__ = history.replace;
+history.replace = (url: string, state?: any) => (history as any).__replace__(removeBasePath(url), state);
-history.__push__ = history.push;
-history.push = (url: string) => (history as any).__push__(removeBasePath(url));
+history.__push__ = history.push;
+history.push = (url: string, state?: any) => (history as any).__push__(removeBasePath(url), state);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/public/components/utils/router.ts` around lines 44 - 48, The
monkey-patched overrides for history.replace/history.push
(history.__replace__/history.__push and the new history.replace/history.push)
drop the optional state parameter and mishandle non-string locations; update the
patched signatures to match the original (to: To, state?: any) and preserve
state when delegating to the originals; inside the new functions, detect if to
is a string and call removeBasePath(to) before passing it through, otherwise
pass the location object unchanged, and forward the optional state argument to
(history as any).__replace__ / __push__; also update typings/JSDoc for
history.replace and history.push to reflect the original signature.
63e9560 to
1f60c7a
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: vojtechszocs The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
logonoff
force-pushed
the
drop-react-router-v5
branch
3 times, most recently
from
2225c09 to
3b134b3
Compare
|
/uncc christoph-jerolimov |
logonoff
force-pushed
the
drop-react-router-v5
branch
4 times, most recently
from
48fa012 to
708a672
Compare
|
/remove-label tide/merge-method-squash |
openshift-ci
bot
removed
the
tide/merge-method-squash
logonoff
force-pushed
the
drop-react-router-v5
branch
from
708a672 to
6be4f11
Compare
|
/retitle CONSOLE-4439: Upgrade to React Router 7 |
This is a re-implementation of the previous commit head https://github.com/openshift/console/tree/1d70080fb482a10dec9084d92165779dc0ad3ff6 Except instead of rewriting every `react-router-dom-v5-compat` import we will alias them for the time being, to save ourselves from rebase hell Co-Authored-By: Vojtech Szocs <vojtech.szocs@gmail.com> Co-Authored-By: Marek Ziska <49416557+mylanos@users.noreply.github.com>
Replace the `react-router-hash-link` package with a custom `HashLink` component in `@console/shared`. The new implementation handles both same-page and cross-page hash navigation using React Router v7 APIs, with proper cleanup on unmount and support for smooth scrolling. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
@vojtechszocs: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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. |
8 participants
Work in progress 🚧
Summary by CodeRabbit
New Features
Updates