From f2bbfcfb8d8ebcae8fe8f8bd88d24304ba55fbe2 Mon Sep 17 00:00:00 2001 From: axel7083 <42176370+axel7083@users.noreply.github.com> Date: Thu, 11 Jul 2024 09:37:02 +0200 Subject: [PATCH] chore(navigation): adding type safety handleNavigation (#7933) * chore(navigation): adding type safety handleNavigation Signed-off-by: axel7083 <42176370+axel7083@users.noreply.github.com> * fix: window event type Signed-off-by: axel7083 <42176370+axel7083@users.noreply.github.com> * fix(NavigationManager): navigateTo typing Signed-off-by: axel7083 <42176370+axel7083@users.noreply.github.com> * fix: updating test Signed-off-by: axel7083 <42176370+axel7083@users.noreply.github.com> * fix: update webview panel Signed-off-by: axel7083 <42176370+axel7083@users.noreply.github.com> * fix: rebase Signed-off-by: axel7083 <42176370+axel7083@users.noreply.github.com> * fix: rename CONTAINER_EXPORT Signed-off-by: axel7083 <42176370+axel7083@users.noreply.github.com> * fix: handleNavigation rebase Signed-off-by: axel7083 <42176370+axel7083@users.noreply.github.com> --------- Signed-off-by: axel7083 <42176370+axel7083@users.noreply.github.com> --- packages/api/src/navigation-request.ts | 36 +++++++++++++++-- .../main/src/plugin/extension-loader.spec.ts | 1 - .../plugin/navigation/navigation-manager.ts | 3 +- .../src/plugin/webview/webview-panel-impl.ts | 2 +- packages/renderer/src/App.svelte | 4 +- .../src/lib/container/ContainerActions.svelte | 37 ++++++++++++----- .../src/lib/container/ContainerExport.svelte | 4 +- .../src/lib/image/RecommendedRegistry.svelte | 4 +- .../renderer/src/lib/pod/PodColumnName.svelte | 11 +++-- packages/renderer/src/navigation.spec.ts | 8 ++-- packages/renderer/src/navigation.ts | 40 ++++++++++--------- 11 files changed, 102 insertions(+), 48 deletions(-) diff --git a/packages/api/src/navigation-request.ts b/packages/api/src/navigation-request.ts index 4bfe5dcee0f75..6b85efb04d844 100644 --- a/packages/api/src/navigation-request.ts +++ b/packages/api/src/navigation-request.ts @@ -18,7 +18,37 @@ import type { NavigationPage } from './navigation-page.js'; -export interface NavigationRequest { - page: NavigationPage; - parameters?: { [key: string]: string }; +// Define the type mapping for parameters +export interface NavigationParameters { + [NavigationPage.CONTAINERS]: never; + [NavigationPage.CONTAINER]: { id: string }; + [NavigationPage.CONTAINER_EXPORT]: { id: string }; + [NavigationPage.CONTAINER_LOGS]: { id: string }; + [NavigationPage.CONTAINER_INSPECT]: { id: string }; + [NavigationPage.CONTAINER_TERMINAL]: { id: string }; + [NavigationPage.CONTAINER_KUBE]: { id: string }; + [NavigationPage.DEPLOY_TO_KUBE]: { id: string; engineId: string }; + [NavigationPage.IMAGES]: never; + [NavigationPage.IMAGE]: { id: string; engineId: string; tag: string }; + [NavigationPage.PODS]: never; + [NavigationPage.POD]: { kind: string; name: string; engineId: string }; + [NavigationPage.VOLUMES]: never; + [NavigationPage.VOLUME]: { name: string }; + [NavigationPage.CONTRIBUTION]: { name: string }; + [NavigationPage.TROUBLESHOOTING]: never; + [NavigationPage.HELP]: never; + [NavigationPage.WEBVIEW]: { id: string }; + [NavigationPage.AUTHENTICATION]: never; + [NavigationPage.RESOURCES]: never; + [NavigationPage.EDIT_CONTAINER_CONNECTION]: { provider: string; name: string }; } + +// the parameters property is optional when the NavigationParameters say it is +export type NavigationRequest = NavigationParameters[T] extends never + ? { + page: T; + } + : { + page: T; + parameters: NavigationParameters[T]; + }; diff --git a/packages/main/src/plugin/extension-loader.spec.ts b/packages/main/src/plugin/extension-loader.spec.ts index 52037c53b11e7..7ace0f8cdcf48 100644 --- a/packages/main/src/plugin/extension-loader.spec.ts +++ b/packages/main/src/plugin/extension-loader.spec.ts @@ -1436,7 +1436,6 @@ describe('Navigation', async () => { page: NavigationPage.VOLUME, parameters: { name: 'valid-name', - engineId: 'valid-engine', }, }); diff --git a/packages/main/src/plugin/navigation/navigation-manager.ts b/packages/main/src/plugin/navigation/navigation-manager.ts index c54aa28b4692c..5bba36e18fec6 100644 --- a/packages/main/src/plugin/navigation/navigation-manager.ts +++ b/packages/main/src/plugin/navigation/navigation-manager.ts @@ -36,7 +36,7 @@ export class NavigationManager { private webviewRegistry: WebviewRegistry, ) {} - navigateTo(navigateRequest: NavigationRequest): void { + navigateTo(navigateRequest: NavigationRequest): void { this.apiSender.send('navigate', navigateRequest); } @@ -147,7 +147,6 @@ export class NavigationManager { page: NavigationPage.VOLUME, parameters: { name: name, - engineId: engineId, }, }); } diff --git a/packages/main/src/plugin/webview/webview-panel-impl.ts b/packages/main/src/plugin/webview/webview-panel-impl.ts index a61aa3eb90fb6..f13bc2dab22fa 100644 --- a/packages/main/src/plugin/webview/webview-panel-impl.ts +++ b/packages/main/src/plugin/webview/webview-panel-impl.ts @@ -152,7 +152,7 @@ export class WebviewPanelImpl implements WebviewPanel { this.assertNotDisposed(); // notify the renderer to reveal the webview - const navigationRequest: NavigationRequest = { + const navigationRequest: NavigationRequest = { page: NavigationPage.WEBVIEW, parameters: { id: this.#internalId, diff --git a/packages/renderer/src/App.svelte b/packages/renderer/src/App.svelte index f88d8cebf87a0..44e789e09f6d4 100644 --- a/packages/renderer/src/App.svelte +++ b/packages/renderer/src/App.svelte @@ -77,8 +77,8 @@ router.subscribe(function (navigation) { }); window.events?.receive('navigate', (navigationRequest: unknown) => { - const navRequest = navigationRequest as NavigationRequest; - handleNavigation(navRequest.page, navRequest.parameters); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + handleNavigation(navigationRequest as NavigationRequest); }); diff --git a/packages/renderer/src/lib/container/ContainerActions.svelte b/packages/renderer/src/lib/container/ContainerActions.svelte index 0c353d6a773cf..efa09670bc8d5 100644 --- a/packages/renderer/src/lib/container/ContainerActions.svelte +++ b/packages/renderer/src/lib/container/ContainerActions.svelte @@ -97,8 +97,11 @@ function openBrowser(): void { } function openLogs(): void { - handleNavigation(NavigationPage.CONTAINER_LOGS, { - id: container.id, + handleNavigation({ + page: NavigationPage.CONTAINER_LOGS, + parameters: { + id: container.id, + }, }); } @@ -114,27 +117,39 @@ async function deleteContainer(): Promise { } async function exportContainer(): Promise { - handleNavigation(NavigationPage.CONTAINER_EXPORT, { - id: container.id, + handleNavigation({ + page: NavigationPage.CONTAINER_EXPORT, + parameters: { + id: container.id, + }, }); } function openTerminalContainer(): void { - handleNavigation(NavigationPage.CONTAINER_TERMINAL, { - id: container.id, + handleNavigation({ + page: NavigationPage.CONTAINER_TERMINAL, + parameters: { + id: container.id, + }, }); } function openGenerateKube(): void { - handleNavigation(NavigationPage.CONTAINER_KUBE, { - id: container.id, + handleNavigation({ + page: NavigationPage.CONTAINER_KUBE, + parameters: { + id: container.id, + }, }); } function deployToKubernetes(): void { - handleNavigation(NavigationPage.DEPLOY_TO_KUBE, { - id: container.id, - engineId: container.engineId, + handleNavigation({ + page: NavigationPage.DEPLOY_TO_KUBE, + parameters: { + id: container.id, + engineId: container.engineId, + }, }); } diff --git a/packages/renderer/src/lib/container/ContainerExport.svelte b/packages/renderer/src/lib/container/ContainerExport.svelte index f6816428d5691..38c34390d07b6 100644 --- a/packages/renderer/src/lib/container/ContainerExport.svelte +++ b/packages/renderer/src/lib/container/ContainerExport.svelte @@ -35,7 +35,9 @@ onMount(() => { if (matchingContainer) { container = containerUtils.getContainerInfoUI(matchingContainer); } else { - handleNavigation(NavigationPage.CONTAINERS); + handleNavigation({ + page: NavigationPage.CONTAINERS, + }); } }); }); diff --git a/packages/renderer/src/lib/image/RecommendedRegistry.svelte b/packages/renderer/src/lib/image/RecommendedRegistry.svelte index a4824310f9bd1..d4e3a6370b996 100644 --- a/packages/renderer/src/lib/image/RecommendedRegistry.svelte +++ b/packages/renderer/src/lib/image/RecommendedRegistry.svelte @@ -21,7 +21,9 @@ $: recommendedRegistriesToInstall = registriesFilteredByIds.filter(registry => ); function goToAuthPage() { - handleNavigation(NavigationPage.AUTHENTICATION); + handleNavigation({ + page: NavigationPage.AUTHENTICATION, + }); } diff --git a/packages/renderer/src/lib/pod/PodColumnName.svelte b/packages/renderer/src/lib/pod/PodColumnName.svelte index 604afc40a7f50..fe7c778fc315a 100644 --- a/packages/renderer/src/lib/pod/PodColumnName.svelte +++ b/packages/renderer/src/lib/pod/PodColumnName.svelte @@ -10,10 +10,13 @@ export let object: PodInfoUI; const podUtils = new PodUtils(); function openDetailsPod(pod: PodInfoUI) { - handleNavigation(NavigationPage.POD, { - kind: encodeURI(pod.kind), - name: encodeURI(pod.name), - engineId: encodeURIComponent(pod.engineId), + handleNavigation({ + page: NavigationPage.POD, + parameters: { + kind: encodeURI(pod.kind), + name: encodeURI(pod.name), + engineId: encodeURIComponent(pod.engineId), + }, }); } diff --git a/packages/renderer/src/navigation.spec.ts b/packages/renderer/src/navigation.spec.ts index 892aae85f13e0..b8dde28041d3e 100644 --- a/packages/renderer/src/navigation.spec.ts +++ b/packages/renderer/src/navigation.spec.ts @@ -33,25 +33,25 @@ vi.mock('tinro', () => { }); test('Test navigationHandle to a specific container', () => { - handleNavigation(NavigationPage.CONTAINER, { id: '123' }); + handleNavigation({ page: NavigationPage.CONTAINER, parameters: { id: '123' } }); expect(vi.mocked(router.goto)).toHaveBeenCalledWith('/containers/123/'); }); test('Test navigationHandle to a specific webview', () => { - handleNavigation(NavigationPage.WEBVIEW, { id: '123' }); + handleNavigation({ page: NavigationPage.WEBVIEW, parameters: { id: '123' } }); expect(vi.mocked(router.goto)).toHaveBeenCalledWith('/webviews/123'); }); test('Test navigationHandle to resources page', () => { - handleNavigation(NavigationPage.RESOURCES); + handleNavigation({ page: NavigationPage.RESOURCES }); expect(vi.mocked(router.goto)).toHaveBeenCalledWith('/preferences/resources'); }); test('Test navigationHandle to a specific edit page', () => { - handleNavigation(NavigationPage.EDIT_CONTAINER_CONNECTION, { provider: '123', name: 'test' }); + handleNavigation({ page: NavigationPage.EDIT_CONTAINER_CONNECTION, parameters: { provider: '123', name: 'test' } }); expect(vi.mocked(router.goto)).toHaveBeenCalledWith('/preferences/container-connection/edit/123/test'); }); diff --git a/packages/renderer/src/navigation.ts b/packages/renderer/src/navigation.ts index 81de2325c2812..d0ae6213d949c 100644 --- a/packages/renderer/src/navigation.ts +++ b/packages/renderer/src/navigation.ts @@ -19,6 +19,11 @@ import { router } from 'tinro'; import { NavigationPage } from '/@api/navigation-page'; +import type { NavigationRequest } from '/@api/navigation-request'; + +// help method to ensure the handleNavigation is able to infer type properly through the switch +// ref https://www.typescriptlang.org/docs/handbook/2/conditional-types.html#distributive-conditional-types +type InferredNavigationRequest = T extends NavigationPage ? NavigationRequest : never; /** * Navigation hints for setting current page and history (breadcrumbs): @@ -29,55 +34,54 @@ import { NavigationPage } from '/@api/navigation-page'; */ export type NavigationHint = 'root' | 'details' | 'tab'; -export const handleNavigation = (page: NavigationPage, parameters?: { [key: string]: string }) => { - switch (page) { +export const handleNavigation = (request: InferredNavigationRequest) => { + switch (request.page) { case NavigationPage.CONTAINERS: router.goto('/containers'); break; case NavigationPage.CONTAINER_EXPORT: - router.goto(`/containers/${parameters?.['id']}/export`); + router.goto(`/containers/${request.parameters.id}/export`); break; case NavigationPage.CONTAINER: - router.goto(`/containers/${parameters?.['id']}/`); + router.goto(`/containers/${request.parameters.id}/`); break; case NavigationPage.CONTAINER_LOGS: - router.goto(`/containers/${parameters?.['id']}/logs`); + router.goto(`/containers/${request.parameters.id}/logs`); break; case NavigationPage.CONTAINER_INSPECT: - router.goto(`/containers/${parameters?.['id']}/inspect`); + router.goto(`/containers/${request.parameters.id}/inspect`); break; case NavigationPage.CONTAINER_TERMINAL: - router.goto(`/containers/${parameters?.['id']}/terminal`); + router.goto(`/containers/${request.parameters.id}/terminal`); break; case NavigationPage.CONTAINER_KUBE: - router.goto(`/containers/${parameters?.['id']}/kube`); + router.goto(`/containers/${request.parameters.id}/kube`); break; case NavigationPage.DEPLOY_TO_KUBE: - router.goto(`/deploy-to-kube/${parameters?.['id']}/${parameters?.['engineId']}`); + router.goto(`/deploy-to-kube/${request.parameters.id}/${request.parameters.engineId}`); break; case NavigationPage.IMAGES: router.goto(`/images`); break; case NavigationPage.IMAGE: - if (parameters) { - const tagBase64 = Buffer.from(parameters['tag']).toString('base64'); - router.goto(`/images/${parameters['id']}/${parameters['engineId']}/${tagBase64}`); - } + router.goto( + `/images/${request.parameters.id}/${request.parameters.engineId}/${Buffer.from(request.parameters.tag).toString('base64')}`, + ); break; case NavigationPage.PODS: router.goto(`/pods`); break; case NavigationPage.POD: - router.goto(`/pods/${parameters?.['kind']}/${parameters?.['name']}/${parameters?.['engineId']}/`); + router.goto(`/pods/${request.parameters.kind}/${request.parameters.name}/${request.parameters.engineId}/`); break; case NavigationPage.VOLUMES: router.goto('/volumes'); break; case NavigationPage.VOLUME: - router.goto(`/volumes/${parameters?.['name']}/`); + router.goto(`/volumes/${request.parameters.name}/`); break; case NavigationPage.CONTRIBUTION: - router.goto(`/contribs/${parameters?.['name']}/`); + router.goto(`/contribs/${request.parameters.name}/`); break; case NavigationPage.TROUBLESHOOTING: router.goto('/troubleshooting/repair-connections'); @@ -86,7 +90,7 @@ export const handleNavigation = (page: NavigationPage, parameters?: { [key: stri router.goto('/help'); break; case NavigationPage.WEBVIEW: - router.goto(`/webviews/${parameters?.['id']}`); + router.goto(`/webviews/${request.parameters.id}`); break; case NavigationPage.AUTHENTICATION: router.goto('/preferences/authentication-providers'); @@ -95,7 +99,7 @@ export const handleNavigation = (page: NavigationPage, parameters?: { [key: stri router.goto('/preferences/resources'); break; case NavigationPage.EDIT_CONTAINER_CONNECTION: - router.goto(`/preferences/container-connection/edit/${parameters?.['provider']}/${parameters?.['name']}`); + router.goto(`/preferences/container-connection/edit/${request.parameters.provider}/${request.parameters.name}`); break; } };