add strict null checks for web-common/src/features/models/ (#3749)

* fix Refernces * fix ModelInspectorHeader * fix ModelWorkspaceHeadre * fix ModelWorkspaceCTAs * fix ModelBody * fix Editor * fix ModelNames * fix ModelAssets * remove errant cvomment
rilldata · Jan 1, 2024 · 711889d · 711889d · github-actions · Jan 1, 2024
1 parent 018ae8a
commit 711889d
Show file tree

Hide file tree

Showing 10 changed files with 80 additions and 71 deletions.
diff --git a/web-common/src/features/entity-management/file-selectors.ts b/web-common/src/features/entity-management/file-selectors.ts
@@ -36,7 +36,7 @@ export function useMainEntityFiles(
             // sort alphabetically case-insensitive
             .sort((a, b) =>
               a.localeCompare(b, undefined, { sensitivity: "base" })
-            ),
+            ) ?? [],
       },
     }
   );

diff --git a/web-common/src/features/models/navigation/ModelAssets.svelte b/web-common/src/features/models/navigation/ModelAssets.svelte
@@ -17,12 +17,13 @@
   import ModelTooltip from "./ModelTooltip.svelte";
 
   $: sourceNames = useSourceFileNames($runtime.instanceId);
-  $: modelNames = useModelFileNames($runtime.instanceId);
+  $: useModelNames = useModelFileNames($runtime.instanceId);
 
