Skip to content

OCPBUGS-85121: Skip redundant navigate() in setPerspective when target matches current URL #16683

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
TheRealJon wants to merge 2 commits into
base: main
Choose a base branch
from
Open

OCPBUGS-85121: Skip redundant navigate() in setPerspective when target matches current URL #16683

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2128951
OCPBUGS-85121: Skip redundant navigate() in setPerspective when targe…
logonoff Jun 25, 2026
4c0b96e
OCPBUGS-85121: Navigate to landing page when switching perspectives
TheRealJon Jun 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 13 additions & 4 deletions frontend/packages/console-app/src/components/detect-context/DetectContext.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import {
PageSidebarBody,
} from '@patternfly/react-core';
import { useTranslation } from 'react-i18next';
import { createPath, useLocation } from 'react-router';
import { createPath, useLocation, useNavigate } from 'react-router';
import type { Perspective, ReduxReducer, ContextProvider } from '@console/dynamic-plugin-sdk';
import {
PerspectiveContext,
Expand Down Expand Up @@ -168,12 +168,21 @@ export const DetectContext: FC<{ children: ReactNode }> = ({ children }) => {
const perspectiveExtensions = usePerspectives();
const perspectiveParam = getPerspectiveURLParam(perspectiveExtensions);
const location = useLocation();
const navigate = useNavigate();

useEffect(() => {
if (perspectiveParam && perspectiveParam !== activePerspective) {
setActivePerspective(perspectiveParam, createPath(location));
if (perspectiveParam) {
const params = new URLSearchParams(location.search);
params.delete('perspective');
const search = params.toString();
const cleanPath = createPath({ ...location, search: search ? `?${search}` : '' });
if (perspectiveParam !== activePerspective) {
setActivePerspective(perspectiveParam, cleanPath);
} else {
navigate(cleanPath, { replace: true });
}
}
}, [perspectiveParam, activePerspective, setActivePerspective, location]);
}, [perspectiveParam, activePerspective, setActivePerspective, navigate, location]);

useEffect(() => {
if (reducersResolved) {
Expand Down
3 changes: 2 additions & 1 deletion frontend/packages/console-app/src/components/detect-context/__tests__/DetectContext.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,8 @@ jest.mock('@console/internal/redux', () => ({

jest.mock('react-router', () => ({
useLocation: jest.fn(),
createPath: jest.fn((loc) => loc.pathname),
useNavigate: jest.fn(() => jest.fn()),
createPath: jest.fn((loc) => `${loc.pathname}${loc.search || ''}${loc.hash || ''}`),
}));

const useValuesForPerspectiveContextMock = useValuesForPerspectiveContext as jest.Mock;
Expand Down
8 changes: 5 additions & 3 deletions ...tend/packages/console-app/src/components/detect-context/useValuesForPerspectiveContext.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { useCallback, useState } from 'react';
import { useNavigate } from 'react-router';
import { createPath, useNavigate } from 'react-router';
import type { PerspectiveType, UseActivePerspective } from '@console/dynamic-plugin-sdk';
import {
usePerspectiveExtension,
Expand Down Expand Up @@ -38,8 +38,10 @@ export const useValuesForPerspectiveContext = (): [
(newPerspective, next) => {
setLastPerspective(newPerspective);
setActivePerspective(newPerspective);
// Navigate to next or root and let the default page determine where to go to next
navigate(next || '/');
// Only navigate if a path is provided to avoid unnecessary navigation
if (next && next !== createPath(window.location)) {
navigate(next);
}

@coderabbitai coderabbitai Bot Jun 25, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# How basename is configured for the router and how setActivePerspective `next` args are produced
rg -nP '\bbasename\b' frontend/public frontend/packages -g '*.ts*' -C2
rg -nP 'setActivePerspective\s*\(' frontend/public frontend/packages -g '*.ts*' -C2
rg -nP 'createPath\s*\(' frontend/packages -g '*.ts*' -C2

Repository: openshift/console

Length of output: 9757

🏁 Script executed:

sed -n '1,220p' frontend/packages/console-app/src/components/detect-context/useValuesForPerspectiveContext.ts
printf '\n---\n'
sed -n '1,240p' frontend/packages/console-app/src/components/detect-context/DetectContext.tsx
printf '\n---\n'
sed -n '1,140p' frontend/packages/console-app/src/components/detect-context/PerspectiveDetector.tsx
printf '\n---\n'
sed -n '1,120p' frontend/public/components/app.tsx

Repository: openshift/console

Length of output: 19626

🏁 Script executed:

sed -n '1,220p' frontend/packages/console-app/src/components/detect-context/useValuesForPerspectiveContext.ts
printf '\n---\n'
sed -n '1,240p' frontend/packages/console-app/src/components/detect-context/DetectContext.tsx
printf '\n---\n'
sed -n '1,140p' frontend/packages/console-app/src/components/detect-context/PerspectiveDetector.tsx
printf '\n---\n'
sed -n '330,360p' frontend/public/components/app.tsx

Repository: openshift/console

Length of output: 14933

🌐 Web query:

React Router BrowserRouter basename useLocation pathname basename-relative createPath window.location

💡 Result:

In React Router (v6 and later), the basename is integrated into the routing system to simplify path management [1][2]. When you configure a BrowserRouter with a basename (e.g., ), the router handles the basename automatically for most routing operations [2][3]. Key behaviors regarding basename and paths: 1. useLocation and basename: The useLocation hook returns a location object where the basename is stripped from the pathname [1]. For instance, if the browser URL is /app/home, useLocation will return a pathname of /home [1]. This ensures that your routes, defined as path="/home", match correctly without manual adjustment [1]. 2. Raw path access: If you specifically need the "full" URL including the basename, you should rely on the browser's native window.location.pathname [4]. The location object provided by React Router is intentionally abstracted to be relative to the application's root (the basename). 3. Relative Path Resolution (useResolvedPath and createPath): - The useResolvedPath hook resolves relative paths against the current location's pathname [5][6]. Because React Router handles the basename internally, paths resolved via this hook correctly respect the application structure [3]. - The createPath utility creates a string URL path from pathname, search, and hash components [7]. It is generally used for constructing paths to be consumed by navigation APIs; it does not automatically prepend a basename unless that basename is part of the provided path structure or handled by the router's internal navigation logic [8]. 4. Best Practices: - Avoid using window.location.pathname for routing logic inside your components; always use hooks like useLocation and useResolvedPath to ensure compatibility with your basename configuration [4]. - When performing manual path matching, use the location object provided by the router, as it provides the basename-stripped path, ensuring your logic remains consistent across different environments (e.g., development vs. production under different sub-paths) [4]. In summary, React Router v6+ abstracts the basename away from the location pathname, making it easier to write path-agnostic code while maintaining correct routing behavior [1][4].

Citations:

🏁 Script executed:

rg -n "createPath\(window\.location\)|window\.location\.pathname|createPath\(location\)" frontend/packages/console-app frontend/public frontend/packages/console-shared -C2

Repository: openshift/console

Length of output: 8225

🌐 Web query:

React Router basename useLocation pathname includes basename or stripped createPath(location)

💡 Result:

In React Router (v6 and later), the behavior of useLocation and basename is as follows: 1. useLocation pathname: The pathname returned by the useLocation hook has the basename stripped [1]. React Router automatically removes the configured basename from the current URL pathname so that your application code can work with paths relative to your application root [1][2]. 2. createPath behavior: The createPath utility function is a low-level helper that simply takes a partial location object (containing pathname, search, and hash) and joins them into a string [3]. It does not automatically prepend or strip a basename; it is basename-agnostic [3]. When navigating or creating links, React Router’s internal navigation logic (such as the navigate function or Link components) handles the basename automatically by prepending it when performing actions [4][5]. If you are manually constructing paths for navigation and want to respect the basename, you should rely on React Router's navigation hooks or utilities that are aware of the router context rather than manually appending the basename yourself [4][5]. If you specifically need to use createPath in a scenario involving a basename, you must manually prepend the basename to the pathname if necessary, as createPath will not perform this transformation for you [4][5].

Citations:

🏁 Script executed:

rg -n '"react-router|react-router-dom"' frontend/package.json frontend/packages/**/package.json -C1

Repository: openshift/console

Length of output: 321

Compare against the router location, not window.location. This app uses BrowserRouter basename={window.SERVER_FLAGS.basePath}, and the next values passed here are built from createPath(location), so they’re basename-relative. window.location includes the basename, which makes the guard miss same-route matches and trigger an unnecessary navigate. Use useLocation() for the comparison and add it to the callback deps.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@frontend/packages/console-app/src/components/detect-context/useValuesForPerspectiveContext.ts`
around lines 41 - 44, The navigation guard in useValuesForPerspectiveContext is
comparing next against createPath(window.location), but next is built from the
router location and is basename-relative, so this can incorrectly trigger
navigate on the same route. Update the comparison to use the current router
location from useLocation() instead of window.location, and make sure the
location value is included in the callback dependencies alongside navigate and
createPath.

fireTelemetryEvent('Perspective Changed', { perspective: newPerspective });
},
[setLastPerspective, setActivePerspective, navigate, fireTelemetryEvent],
Expand Down