chore(navigation): adding type safety handleNavigation (podman-deskto…

…p#7933) * chore(navigation): adding type safety handleNavigation Signed-off-by: axel7083 <[email protected]> * fix: window event type Signed-off-by: axel7083 <[email protected]> * fix(NavigationManager): navigateTo typing Signed-off-by: axel7083 <[email protected]> * fix: updating test Signed-off-by: axel7083 <[email protected]> * fix: update webview panel Signed-off-by: axel7083 <[email protected]> * fix: rebase Signed-off-by: axel7083 <[email protected]> * fix: rename CONTAINER_EXPORT Signed-off-by: axel7083 <[email protected]> * fix: handleNavigation rebase Signed-off-by: axel7083 <[email protected]> --------- Signed-off-by: axel7083 <[email protected]>
axel7083 · Jul 11, 2024 · f2bbfcf · f2bbfcf
1 parent 5170029
commit f2bbfcf
Show file tree

Hide file tree

Showing 11 changed files with 102 additions and 48 deletions.
diff --git 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<T extends NavigationPage> = 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
@@ -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
@@ -36,7 +36,7 @@ export class NavigationManager {
     private webviewRegistry: WebviewRegistry,
   ) {}
 
-  navigateTo(navigateRequest: NavigationRequest): void {
+  navigateTo<T extends NavigationPage>(navigateRequest: NavigationRequest<T>): 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
@@ -152,7 +152,7 @@ export class WebviewPanelImpl implements WebviewPanel {
     this.assertNotDisposed();
 
     // notify the renderer to reveal the webview
-    const navigationRequest: NavigationRequest = {
+    const navigationRequest: NavigationRequest<NavigationPage.WEBVIEW> = {
       page: NavigationPage.WEBVIEW,
       parameters: {
         id: this.#internalId,

diff --git 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<any>);
 });
 </script>
 

diff --git 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<void> {
 }
 
 async function exportContainer(): Promise<void> {
-  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
@@ -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
@@ -21,7 +21,9 @@ $: recommendedRegistriesToInstall = registriesFilteredByIds.filter(registry =>
 );
 
 function goToAuthPage() {
-  handleNavigation(NavigationPage.AUTHENTICATION);
+  handleNavigation({
+    page: NavigationPage.AUTHENTICATION,
+  });
 }
 </script>
 

diff --git 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),
+    },
   });
 }
 </script>

diff --git 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
@@ -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> = T extends NavigationPage ? NavigationRequest<T> : 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<NavigationPage>) => {
+  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;
   }
 };