Add tsc check to CI (rilldata#3785)

* add tsc check to CI

* dummy change to trigger CI

* function `forwardEvents` is never used, string search shows it's only in code commented out 18 months ago

* `TimeGrain` needs the d3format field

* fix missing `describe, it, expect`

* remove return type from function that does nothing

* type guards in pathIsDefined

* regex exec may return null. if so, default to `indent` being empty string

* add missing import of SvelteComponent

* fix variables used before being assigned

* remove assignment to field that is not actually on MetricsExplorerEntity

* remove assigment of "name" field that doesn't exist on V1MetricsViewSpec

* add missing imports describe, it, expect

* add missing expected field "view" to placeholder props

* make sure callback fn exists before calling it

* include "undefined" as possible field types for AppStore, since it's explicitly initialized that way farther down the file

* add types for callback fns to indicate that they are indeed callable

* cleanup subtractFromPeriod types

* make sure resolve / reject exist before calling them

* clean up computeSegments

* add Expand<T> helper type

* update temporary whitelist of allowed failing TS codes

* one more type check exemption

* move tsc check to bash script

.github/workflows/web-test-code-quality.yml

@@ -64,3 +64,6 @@ jobs:
           npx eslint web-auth --quiet
           npx svelte-check --workspace web-auth --no-tsconfig
 
+      - name: type check non-svelte files (with temporary whitelist)
+        run: |-
+          sh ./scripts/tsc-with-whitelist.sh

scripts/tsc-with-whitelist.sh

@@ -0,0 +1,14 @@
+#!/usr/bin/env bash
+
+# Run TypeScript compiler and filter out some errors
+output=$(npx tsc --noEmit | grep "error TS" | grep -v -E 'TS18048|TS2345|TS2322|TS18047|TS2532|TS2339|TS2538|TS2769|TS18046|TS2614')
+
+# Check if 'error' is in the output
+if echo "$output" | grep -q "error"; then
+    echo "TypeScript errors found:"
+    echo "$output"
+    exit 1  # Exit with error code
+else
+    echo "No TypeScript errors detected."
+    exit 0  # Exit without error
+fi

web-common/src/components/data-graphic/elements/SimpleDataGraphic.svelte

@@ -39,7 +39,6 @@ A simple composable container for SVG-based data graphics.
   /** this makes a wide variety of normal events, such as on:click, available
    * to the consumer
    */
-  // const forwardAll = forwardEvents(getComponent(), []);
 </script>
 
 <GraphicContext

web-common/src/components/data-graphic/marks/ChunkedLine.svelte

@@ -21,7 +21,6 @@ Over time, we'll make this the default Line implementation, but it's not quite t
   import {
     areaFactory,
     lineFactory,
-    pathIsDefined,
   } from "@rilldata/web-common/components/data-graphic/utils";
   import {
     AreaSubRangeColor,
@@ -86,7 +85,7 @@ Over time, we'll make this the default Line implementation, but it's not quite t
     }
   }
 
-  $: segments = computeSegments(data, pathIsDefined(yAccessor));
+  $: segments = computeSegments(data, yAccessor);
   /** plot these as points */
   $: singletons = segments.filter((segment) => segment.length === 1);
 

web-common/src/components/data-graphic/marks/segment.ts

@@ -1,39 +1,50 @@
+function pathIsDefined(yAccessor: string) {
+  return (d: Record<string, number>) => {
+    const val = d[yAccessor];
+    return !(
+      val === undefined ||
+      (typeof val === "number" && isNaN(val)) ||
+      val === null
+    );
+  };
+}
+
 /**
  * Helper function to compute the contiguous segments of the data
  * based on https://github.com/pbeshai/d3-line-chunked/blob/master/src/lineChunked.js
  */