+  $: modelNames = $useModelNames?.data ?? [];
   let showModels = true;
 
   async function handleAddModel() {
-    await createModel($runtime.instanceId, getName("model", $modelNames.data));
+    await createModel($runtime.instanceId, getName("model", modelNames));
     // if the models are not visible in the assets list, show them.
     if (!showModels) {
       showModels = true;
@@ -31,14 +32,16 @@
 
   /** rename the model */
   let showRenameModelModal = false;
-  let renameModelName = null;
+  let renameModelName: string | null = null;
   const openRenameModelModal = (modelName: string) => {
     showRenameModelModal = true;
     renameModelName = modelName;
   };
 
   $: hasSourceButNoModels =
-    $sourceNames?.data?.length > 0 && $modelNames?.data?.length === 0;
+    $sourceNames?.data?.length !== undefined &&
+    $sourceNames?.data?.length > 0 &&
+    modelNames.length === 0;
 </script>
 
 <NavigationHeader bind:show={showModels} toggleText="models"
@@ -51,35 +54,34 @@
     transition:slide|global={{ duration: LIST_SLIDE_DURATION }}
     id="assets-model-list"
   >
-    {#if $modelNames?.data}
-      {#each $modelNames.data as modelName (modelName)}
-        <NavigationEntry
-          name={modelName}
-          href={`/model/${modelName}`}
-          open={$page.url.pathname === `/model/${modelName}`}
-        >
-          <svelte:fragment slot="more">
-            <div transition:slide={{ duration: LIST_SLIDE_DURATION }}>
-              <ColumnProfile indentLevel={1} objectName={modelName} />
-            </div>
-          </svelte:fragment>
+    {#each modelNames as modelName (modelName)}
+      <NavigationEntry
+        name={modelName}
+        href={`/model/${modelName}`}
+        open={$page.url.pathname === `/model/${modelName}`}
+      >
+        <svelte:fragment slot="more">
+          <div transition:slide={{ duration: LIST_SLIDE_DURATION }}>
+            <ColumnProfile indentLevel={1} objectName={modelName} />
+          </div>
+        </svelte:fragment>
 
-          <svelte:fragment slot="tooltip-content">
-            <ModelTooltip {modelName} />
-          </svelte:fragment>
+        <svelte:fragment slot="tooltip-content">
+          <ModelTooltip {modelName} />
+        </svelte:fragment>
+
+        <svelte:fragment slot="menu-items" let:toggleMenu>
+          <ModelMenuItems
+            {modelName}
+            {toggleMenu}
+            on:rename-asset={() => {
+              openRenameModelModal(modelName);
+            }}
+          />
+        </svelte:fragment>
+      </NavigationEntry>
+    {/each}
 
-          <svelte:fragment slot="menu-items" let:toggleMenu>
-            <ModelMenuItems
-              {modelName}
-              {toggleMenu}
-              on:rename-asset={() => {
-                openRenameModelModal(modelName);
-              }}
-            />
-          </svelte:fragment>
-        </NavigationEntry>
-      {/each}
-    {/if}
     <AddAssetButton
       id="create-model-button"
       label="Add model"
@@ -89,7 +91,7 @@
   </div>
 {/if}
 
-{#if showRenameModelModal}
+{#if showRenameModelModal && renameModelName !== null}
   <RenameAssetModal
     entityType={EntityType.Model}
     closeModal={() => (showRenameModelModal = false)}

diff --git a/web-common/src/features/models/navigation/ModelMenuItems.svelte b/web-common/src/features/models/navigation/ModelMenuItems.svelte
@@ -53,12 +53,14 @@
   }
 
   const handleDeleteModel = async (modelName: string) => {
-    await deleteFileArtifact(
-      $runtime.instanceId,
-      modelName,
-      EntityType.Model,
-      $modelNames.data
-    );
+    if ($modelNames.data) {
+      await deleteFileArtifact(
+        $runtime.instanceId,
+        modelName,
+        EntityType.Model,
+        $modelNames.data
+      );
+    }
     toggleMenu();
   };
 </script>

diff --git a/web-common/src/features/models/tsconfig.json b/web-common/src/features/models/tsconfig.json
diff --git a/web-common/src/features/models/workspace/Editor.svelte b/web-common/src/features/models/workspace/Editor.svelte
@@ -97,8 +97,10 @@
   $: if ($allSourceColumns?.length) {
     for (const sourceTable of $allSourceColumns) {
       const sourceIdentifier = sourceTable?.tableName;
-      schema[sourceIdentifier] =
-        sourceTable.profileColumns?.map((c) => c.name) ?? [];
+      schema[sourceIdentifier] = sourceTable.profileColumns
+        ?.filter((c) => c.name !== undefined)
+        // CAST SAFETY: already filtered out undefined values
+        .map((c) => c.name as string);
     }
   }
 
@@ -107,18 +109,21 @@
   $: if ($allModelColumns?.length) {
     for (const modelTable of $allModelColumns) {
       const modelIdentifier = modelTable?.tableName;
-      schema[modelIdentifier] = modelTable.profileColumns?.map((c) => c.name);
+      schema[modelIdentifier] = modelTable.profileColumns
+        ?.filter((c) => c.name !== undefined)
+        // CAST SAFETY: already filtered out undefined values
+        ?.map((c) => c.name as string);
     }
   }
 
   function getTableNameFromFromClause(
     sql: string,
     schema: { [table: string]: string[] }
-  ): string | null {
-    if (!sql || !schema) return null;
+  ): string | undefined {
+    if (!sql || !schema) return undefined;
 
     const fromMatch = sql.toUpperCase().match(/\bFROM\b\s+(\w+)/);
-    const tableName = fromMatch ? fromMatch[1] : null;
+    const tableName = fromMatch ? fromMatch[1] : undefined;
 
     // Get the tableName from the schema map, so we can use the correct case
     for (const schemaTableName of Object.keys(schema)) {
@@ -127,7 +132,7 @@
       }
     }
 
-    return null;
+    return undefined;
   }
 
   function makeAutocompleteConfig(

diff --git a/web-common/src/features/models/workspace/ModelBody.svelte b/web-common/src/features/models/workspace/ModelBody.svelte
@@ -51,7 +51,7 @@
 
   $: modelEmpty = useModelFileIsEmpty(runtimeInstanceId, modelName);
 
-  $: modelSql = $modelSqlQuery?.data?.blob;
+  $: modelSql = $modelSqlQuery?.data?.blob ?? "";
   $: hasModelSql = typeof modelSql === "string";
 
   $: modelQuery = useModel(runtimeInstanceId, modelName);
@@ -68,13 +68,13 @@
 
   $: tableQuery = createQueryServiceTableRows(
     runtimeInstanceId,
-    $modelQuery.data?.model?.state?.table,
+    $modelQuery.data?.model?.state?.table ?? "",
     {
       limit,
     }
   );
 
-  $: runtimeError = ($tableQuery.error as any)?.response.data;
+  $: runtimeError = $tableQuery.error?.response.data;
 
   const outputLayout = getContext(
     "rill:app:output-layout"
@@ -117,7 +117,7 @@
     to: selection?.referenceIndex + selection?.reference?.length,
   })) as SelectionRange[];
 
-  let errors = [];
+  let errors: string[] = [];
   $: {
     errors = [];
     // only add error if sql is present

diff --git a/web-common/src/features/models/workspace/ModelWorkspaceCTAs.svelte b/web-common/src/features/models/workspace/ModelWorkspaceCTAs.svelte
@@ -110,7 +110,9 @@
   <Tooltip distance={8} alignment="end">
     <Button
       on:click={() => {
-        goto(`/dashboard/${availableDashboards[0].meta.name.name}`);
+        if (availableDashboards[0]?.meta?.name?.name) {
+          goto(`/dashboard/${availableDashboards[0].meta.name.name}`);
+        }
       }}
     >
       <IconSpaceFixer pullLeft pullRight={collapse}>
@@ -145,11 +147,13 @@
         {#each availableDashboards as resource}
           <MenuItem
             on:select={() => {
-              goto(`/dashboard/${resource.meta.name.name}`);
-              toggleFloatingElement();
+              if (resource?.meta?.name?.name) {
+                goto(`/dashboard/${resource.meta.name.name}`);
+                toggleFloatingElement();
+              }
             }}
           >
-            {resource.meta.name.name}
+            {resource?.meta?.name?.name ?? "Loading..."}
           </MenuItem>
         {/each}
       </Menu>

diff --git a/web-common/src/features/models/workspace/ModelWorkspaceHeader.svelte b/web-common/src/features/models/workspace/ModelWorkspaceHeader.svelte
@@ -61,7 +61,9 @@
       e.target.value = modelName; // resets the input
       return;
     }
-    if (isDuplicateName(e.target.value, modelName, $allNamesQuery.data)) {
+    if (
+      isDuplicateName(e.target.value, modelName, $allNamesQuery?.data ?? [])
+    ) {
       notifications.send({
         message: `Name ${e.target.value} is already in use`,
       });
@@ -112,7 +114,7 @@
     {@const collapse = width < 800}
     <PanelCTA side="right">
       <ModelWorkspaceCTAs
-        availableDashboards={$availableDashboards?.data}
+        availableDashboards={$availableDashboards?.data ?? []}
         {collapse}
         modelHasError={$modelHasError}
         {modelName}

diff --git a/web-common/src/features/models/workspace/inspector/ModelInspectorHeader.svelte b/web-common/src/features/models/workspace/inspector/ModelInspectorHeader.svelte
@@ -14,7 +14,6 @@
   import {
     createQueryServiceTableCardinality,
     createQueryServiceTableColumns,
-    V1ModelV2,
     V1TableCardinalityResponse,
   } from "@rilldata/web-common/runtime-client";
   import { useQueryClient } from "@tanstack/svelte-query";
@@ -32,7 +31,6 @@
   const queryClient = useQueryClient();
 
   $: modelQuery = useModel($runtime.instanceId, modelName);
-  let model: V1ModelV2;
   $: model = $modelQuery?.data?.model;
 
   $: modelPath = getFilePathFromNameAndType(modelName, EntityType.Model);
@@ -71,17 +69,17 @@
     cardinalityQueries = referencedThings?.map(([resource]) => {
       return createQueryServiceTableCardinality(
         $runtime?.instanceId,
-        resource.meta.name.name,
+        resource.meta?.name?.name ?? "",
         {},
-        { query: { select: (data) => +data?.cardinality || 0 } }
+        { query: { select: (data) => +(data?.cardinality ?? 0) } }
       );
     });
 
     // then we'll get the total number of columns for comparison.
     sourceProfileColumns = referencedThings?.map(([resource]) => {
       return createQueryServiceTableColumns(
         $runtime?.instanceId,
-        resource.meta.name.name,
+        resource.meta?.name?.name ?? "",
         {},
         { query: { select: (data) => data?.profileColumns?.length || 0 } }
       );
@@ -92,7 +90,7 @@
 
   $: inputCardinalities = derived(cardinalityQueries, ($cardinalities) => {
     return $cardinalities
-      .map((c: { data: number }) => c?.data)
+      .map((c) => c?.data ?? 0)
       .reduce((total: number, cardinality: number) => total + cardinality, 0);
   });
 
@@ -101,7 +99,7 @@
     sourceProfileColumns,
     (columns) => {
       return columns
-        .map((col) => col.data)
+        .map((col) => col.data ?? 0)
         .reduce((total: number, columns: number) => columns + total, 0);
     },
     0

diff --git a/web-common/src/features/models/workspace/inspector/References.svelte b/web-common/src/features/models/workspace/inspector/References.svelte
@@ -27,15 +27,15 @@
   let showSourceTables = true;
   let modelHasError = false;
 
-  const queryHighlight: Writable<QueryHighlightState> = getContext(
+  const queryHighlight: Writable<QueryHighlightState | undefined> = getContext(
     "rill:app:query-highlight"
   );
 
   $: getModelFile = createRuntimeServiceGetFile(
     $runtime?.instanceId,
     getFilePathFromNameAndType(modelName, EntityType.Model)
   );
-  $: references = getTableReferences($getModelFile?.data.blob ?? "");
+  $: references = getTableReferences($getModelFile?.data?.blob ?? "");
 
   $: getAllSources = useSources($runtime?.instanceId);
 
@@ -56,13 +56,13 @@
           writable(ref),
           createQueryServiceTableCardinality(
             $runtime?.instanceId,
-            resource.meta.name.name
+            resource?.meta?.name?.name ?? ""
           ),
         ],
         ([resource, ref, cardinality]) => ({
           resource,
           reference: ref,
-          totalRows: +cardinality?.data?.cardinality,
+          totalRows: +(cardinality?.data?.cardinality ?? 0),
         })
       );
     }),