Update to version 6 of react-router (Issue #1869, PR #1879)

# Description

This PR has 2 parts. The first part is using the new api provided by react-router v6.
All of the routing logic changes are related to this.
It's mostly replacing references to history with location and navigate.
Typing for routing has also changed a bit (RouteMatch, useRouteParams).
The second part is changing the way an environment change is handled.
Most of the changes are related to removing the environment from the constructor.
And making sure the environmentHandler has an environment to provide to the QueryManagers.

closes #1869

# Self Check:

Strike through any lines that are not applicable (`~~line~~`) then check the box

- [x] Attached issue to pull request
- [x] Changelog entry
- [x] Code is clear and sufficiently documented
- [x] Sufficient test cases (reproduces the bug/tests the requested feature)
- [x] Correct, in line with design
- [ ] ~~End user documentation is included or an issue is created for end-user documentation (add ref to issue here: )~~

# Reviewer Checklist:

- [ ] Sufficient test cases (reproduces the bug/tests the requested feature)
- [ ] Code is clear and sufficiently documented
- [ ] Correct, in line with design

changelogs/unreleased/1869-react-router-update.yml

@@ -0,0 +1,4 @@
+description: Update to version 6 of react-router
+issue-nr: 1869
+change-type: patch
+destination-branches: [master, iso4]

cypress/integration/service_catalog.spec.js

@@ -49,6 +49,12 @@ describe("Service catalog", function () {
 
     cy.contains("Lifecycle States").click();
 
+    /**
+     * @NOTE This assertion indirectly verifies that no full page rerender is triggered.
+     * We do not have the prop unMountOnExit set to true for the Tabs component.
+     * This means, the TabContent of previous viewed tabs is not destroyed.
+     * So here we correctly assume there are 2 TabContent components in the DOM.
+     */
     cy.get("#e2e_service-expand")
       .find(".pf-c-tab-content")
       .should("have.length", 2);

package.json

@@ -62,13 +62,13 @@
     "@types/styled-components": "^5.1.15",
     "@types/webpack": "^5.28.0",
     "@types/xml-formatter": "^2.1.1",
-    "clean-css": "^5.2.2",
-    "copy-webpack-plugin": "^9.1.0",
-    "css-minimizer-webpack-plugin": "^3.1.3",
     "@typescript-eslint/eslint-plugin": "^5.3.1",
     "@typescript-eslint/parser": "^5.4.0",
     "babel-loader": "^8.2.3",
+    "clean-css": "^5.2.2",
+    "copy-webpack-plugin": "^9.1.0",
     "css-loader": "^6.5.1",
+    "css-minimizer-webpack-plugin": "^3.1.3",
     "cypress": "^9.0.0",
     "cypress-multi-reporters": "^1.5.0",
     "eslint": "^7.32.0",
@@ -139,7 +139,7 @@
     "process": "^0.11.10",
     "qs": "^6.10.1",
     "react-keycloak": "^6.1.1",
-    "react-router-dom": "^5.3.0",
+    "react-router-dom": "^6.0.1",
     "react-syntax-highlighter": "^15.4.4",
     "styled-components": "^5.3.3",
     "xml-formatter": "^2.5.1"

src/Core/Contracts/EnvironmentHandler.ts

@@ -1,6 +1,16 @@
+import { Location } from "history";
 import { FlatEnvironment } from "@/Core/Domain";
 
 export interface EnvironmentHandler {
-  set(environmentId: string): void;
+  set(location: Location, environmentId: string): void;
   useSelected(): FlatEnvironment | undefined;
+  /**
+   * If this function is called, it means the environment is required.
+   * If it is required, the environment should be defined.
+   * If it is not defined, something is wrong with the code.
+   * This should never happen during runtime.
+   * @returns the environment id
+   * @throws error when there is no environment defined
+   */
+  useId(): string;
 }

src/Core/Contracts/PageManager.ts

@@ -1,8 +1,8 @@
-import { ComponentType } from "react";
+import { ReactElement } from "react";
 import { Route } from "@/Core/Domain";
 
 export interface Page extends Route {
-  component: ComponentType;
+  element: ReactElement | null;
 }
 
 export interface PageManager {

src/Core/Contracts/RouteManager.ts

@@ -1,13 +1,12 @@
-import { RouteKind, Route, RouteParams } from "@/Core/Domain";
+import { RouteKind, Route, RouteParams, RouteMatch } from "@/Core/Domain";
 
 export type RouteDictionary = Record<RouteKind, Route>;
-export type MatchedParams = Record<string, string>;
 
 export interface RouteManager {
   getRoutes(): Route[];
   getRouteDictionary(): RouteDictionary;
   getRoute(routeKind: RouteKind): Route;
-  getUrl(kind: RouteKind, params: RouteParams<typeof kind>): string;
+  getUrl(kind: RouteKind, params: RouteParams<RouteKind>): string;
   /**
    * Gets the closest url in the lineage without params.
    * When switching environments, we can't go to pages with params,
@@ -21,5 +20,5 @@ export interface RouteManager {
    * @param routes the buildup of parent routes as this is a recursive function
    */
   getLineageFromRoute(route: Route, routes?: Route[]): Route[];
-  getRouteWithParamsFromUrl(url: string): [Route, MatchedParams] | undefined;
+  getRouteMatchFromUrl(url: string): RouteMatch | undefined;
 }

src/Core/Domain/Route.ts

@@ -28,23 +28,28 @@ export interface Route {
   clearEnv?: boolean;
 }
 
-interface RouteParamsManifest {
-  Catalog: undefined;
-  Inventory: { service: string };
-  CreateInstance: { service: string };
-  EditInstance: { service: string; instance: string };
-  History: { service: string; instance: string };
-  Events: { service: string; instance: string };
-  Diagnose: { service: string; instance: string };
-  Resources: undefined;
-  ResourceHistory: { resourceId: string };
-  ResourceLogs: { resourceId: string };
-  CompileReports: undefined;
-  CompileDetails: { id: string };
-  Home: undefined;
-  ResourceDetails: { resourceId: string };
-  Settings: undefined;
-  CreateEnvironment: undefined;
+interface RouteParamKeysManifest {
+  Inventory: "service";
+  CreateInstance: "service";
+  EditInstance: "service" | "instance";
+  History: "service" | "instance";
+  Events: "service" | "instance";
+  Diagnose: "service" | "instance";
+  ResourceHistory: "resourceId";
+  ResourceLogs: "resourceId";
+  CompileDetails: "id";
+  ResourceDetails: "resourceId";
 }
 
-export type RouteParams<R extends RouteKind> = RouteParamsManifest[R];
+export type RouteParamKeys<K extends RouteKind> =
+  K extends keyof RouteParamKeysManifest ? RouteParamKeysManifest[K] : never;
+
+export type RouteParams<K extends RouteKind> =
+  K extends keyof RouteParamKeysManifest
+    ? Record<RouteParamKeysManifest[K], string>
+    : undefined;
+
+export interface RouteMatch {
+  route: Route;
+  params: RouteParams<RouteKind>;
+}

src/Data/API/FileFetcherImpl.ts

@@ -1,5 +1,4 @@
-import { FileFetcher, Either, Maybe } from "@/Core";
-import { BaseApiHelper } from "./BaseApiHelper";
+import { FileFetcher, Either, Maybe, ApiHelper } from "@/Core";
 
 interface RawResponse {
   data?: {
@@ -11,10 +10,7 @@ interface RawResponse {
 export class FileFetcherImpl implements FileFetcher {
   private environment: Maybe.Type<string> = Maybe.none();
 
-  constructor(
-    private readonly baseApiHelper: BaseApiHelper,
-    environment?: string
-  ) {
+  constructor(private readonly apiHelper: ApiHelper, environment?: string) {
     if (typeof environment === "undefined") return;
     this.environment = Maybe.some(environment);
   }
@@ -38,7 +34,7 @@ export class FileFetcherImpl implements FileFetcher {
 
   async get(fileId: string): Promise<Either.Type<string, string>> {
     return this.unpack(
-      await this.baseApiHelper.get<RawResponse>(
+      await this.apiHelper.get<RawResponse>(
         this.getUrl(fileId),
         this.getEnvironment()
       )

src/Data/Common/UrlState/helpers.ts

@@ -1,4 +1,4 @@
-import { useHistory, useLocation } from "react-router-dom";
+import { useNavigate, useLocation } from "react-router-dom";
 import { RouteKind } from "@/Core";
 
 export interface Location {
@@ -7,9 +7,7 @@ export interface Location {
   hash: string;
 }
 
-export interface History {
-  replace(to: string): void;
-}
+export type Replace = (path: string) => void;
 
 export type Update<Data> = (data: Data) => void;
 
@@ -25,7 +23,7 @@ export interface StateConfig<Data> {
 type Handler<ConfigType, ReturnValue> = (
   config: ConfigType,
   location: Location,
-  history: History
+  replace: Replace
 ) => ReturnValue;
 
 type ProvidedHandler<ConfigType, ReturnValue> = (
@@ -37,7 +35,9 @@ export const provide = <ConfigType, ReturnValue>(
 ): ProvidedHandler<ConfigType, ReturnValue> => {
   return (config) => {
     const location = useLocation();
-    const history = useHistory();
-    return handler(config, location, history);
+    const navigate = useNavigate();
+    return handler(config, location, (path) =>
+      navigate(path, { replace: true })
+    );
   };
 };

src/Data/Common/UrlState/useUrlState.ts

@@ -1,6 +1,6 @@
 import { isObject } from "@/Core";
 import { SearchHelper } from "@/UI/Routing";
-import { provide, Location, History, Update, StateConfig } from "./helpers";
+import { provide, Location, Replace, Update, StateConfig } from "./helpers";
 
 const searchHelper = new SearchHelper();
 
@@ -9,7 +9,7 @@ export const useUrlState = provide(handleUrlState);
 export function handleUrlState<Data>(
   config: StateConfig<Data>,
   location: Location,
-  history: History
+  replace: Replace
 ): [Data, Update<Data>] {
   const parsedSearch = searchHelper.parse(location.search);
   const state = getKeyOrEmpty(parsedSearch, "state");
@@ -43,7 +43,7 @@ export function handleUrlState<Data>(
       },
     });
 
-    history.replace(`${location.pathname}${newSearch}${location.hash}`);
+    replace(`${location.pathname}${newSearch}${location.hash}`);
   };
 
   return [currentValue, setValue];

src/Data/Common/UrlState/useUrlStateWithExpansion.spec.ts

@@ -1,4 +1,3 @@
-import { getMockedLocation, mockedHistory } from "./helpers.mocked";
 import { handleUrlStateWithExpansion } from "./useUrlStateWithExpansion";
 
 test.each`
@@ -11,8 +10,8 @@ test.each`
   async ({ search, expectedValue }) => {
     const [value] = handleUrlStateWithExpansion(
       { route: "Inventory" },
-      getMockedLocation(search),
-      mockedHistory
+      { pathname: "", search, hash: "" },
+      () => undefined
     );
     expect(value).toEqual(expectedValue);
   }

src/Data/Common/UrlState/useUrlStateWithExpansion.ts

@@ -1,6 +1,6 @@
 import { isEqual, identity } from "lodash";
 import { toggleValueInList } from "@/Core";
-import { provide, Location, History, StateConfig, Update } from "./helpers";
+import { provide, Location, Replace, StateConfig, Update } from "./helpers";
 import { handleUrlState } from "./useUrlState";
 
 type IsExpanded = (id: string) => boolean;
@@ -16,7 +16,7 @@ export const useUrlStateWithExpansion = provide(
 export function handleUrlStateWithExpansion(
   config: Config,
   location: Location,
-  history: History
+  replace: Replace
 ): [string[], Update<string[]>] {
   return handleUrlState<string[]>(
     {
@@ -28,19 +28,19 @@ export function handleUrlStateWithExpansion(
       equals: isEqual,
     },
     location,
-    history
+    replace
   );
 }
 
 function handleUrlStateWithExpansionWrapped(
   config: Config,
   location: Location,
-  history: History
+  replace: Replace
 ): [IsExpanded, OnExpansion] {
   const [expandedKeys, setExpandedKeys] = handleUrlStateWithExpansion(
     config,
     location,
-    history
+    replace
   );
 
   return [

src/Data/Common/UrlState/useUrlStateWithFilter.spec.ts

@@ -1,4 +1,3 @@
-import { getMockedLocation, mockedHistory } from "./helpers.mocked";
 import { handleUrlStateWithFilter } from "./useUrlStateWithFilter";
 
 const from = {
@@ -24,8 +23,8 @@ test.each`
   async ({ search, expectedValue }) => {
     const [value] = handleUrlStateWithFilter(
       { route: "Inventory" },
-      getMockedLocation(search),
-      mockedHistory
+      { pathname: "", search, hash: "" },
+      () => undefined
     );
     expect(value).toEqual(expectedValue);
   }

src/Data/Common/UrlState/useUrlStateWithFilter.ts

@@ -1,14 +1,14 @@
 import { isEqual, pickBy } from "lodash";
 import { isObject, DateRange, isNotUndefined } from "@/Core";
-import { provide, Location, History, StateConfig, Update } from "./helpers";
+import { provide, Location, StateConfig, Update, Replace } from "./helpers";
 import { handleUrlState } from "./useUrlState";
 
 export const useUrlStateWithFilter = provide(handleUrlStateWithFilter);
 
 export function handleUrlStateWithFilter<Data>(
   config: Pick<StateConfig<Data>, "route"> & { dateRangeKey?: string },
   location: Location,
-  history: History
+  replace: Replace
 ): [Data, Update<Data>] {
   const serialize = (data: Data): string | Data => {
     if (!config.dateRangeKey) return data;
@@ -43,6 +43,6 @@ export function handleUrlStateWithFilter<Data>(
       parse,
     },
     location,
-    history
+    replace
   );
 }

src/Data/Common/UrlState/useUrlStateWithPageSize.spec.ts

@@ -1,5 +1,4 @@
 import { PageSize } from "@/Core";
-import { getMockedLocation, mockedHistory } from "./helpers.mocked";
 import { handleUrlStateWithPageSize } from "./useUrlStateWithPageSize";
 
 test.each`
@@ -11,8 +10,8 @@ test.each`
   async ({ search, expectedValue }) => {
     const [value] = handleUrlStateWithPageSize(
       { route: "Inventory" },
-      getMockedLocation(search),
-      mockedHistory
+      { pathname: "", search, hash: "" },
+      () => undefined
     );
     expect(value).toEqual(expectedValue);
   }

src/Data/Common/UrlState/useUrlStateWithPageSize.ts

@@ -1,13 +1,13 @@
 import { PageSize } from "@/Core";
-import { provide, Location, History, StateConfig, Update } from "./helpers";
+import { provide, Location, Replace, StateConfig, Update } from "./helpers";
 import { handleUrlState } from "./useUrlState";
 
 export const useUrlStateWithPageSize = provide(handleUrlStateWithPageSize);
 
 export function handleUrlStateWithPageSize(
   config: Pick<StateConfig<PageSize.Type>, "route">,
   location: Location,
-  history: History
+  replace: Replace
 ): [PageSize.Type, Update<PageSize.Type>] {
   return handleUrlState<PageSize.Type>(
     {
@@ -19,6 +19,6 @@ export function handleUrlStateWithPageSize(
       equals: PageSize.equals,
     },
     location,
-    history
+    replace
   );
 }