diff --git a/src/features/navigating-between-routes.test.tsx b/src/features/navigating-between-routes.test.tsx index bcff0e2a11..3a4a009086 100644 --- a/src/features/navigating-between-routes.test.tsx +++ b/src/features/navigating-between-routes.test.tsx @@ -75,7 +75,7 @@ describe("navigating between routes", () => { }); it("does not have path parameters", () => { - const pathParameters = windowDi.inject(routePathParametersInjectable, route); + const pathParameters = windowDi.inject(routePathParametersInjectable)(route); expect(pathParameters.get()).toEqual({}); }); @@ -116,7 +116,10 @@ describe("navigating between routes", () => { }); describe("when navigating to route with path parameters", () => { - let route: Route; + let route: Route<{ + someParameter?: string | undefined; + someOtherParameter?: string | undefined; + }>; beforeEach(() => { route = windowDi.inject(routeWithOptionalPathParametersInjectable); @@ -150,7 +153,7 @@ describe("navigating between routes", () => { }); it("knows path parameters", () => { - const pathParameters = windowDi.inject(routePathParametersInjectable, route); + const pathParameters = windowDi.inject(routePathParametersInjectable)(route); expect(pathParameters.get()).toEqual({ someParameter: "some-value", @@ -160,7 +163,10 @@ describe("navigating between routes", () => { }); describe("when navigating to route without path parameters", () => { - let route: Route; + let route: Route<{ + someParameter?: string | undefined; + someOtherParameter?: string | undefined; + }>; beforeEach(() => { route = windowDi.inject(routeWithOptionalPathParametersInjectable); @@ -183,7 +189,7 @@ describe("navigating between routes", () => { }); it("knows path parameters", () => { - const pathParameters = windowDi.inject(routePathParametersInjectable, route); + const pathParameters = windowDi.inject(routePathParametersInjectable)(route); expect(pathParameters.get()).toEqual({ someParameter: undefined, @@ -232,7 +238,7 @@ const routeWithOptionalPathParametersComponentInjectable = getInjectable({ instantiate: (di) => { const route = di.inject(routeWithOptionalPathParametersInjectable); - const pathParameters = di.inject(routePathParametersInjectable, route); + const pathParameters = di.inject(routePathParametersInjectable)(route); return { route, diff --git a/src/renderer/components/+catalog/catalog-route-parameters.injectable.ts b/src/renderer/components/+catalog/catalog-route-parameters.injectable.ts index d5aafe263e..35c65c7497 100644 --- a/src/renderer/components/+catalog/catalog-route-parameters.injectable.ts +++ b/src/renderer/components/+catalog/catalog-route-parameters.injectable.ts @@ -15,8 +15,8 @@ const catalogRouteParametersInjectable = getInjectable({ const pathParameters = di.inject(routePathParametersInjectable)(route); return { - group: computed(() => pathParameters.get().group), - kind: computed(() => pathParameters.get().kind), + group: computed(() => pathParameters.get()?.group), + kind: computed(() => pathParameters.get()?.kind), }; }, }); diff --git a/src/renderer/components/+custom-resources/custom-resources-route-parameters.injectable.ts b/src/renderer/components/+custom-resources/custom-resources-route-parameters.injectable.ts index a0671fded9..3e0ab46fa8 100644 --- a/src/renderer/components/+custom-resources/custom-resources-route-parameters.injectable.ts +++ b/src/renderer/components/+custom-resources/custom-resources-route-parameters.injectable.ts @@ -15,8 +15,8 @@ const customResourcesRouteParametersInjectable = getInjectable({ const pathParameters = di.inject(routePathParametersInjectable)(route); return { - group: computed(() => pathParameters.get().group), - name: computed(() => pathParameters.get().name), + group: computed(() => pathParameters.get()?.group), + name: computed(() => pathParameters.get()?.name), }; }, }); diff --git a/src/renderer/components/+custom-resources/sidebar-items-for-definition-groups.injectable.ts b/src/renderer/components/+custom-resources/sidebar-items-for-definition-groups.injectable.ts index 895df08dc4..04719d2be5 100644 --- a/src/renderer/components/+custom-resources/sidebar-items-for-definition-groups.injectable.ts +++ b/src/renderer/components/+custom-resources/sidebar-items-for-definition-groups.injectable.ts @@ -24,7 +24,7 @@ const sidebarItemsForDefinitionGroupsInjectable = getInjectable({ const crdRoute = di.inject(customResourcesRouteInjectable); const crdRouteIsActive = di.inject(routeIsActiveInjectable, crdRoute); const crdListRoute = di.inject(crdListRouteInjectable); - const pathParameters = di.inject(routePathParametersInjectable, crdRoute); + const pathParameters = di.inject(routePathParametersInjectable)(crdRoute); const navigateToCustomResources = di.inject(navigateToCustomResourcesInjectable); return computed((): SidebarItemRegistration[] => { @@ -51,9 +51,15 @@ const sidebarItemsForDefinitionGroupsInjectable = getInjectable({ onClick: () => navigateToCustomResources(crdPathParameters), isActive: computed( - () => - crdRouteIsActive.get() && - matches(crdPathParameters, pathParameters.get()), + () => { + const params = pathParameters.get(); + + if (!crdRouteIsActive.get() || !params) { + return false; + } + + return matches(crdPathParameters, params); + }, ), isVisible: crdListRoute.isEnabled, diff --git a/src/renderer/components/+entity-settings/entity-settings-route-parameters.injectable.ts b/src/renderer/components/+entity-settings/entity-settings-route-parameters.injectable.ts index 5adf55c72b..926d5771e0 100644 --- a/src/renderer/components/+entity-settings/entity-settings-route-parameters.injectable.ts +++ b/src/renderer/components/+entity-settings/entity-settings-route-parameters.injectable.ts @@ -15,7 +15,7 @@ const entitySettingsRouteParametersInjectable = getInjectable({ const pathParameters = di.inject(routePathParametersInjectable)(route); return { - entityId: computed(() => pathParameters.get().entityId), + entityId: computed(() => pathParameters.get()?.entityId), }; }, }); diff --git a/src/renderer/components/+helm-charts/helm-charts-route-parameters.injectable.ts b/src/renderer/components/+helm-charts/helm-charts-route-parameters.injectable.ts index bc9b3cc31d..ef93d44c8c 100644 --- a/src/renderer/components/+helm-charts/helm-charts-route-parameters.injectable.ts +++ b/src/renderer/components/+helm-charts/helm-charts-route-parameters.injectable.ts @@ -15,8 +15,8 @@ const helmChartsRouteParametersInjectable = getInjectable({ const pathParameters = di.inject(routePathParametersInjectable)(route); return { - chartName: computed(() => pathParameters.get().chartName), - repo: computed(() => pathParameters.get().repo), + chartName: computed(() => pathParameters.get()?.chartName), + repo: computed(() => pathParameters.get()?.repo), }; }, }); diff --git a/src/renderer/components/+helm-releases/helm-releases-route-parameters.injectable.ts b/src/renderer/components/+helm-releases/helm-releases-route-parameters.injectable.ts index 685bb3d8e9..251d33ae92 100644 --- a/src/renderer/components/+helm-releases/helm-releases-route-parameters.injectable.ts +++ b/src/renderer/components/+helm-releases/helm-releases-route-parameters.injectable.ts @@ -15,8 +15,8 @@ const helmReleasesRouteParametersInjectable = getInjectable({ const pathParameters = di.inject(routePathParametersInjectable)(route); return { - namespace: computed(() => pathParameters.get().namespace), - name: computed(() => pathParameters.get().name), + namespace: computed(() => pathParameters.get()?.namespace), + name: computed(() => pathParameters.get()?.name), }; }, }); diff --git a/src/renderer/components/+network-port-forwards/port-forwards-route-parameters.injectable.ts b/src/renderer/components/+network-port-forwards/port-forwards-route-parameters.injectable.ts index b1c77bc958..209b799336 100644 --- a/src/renderer/components/+network-port-forwards/port-forwards-route-parameters.injectable.ts +++ b/src/renderer/components/+network-port-forwards/port-forwards-route-parameters.injectable.ts @@ -15,7 +15,7 @@ const portForwardsRouteParametersInjectable = getInjectable({ const pathParameters = di.inject(routePathParametersInjectable)(route); return { - forwardport: computed(() => pathParameters.get().forwardport), + forwardport: computed(() => pathParameters.get()?.forwardport), }; }, }); diff --git a/src/renderer/components/cluster-manager/cluster-frame-handler.injectable.ts b/src/renderer/components/cluster-manager/cluster-frame-handler.injectable.ts index 4354b78722..96b7ca1fb0 100644 --- a/src/renderer/components/cluster-manager/cluster-frame-handler.injectable.ts +++ b/src/renderer/components/cluster-manager/cluster-frame-handler.injectable.ts @@ -7,14 +7,14 @@ import getClusterByIdInjectable from "../../../common/cluster/get-by-id.injectab import loggerInjectable from "../../../common/logger.injectable"; import sendSetVisibleClusterInjectable from "../../cluster/send-set-visible.injectable"; import { ClusterFrameHandler } from "./cluster-frame-handler"; -import clusterFrameParentElementInjectable from "./parent-element.injectable"; +import getElementByIdInjectable from "./get-element-by-id.injectable"; const clusterFrameHandlerInjectable = getInjectable({ id: "cluster-frame-handler", instantiate: (di) => new ClusterFrameHandler({ getClusterById: di.inject(getClusterByIdInjectable), sendSetVisibleCluster: di.inject(sendSetVisibleClusterInjectable), - parentElem: di.inject(clusterFrameParentElementInjectable), + getElementById: di.inject(getElementByIdInjectable), logger: di.inject(loggerInjectable), }), }); diff --git a/src/renderer/components/cluster-manager/cluster-frame-handler.ts b/src/renderer/components/cluster-manager/cluster-frame-handler.ts index 060747168f..ce00470794 100644 --- a/src/renderer/components/cluster-manager/cluster-frame-handler.ts +++ b/src/renderer/components/cluster-manager/cluster-frame-handler.ts @@ -11,6 +11,7 @@ import assert from "assert"; import type { GetClusterById } from "../../../common/cluster/get-by-id.injectable"; import type { SendSetVisibleCluster } from "../../cluster/send-set-visible.injectable"; import type { Logger } from "../../../common/logger"; +import type { GetElementById } from "./get-element-by-id.injectable"; export interface LensView { isLoaded: boolean; @@ -20,7 +21,7 @@ export interface LensView { export interface ClusterFrameHandlerDependencies { getClusterById: GetClusterById; sendSetVisibleCluster: SendSetVisibleCluster; - readonly parentElem: HTMLElement; + getElementById: GetElementById; readonly logger: Logger; } @@ -38,6 +39,7 @@ export class ClusterFrameHandler { @action public initView(clusterId: ClusterId) { const cluster = this.dependencies.getClusterById(clusterId); + const parentElem = this.dependencies.getElementById("lens-views"); if (!cluster || this.views.has(clusterId)) { return; @@ -57,7 +59,7 @@ export class ClusterFrameHandler { view.isLoaded = true; }), { once: true }); this.views.set(clusterId, { frame: iframe, isLoaded: false }); - this.dependencies.parentElem.appendChild(iframe); + parentElem.appendChild(iframe); this.dependencies.logger.info(`[LENS-VIEW]: waiting cluster to be ready, clusterId=${clusterId}`); @@ -79,7 +81,7 @@ export class ClusterFrameHandler { () => { this.dependencies.logger.info(`[LENS-VIEW]: remove dashboard, clusterId=${clusterId}`); this.views.delete(clusterId); - this.dependencies.parentElem.removeChild(iframe); + parentElem.removeChild(iframe); dispose(); }, ); diff --git a/src/renderer/components/cluster-manager/cluster-view-route-parameters.injectable.ts b/src/renderer/components/cluster-manager/cluster-view-route-parameters.injectable.ts index b89aa8742d..c06800c0a9 100644 --- a/src/renderer/components/cluster-manager/cluster-view-route-parameters.injectable.ts +++ b/src/renderer/components/cluster-manager/cluster-view-route-parameters.injectable.ts @@ -15,7 +15,7 @@ const clusterViewRouteParametersInjectable = getInjectable({ const pathParameters = di.inject(routePathParametersInjectable)(route); return { - clusterId: computed(() => pathParameters.get().clusterId), + clusterId: computed(() => pathParameters.get()?.clusterId), }; }, }); diff --git a/src/renderer/components/cluster-manager/get-element-by-id.injectable.ts b/src/renderer/components/cluster-manager/get-element-by-id.injectable.ts new file mode 100644 index 0000000000..9fc3511850 --- /dev/null +++ b/src/renderer/components/cluster-manager/get-element-by-id.injectable.ts @@ -0,0 +1,23 @@ +/** + * Copyright (c) OpenLens Authors. All rights reserved. + * Licensed under MIT License. See LICENSE in root directory for more information. + */ +import { getInjectable } from "@ogre-tools/injectable"; + +export type GetElementById = (id: string) => HTMLElement; + +const getElementByIdInjectable = getInjectable({ + id: "get-element-by-id", + instantiate: (): GetElementById => (id) => { + const element = document.getElementById(id); + + if (!element) { + throw new Error(`Failed to find element: ${id}`); + } + + return element; + }, + causesSideEffects: true, +}); + +export default getElementByIdInjectable; diff --git a/src/renderer/components/cluster-manager/parent-element.injectable.ts b/src/renderer/components/cluster-manager/parent-element.injectable.ts deleted file mode 100644 index 7f9eafb7a6..0000000000 --- a/src/renderer/components/cluster-manager/parent-element.injectable.ts +++ /dev/null @@ -1,20 +0,0 @@ -/** - * Copyright (c) OpenLens Authors. All rights reserved. - * Licensed under MIT License. See LICENSE in root directory for more information. - */ -import { getInjectable } from "@ogre-tools/injectable"; -import assert from "assert"; - -const clusterFrameParentElementInjectable = getInjectable({ - id: "cluster-frame-parent-element", - instantiate: () => { - const elem = document.getElementById("#lens-view"); - - assert(elem, "DOM with #lens-views must be present"); - - return elem; - }, - causesSideEffects: true, -}); - -export default clusterFrameParentElementInjectable; diff --git a/src/renderer/getDiForUnitTesting.tsx b/src/renderer/getDiForUnitTesting.tsx index 9dcf88550a..82541a78f4 100644 --- a/src/renderer/getDiForUnitTesting.tsx +++ b/src/renderer/getDiForUnitTesting.tsx @@ -65,7 +65,8 @@ import setupSystemCaInjectable from "./frames/root-frame/setup-system-ca.injecta import extensionShouldBeEnabledForClusterFrameInjectable from "./extension-loader/extension-should-be-enabled-for-cluster-frame.injectable"; import { asyncComputed } from "@ogre-tools/injectable-react"; import forceUpdateModalRootFrameComponentInjectable from "./application-update/force-update-modal/force-update-modal-root-frame-component.injectable"; -import clusterFrameParentElementInjectable from "./components/cluster-manager/parent-element.injectable"; +import getElementByIdInjectable from "./components/cluster-manager/get-element-by-id.injectable"; +import { getOrInsertWith } from "./utils"; import legacyOnChannelListenInjectable from "./ipc/legacy-channel-listen.injectable"; import getEntitySettingCommandsInjectable from "./components/command-palette/registered-commands/get-entity-setting-commands.injectable"; import storageSaveDelayInjectable from "./utils/create-storage/storage-save-delay.injectable"; @@ -109,7 +110,11 @@ export const getDiForUnitTesting = (opts: { doGeneralOverrides?: boolean } = {}) di.override(terminalSpawningPoolInjectable, () => document.createElement("div")); di.override(hostedClusterIdInjectable, () => undefined); - di.override(clusterFrameParentElementInjectable, () => document.createElement("div")); + di.override(getElementByIdInjectable, () => { + const elements = new Map(); + + return (id) => getOrInsertWith(elements, id, () => document.createElement("div")); + }); di.override(getAbsolutePathInjectable, () => getAbsolutePathFake); di.override(joinPathsInjectable, () => joinPathsFake); diff --git a/src/renderer/routes/route-path-parameters.injectable.ts b/src/renderer/routes/route-path-parameters.injectable.ts index e105476314..41c9358bef 100644 --- a/src/renderer/routes/route-path-parameters.injectable.ts +++ b/src/renderer/routes/route-path-parameters.injectable.ts @@ -15,15 +15,15 @@ const routePathParametersInjectable = getInjectable({ instantiate: (di) => { const currentPath = di.inject(currentPathInjectable); - const pathParametersCache = new Map, IComputedValue>>>(); + const pathParametersCache = new Map, IComputedValue>(); - return (route: Route): IComputedValue> => ( + return (route: Route): IComputedValue => ( getOrInsertWith(pathParametersCache, route, () => computed(() => ( - matchPath(currentPath.get(), { + matchPath(currentPath.get(), { path: route.path, exact: true, - })?.params ?? {} - ))) + })?.params ?? null + ))) as IComputedValue ); }, });