-export function computeSegments(lineData, defined, isNext = () => true) {
+export function computeSegments(
+  lineData: Record<string, number>[],
+  yAccessor: string,
+): Record<string, number>[][] {
   let startNewSegment = true;
 
+  const defined = pathIsDefined(yAccessor);
   // split into segments of continuous data
-  const segments = lineData.reduce(function (segments, d) {
-    // skip if this point has no data
-    if (!defined(d)) {
-      startNewSegment = true;
-      return segments;
-    }
-
-    // if we are starting a new segment, start it with this point
-    if (startNewSegment) {
-      segments.push([d]);
-      startNewSegment = false;
-
-      // otherwise see if we are adding to the last segment
-    } else {
-      const lastSegment = segments[segments.length - 1];
-      const lastDatum = lastSegment[lastSegment.length - 1];
-      // if we expect this point to come next, add it to the segment
-      if (isNext(lastDatum, d)) {
-        lastSegment.push(d);
+  const segments = lineData.reduce(
+    (segments: Record<string, number>[][], d) => {
+      // skip if this point has no data
+      if (!defined(d)) {
+        startNewSegment = true;
+        return segments;
+      }
 
-        // otherwise create a new segment
-      } else {
+      // if we are starting a new segment, start it with this point
+      if (startNewSegment) {
         segments.push([d]);
+        startNewSegment = false;
+
+        // otherwise see if we are adding to the last segment
+      } else {
+        const lastSegment = segments[segments.length - 1];
+        lastSegment.push(d);
+        // if we expect this point to come next, add it to the segment
       }
-    }
 
-    return segments;
-  }, []);
+      return segments;
+    },
+    [],
+  );
 
   return segments;
 }

web-common/src/components/data-graphic/utils.ts

@@ -32,16 +32,6 @@ const curves = {
   curveStepExtended,
 };
 
-export function pathIsDefined(yAccessor: string) {
-  return (d) => {
-    return !(
-      d[yAccessor] === undefined ||
-      isNaN(d[yAccessor]) ||
-      d[yAccessor] === null
-    );
-  };
-}
-
 export function pathDoesNotDropToZero(yAccessor: string) {
   return (d, i: number, arr) => {
     return (

web-common/src/components/date-picker/datetime.ts

@@ -517,7 +517,11 @@ export class DateTime {
     return this;
   }
 
-  public diff(date: DateTime, unit = "seconds"): number {
+  // FIXME: I'm pretty sure this is never used anywhere. Even if it is,
+  // it doesn't do anything, because it doesn't return anything,
+  // and it doesn't mutate anything. Also it has a dangling TODO
+  // that was unfinished when Speros left.
+  public diff(date: DateTime, unit = "seconds") {
     const oneDay = 1000 * 60 * 60 * 24;
 
     switch (unit) {

web-common/src/components/editor/indent-guide/index.ts

@@ -40,7 +40,8 @@ export const indentGuide = () =>
       /** Creates a Monaco-like indent */
       decorationsFromLine(lineNumber) {
         const line = this.view.state.doc.line(lineNumber);
-        const indent = /^\s*/.exec(line.text)[0];
+        // empty string if no whitespace at start of line
+        const indent = /^\s*/.exec(line.text)?.[0] ?? "";
         const indentSize = indent.length;
         const decorations = [];
 

web-common/src/components/editor/line-status/line-number-gutter.ts

@@ -1,6 +1,7 @@
 import { GutterMarker, gutter } from "@codemirror/view";
 import LineNumberGutterMarkerComponent from "./LineNumberGutterMarker.svelte";
 import { lineStatusesStateField, updateLineStatuses } from "./state";
+import type { SvelteComponent } from "svelte";
 
 export const LINE_NUMBER_GUTTER_CLASS = "cm-line-number-gutter";
 

web-common/src/components/notifications/notificationStore.ts

@@ -39,7 +39,7 @@ interface NotificationOptions {
 
 function createNotificationStore(): NotificationStore {
   const _notification = writable({} as NotificationMessage);
-  let timeout: ReturnType<typeof setTimeout>;
+  let timeout: ReturnType<typeof setTimeout> | undefined = undefined;
 
   function send({
     message,

web-common/src/components/searchable-filter-menu/SearchableFilterDropdown.svelte

@@ -39,6 +39,7 @@
     (sel, g) => sel + g.items.length,
     0,
   );
+
   $: searchResultCount = menuGroups.reduce(
     (sel, mg) => sel + mg.items.length,
     0,

web-common/src/features/dashboards/show-hide-selectors.spec.ts

@@ -125,11 +125,7 @@ describe("Show/Hide Selectors", () => {
         },
       ]);
       // we have to manually call sync since in the app it is handled by a reactive statement
-      metricsExplorerStore.sync(
-        AD_BIDS_NAME,
-        AD_BIDS_WITH_DELETED_MEASURE,
-        undefined,
-      );
+      metricsExplorerStore.sync(AD_BIDS_NAME, AD_BIDS_WITH_DELETED_MEASURE);
       expect(get(showHideMeasure).availableKeys).toEqual([
         AD_BIDS_IMPRESSIONS_MEASURE,
       ]);
@@ -153,11 +149,7 @@ describe("Show/Hide Selectors", () => {
 
       mock.setMeasures(AD_BIDS_THREE_MEASURES);
       // we have to manually call sync since in the app it is handled by a reactive statement
-      metricsExplorerStore.sync(
-        AD_BIDS_NAME,
-        AD_BIDS_WITH_THREE_MEASURES,
-        undefined,
-      );
+      metricsExplorerStore.sync(AD_BIDS_NAME, AD_BIDS_WITH_THREE_MEASURES);
       expect(get(showHideMeasure).availableKeys).toEqual([
         AD_BIDS_IMPRESSIONS_MEASURE,
         AD_BIDS_BID_PRICE_MEASURE,
@@ -180,11 +172,7 @@ describe("Show/Hide Selectors", () => {
 
       mock.setMeasures(AD_BIDS_THREE_MEASURES);
       // we have to manually call sync since in the app it is handled by a reactive statement
-      metricsExplorerStore.sync(
-        AD_BIDS_NAME,
-        AD_BIDS_WITH_THREE_MEASURES,
-        undefined,
-      );
+      metricsExplorerStore.sync(AD_BIDS_NAME, AD_BIDS_WITH_THREE_MEASURES);
       expect(get(showHideMeasure).availableKeys).toEqual([
         AD_BIDS_IMPRESSIONS_MEASURE,
         AD_BIDS_BID_PRICE_MEASURE,
@@ -291,11 +279,7 @@ describe("Show/Hide Selectors", () => {
         },
       ]);
       // we have to manually call sync since in the app it is handled by a reactive statement
-      metricsExplorerStore.sync(
-        AD_BIDS_NAME,
-        AD_BIDS_WITH_DELETED_DIMENSION,
-        undefined,
-      );
+      metricsExplorerStore.sync(AD_BIDS_NAME, AD_BIDS_WITH_DELETED_DIMENSION);
       expect(get(showHideDimensions).availableKeys).toEqual([
         AD_BIDS_PUBLISHER_DIMENSION,
       ]);
@@ -320,11 +304,7 @@ describe("Show/Hide Selectors", () => {
 
       mock.setDimensions(AD_BIDS_THREE_DIMENSIONS);
       // we have to manually call sync since in the app it is handled by a reactive statement
-      metricsExplorerStore.sync(
-        AD_BIDS_NAME,
-        AD_BIDS_WITH_THREE_DIMENSIONS,
-        undefined,
-      );
+      metricsExplorerStore.sync(AD_BIDS_NAME, AD_BIDS_WITH_THREE_DIMENSIONS);
       expect(get(showHideDimensions).availableKeys).toEqual([
         AD_BIDS_PUBLISHER_DIMENSION,
         AD_BIDS_DOMAIN_DIMENSION,
@@ -352,11 +332,7 @@ describe("Show/Hide Selectors", () => {
 
       mock.setDimensions(AD_BIDS_THREE_DIMENSIONS);
       // we have to manually call sync since in the app it is handled by a reactive statement
-      metricsExplorerStore.sync(
-        AD_BIDS_NAME,
-        AD_BIDS_WITH_THREE_DIMENSIONS,
-        undefined,
-      );
+      metricsExplorerStore.sync(AD_BIDS_NAME, AD_BIDS_WITH_THREE_DIMENSIONS);
       expect(get(showHideDimensions).availableKeys).toEqual([
         AD_BIDS_PUBLISHER_DIMENSION,
         AD_BIDS_DOMAIN_DIMENSION,

web-common/src/features/dashboards/state-managers/actions/types.ts

@@ -1,4 +1,5 @@
 import type { MetricsExplorerEntity } from "../../stores/metrics-explorer-entity";
+import type { Expand } from "../types";
 
 // Note: the types below are helper types to simplify the type inference
 // used in the creation of StateManagerActions, so that we can have nice

web-common/src/features/dashboards/state-managers/selectors/types.ts

@@ -6,6 +6,7 @@ import type {
   V1ColumnTimeRangeResponse,
   V1MetricsViewSpec,
 } from "@rilldata/web-common/runtime-client";
+import type { Expand } from "../types";
 
 /**
  * DashboardDataSources collects all the information about a dashboard

web-common/src/features/dashboards/state-managers/types.ts

@@ -6,3 +6,14 @@
  * the other keys will be optional
  */
 export type AtLeast<T, K extends keyof T> = Partial<T> & Pick<T, K>;
+
+/**
+ * Helper type based on the internal `Expand` type from the TypeScript.
+ *
+ * If types on actions and selectors ever look nasty,
+ * it's probably because we're missing an `Expand` somewhere.
+ *
+ *
+ * see https://stackoverflow.com/questions/57683303/how-can-i-see-the-full-expanded-contract-of-a-typescript-type
+ */
+export type Expand<T> = T extends infer O ? { [K in keyof O]: O[K] } : never;

web-common/src/features/dashboards/stores/dashboard-store-defaults.ts

@@ -124,7 +124,6 @@ export function getDefaultMetricsExplorerEntity(
 
   const metricsExplorer: MetricsExplorerEntity = {
     name,
-    selectedMeasureNames: [...defaultMeasureNames],
     visibleMeasureKeys: new Set(defaultMeasureNames),
     allMeasuresVisible: true,
     visibleDimensionKeys: new Set(defaultDimNames),

web-common/src/features/dashboards/stores/dashboard-stores-test-data.ts

@@ -174,7 +174,6 @@ export function initAdBidsMirrorInStore() {
   metricsExplorerStore.init(
     AD_BIDS_MIRROR_NAME,
     {
-      name: AD_BIDS_MIRROR_NAME,
       measures: AD_BIDS_INIT_MEASURES,
       dimensions: AD_BIDS_INIT_DIMENSIONS,
     },