chore: cleanup types (elastic#1404)

- remove unnecessary `// @ts-ignore` comments
- add descriptions to legit `// @ts-ignore` comments
- add [rule](https://github.com/typescript-eslint/typescript-eslint/blob/v4.31.1/packages/eslint-plugin/docs/rules/ban-ts-comment.md) to error on all ts-comments with no description
- remove `global.d.ts` in favor of `declarations/`
- fix type issue related to `jest-extended` and custom matchers
- fix types around `sortSeriesBy` hidden setting
- fix type errors around required `domain` options
- fix type errors around union brush events
- remove deprecated [`msSaveBlob`](https://developer.mozilla.org/en-US/docs/Web/API/Navigator/msSaveBlob) usage
- add step to ci to check `yarn typecheck:all` as type errors somehow snuck into `master`

.ci/global_setup.sh

@@ -84,8 +84,6 @@ yarn config set yarn-offline-mirror "$cacheDir/yarn-offline-cache"
 yarnGlobalDir="$(yarn global bin)"
 export PATH="$PATH:$yarnGlobalDir"
 
-# avoid download puppeteer locally (we use the dockerized version)
-export PUPPETEER_SKIP_CHROMIUM_DOWNLOAD=true
 ###
 ### install dependencies
 ###

.eslintrc.js

@@ -178,7 +178,16 @@ module.exports = {
     '@typescript-eslint/ban-ts-ignore': 0,
     '@typescript-eslint/indent': 0,
     '@typescript-eslint/no-inferrable-types': 0,
-    '@typescript-eslint/ban-ts-comment': 1,
+    '@typescript-eslint/ban-ts-comment': [
+      2,
+      {
+        'ts-expect-error': 'allow-with-description',
+        'ts-ignore': 'allow-with-description',
+        'ts-nocheck': 'allow-with-description',
+        'ts-check': 'allow-with-description',
+        minimumDescriptionLength: 3,
+      },
+    ],
     '@typescript-eslint/no-unused-vars': [
       'error',
       {

.github/workflows/ci.yaml

@@ -50,8 +50,6 @@ jobs:
         uses: bahmutov/npm-install@v1
         with:
           useRollingCache: true
-        env:
-          PUPPETEER_SKIP_CHROMIUM_DOWNLOAD: true
 
   eslint:
     name: Eslint
@@ -71,8 +69,6 @@ jobs:
         uses: bahmutov/npm-install@v1
         with:
           useRollingCache: true
-        env:
-          PUPPETEER_SKIP_CHROMIUM_DOWNLOAD: true
       # -----------------------------------
 
       - name: Eslint check
@@ -96,8 +92,6 @@ jobs:
         uses: bahmutov/npm-install@v1
         with:
           useRollingCache: true
-        env:
-          PUPPETEER_SKIP_CHROMIUM_DOWNLOAD: true
       # -----------------------------------
 
       - name: Prettier check
@@ -121,10 +115,10 @@ jobs:
         uses: bahmutov/npm-install@v1
         with:
           useRollingCache: true
-        env:
-          PUPPETEER_SKIP_CHROMIUM_DOWNLOAD: true
       # -----------------------------------
 
+      - name: Run global typecheck
+        run: yarn typecheck:all
       - name: Run API-Extractor
         run: yarn api:check
       - name: Handle API-Extractor failure
@@ -154,8 +148,6 @@ jobs:
         uses: bahmutov/npm-install@v1
         with:
           useRollingCache: true
-        env:
-          PUPPETEER_SKIP_CHROMIUM_DOWNLOAD: true
       # -----------------------------------
 
       - name: TimeZone tests
@@ -182,8 +174,6 @@ jobs:
         uses: bahmutov/npm-install@v1
         with:
           useRollingCache: true
-        env:
-          PUPPETEER_SKIP_CHROMIUM_DOWNLOAD: true
       # -----------------------------------
 
       - name: Building storybook

.github/workflows/release.yaml

@@ -62,8 +62,6 @@ jobs:
         uses: bahmutov/npm-install@v1
         with:
           useRollingCache: true
-        env:
-          PUPPETEER_SKIP_CHROMIUM_DOWNLOAD: true
       - name: Lint check
         run: yarn lint
       - name: Prettier check
@@ -91,8 +89,6 @@ jobs:
         uses: bahmutov/npm-install@v1
         with:
           useRollingCache: true
-        env:
-          PUPPETEER_SKIP_CHROMIUM_DOWNLOAD: true
       - name: Build library
         run: yarn build
 

declarations/@types/jest.d.ts

@@ -0,0 +1,33 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
+ */
+
+export * from 'jest-extended'; // https://github.com/jest-community/jest-extended
+
+/**
+ * Final Matcher type with `this` and `received` args removed from jest matcher function
+ */
+type MatcherParameters<T extends (this: any, received: any, ...args: any[]) => any> = T extends (
+  this: any,
+  received: any,
+  ...args: infer P
+) => any
+  ? P
+  : never;
+
+declare global {
+  namespace jest {
+    interface Matchers<R> {
+      /**
+       * Expect array to be filled with value, and optionally length
+       */
+      toEqualArrayOf(...args: MatcherParameters<typeof toEqualArrayOf>): R;
+    }
+  }
+}
+
+export {}; // ensure this is parsed as a module.

global.d.ts → declarations/@types/node.d.ts

@@ -6,8 +6,7 @@
  * Side Public License, v 1.
  */
 
-import 'jest-extended'; // https://github.com/jest-community/jest-extended
-
+// Super flaky not sure why
 declare global {
   namespace NodeJS {
     interface ProcessEnv {
@@ -56,3 +55,5 @@ declare global {
     }
   }
 }
+
+export {}; // ensure this is parsed as a module.

integration/helpers.ts

@@ -15,7 +15,7 @@ import { getStorybook, configure } from '@storybook/react';
 
 import { Rotation } from '../packages/charts/src';
 import { ThemeId } from '../storybook/use_base_theme';
-// @ts-ignore
+// @ts-ignore - no type declarations
 import { isLegacyVRTServer } from './config';
 
 export type StoryInfo = [string, string, number];

integration/jest.config.js

@@ -13,7 +13,7 @@ const tsPreset = require('ts-jest/jest-preset');
 const { debug } = require('./config');
 
 module.exports = {
-  setupFilesAfterEnv: ['<rootDir>/jest_env_setup.ts'],
+  setupFilesAfterEnv: ['<rootDir>/jest_env_setup.ts', 'jest-extended'],
   globals: {
     'ts-jest': {
       tsconfig: '<rootDir>/tsconfig.json',

integration/page_objects/common.ts

@@ -11,14 +11,13 @@ import Url from 'url';
 import { AXNode } from 'puppeteer';
 
 import { DRAG_DETECTION_TIMEOUT } from '../../packages/charts/src/state/reducers/interactions';
-// @ts-ignore
+// @ts-ignore - no type declarations
 import { port, hostname, debug, isLegacyVRTServer } from '../config';
 import { toMatchImageSnapshot } from '../jest_env_setup';
 
 const legacyBaseUrl = `http://${hostname}:${port}/iframe.html`;
 
-// Use to log console statements from within the page.evaluate blocks
-// @ts-ignore
+// @ts-ignore - used to log console statements from within the page.evaluate blocks
 // page.on('console', (msg) => (msg._type === 'log' ? console.log('PAGE LOG:', msg._text) : null)); // eslint-disable-line no-console
 
 expect.extend({ toMatchImageSnapshot });

integration/server/mocks/@storybook/addon-knobs/index.ts

@@ -45,14 +45,14 @@ export function text(name: string, dftValue: string, groupId?: string) {
 
 export function array(name: string, dftValues: unknown[], options: any, groupId?: string) {
   const params = getParams();
-  const values = [];
-  // eslint-disable-next-line @typescript-eslint/ban-ts-comment
-  // @ts-ignore
-  for (const [key, value] of params) {
+  const values: string[] = [];
+
+  params.forEach((value, key) => {
     if (key.startsWith(`${getKnobKey(name, groupId)}[`)) {
       values.push(value);
     }
-  }
+  });
+
   if (values.length === 0) {
     return dftValues;
   }

integration/tsconfig.json

@@ -1,4 +1,4 @@
 {
   "extends": "../tsconfig",
-  "include": ["./**/*", "../**/*.d.ts", "../scripts/custom_matchers.ts"]
+  "include": ["./**/*", "../scripts/custom_matchers.mock.ts"]
 }

jest.config.js

@@ -10,7 +10,11 @@ module.exports = {
   testMatch: ['**/?(*.)+(test).[jt]s?(x)'],
   roots: ['<rootDir>/packages/charts/src'],
   preset: 'ts-jest',
-  setupFilesAfterEnv: ['<rootDir>/scripts/setup_enzyme.ts', '<rootDir>/scripts/custom_matchers.ts'],
+  setupFilesAfterEnv: [
+    '<rootDir>/scripts/setup_enzyme.ts',
+    '<rootDir>/scripts/custom_matchers.mock.ts',
+    'jest-extended',
+  ],
   coveragePathIgnorePatterns: [
     '<rootDir>/packages/charts/src/mocks',
     '<rootDir>/packages/charts/src/utils/d3-delaunay',

packages/charts/src/chart_types/wordcloud/renderer/svg/connected_component.tsx

@@ -6,7 +6,7 @@
  * Side Public License, v 1.
  */
 
-// @ts-ignore
+// @ts-ignore - remove in workcloud refactor
 import d3TagCloud from 'd3-cloud';
 import React from 'react';
 import { connect } from 'react-redux';

packages/charts/src/chart_types/xy_chart/state/chart_state.interactions.test.tsx

@@ -6,10 +6,8 @@
  * Side Public License, v 1.
  */
 
-// eslint-disable-next-line eslint-comments/disable-enable-pair
 /* eslint-disable jest/no-conditional-expect */
 
-import 'jest-extended';
 import React from 'react';
 import { Store } from 'redux';
 
@@ -1356,3 +1354,5 @@ describe('Clickable annotations', () => {
     expect(onAnnotationClick).toBeCalled();
   });
 });
+
+/* eslint-enable jest/no-conditional-expect */

packages/charts/src/chart_types/xy_chart/state/selectors/compute_legend.ts

@@ -52,7 +52,7 @@ export const computeLegendSelector = createCustomCachedSelector(
       siDataSeriesMap,
       deselectedDataSeries,
       chartTheme,
-      // @ts-ignore
+      // @ts-ignore - hidden method for vislib usage only
       settings.sortSeriesBy,
     );
   },

packages/charts/src/chart_types/xy_chart/state/selectors/compute_series_domains.ts

@@ -32,7 +32,7 @@ export const computeSeriesDomainsSelector = createCustomCachedSelector(
       deselectedDataSeries,
       settingsSpec.orderOrdinalBinsBy,
       smallMultiples,
-      // @ts-ignore
+      // @ts-ignore - hidden method for vislib usage only
       settingsSpec.sortSeriesBy,
     );
   },

packages/charts/src/chart_types/xy_chart/utils/axis_utils.test.ts

@@ -7,10 +7,8 @@
  */
 
 import { DateTime } from 'luxon';
-// @ts-ignore
 import moment from 'moment-timezone';
 
-import 'jest-extended';
 import { ChartType } from '../..';
 import { MockGlobalSpec, MockSeriesSpec } from '../../../mocks/specs/specs';
 import { MockStore } from '../../../mocks/store/store';

packages/charts/src/chart_types/xy_chart/utils/fit_function.test.ts

@@ -504,13 +504,13 @@ describe('Fit Function', () => {
     describe('sorting', () => {
       const spies: jest.SpyInstance[] = [];
       const mockArray: any[] = [];
-      // @ts-ignore
+      // @ts-ignore - mocking array prototype method
       jest.spyOn(mockArray, 'sort');
 
       beforeAll(() => {
-        // @ts-ignore
+        // @ts-ignore - mocking array prototype method
         spies.push(jest.spyOn(dataSeries.data, 'sort'));
-        // @ts-ignore
+        // @ts-ignore - mocking array prototype method
         spies.push(jest.spyOn(dataSeries.data, 'slice').mockReturnValue(mockArray));
       });
 

packages/charts/src/components/portal/tooltip_portal.tsx

@@ -95,7 +95,7 @@ const TooltipPortalComponent = ({
    */
   const popper = useRef<Instance | null>(null);
   const popperSettings = useMemo(
-    // @ts-ignore
+    // @ts-ignore - nesting limitation
     () => mergePartial(DEFAULT_POPPER_SETTINGS, settings, { mergeOptionalPartialValues: true }),
     [settings],
   );

packages/charts/src/mocks/specs/specs.ts

@@ -343,7 +343,7 @@ export class MockGlobalSpec {
   };
 
   static settings(partial?: Partial<SettingsSpec>): SettingsSpec {
-    // @ts-ignore
+    // @ts-ignore - nesting limitation
     return mergePartial<SettingsSpec>(MockGlobalSpec.settingsBase, partial, { mergeOptionalPartialValues: true });
   }
 

packages/charts/src/scales/scale_continuous.test.ts

@@ -498,7 +498,7 @@ describe('Scale Continuous', () => {
     });
 
     it('should throw for NaN values', () => {
-      // @ts-ignore
+      // @ts-ignore - d3Scale method
       jest.spyOn(scale, 'd3Scale').mockImplementationOnce(() => NaN);
       expect(() => scale.scaleOrThrow(1)).toThrow();
     });

packages/charts/src/utils/common.test.ts

@@ -77,14 +77,14 @@ describe('common utilities', () => {
     });
 
     it('should return second partial if partial is undefined', () => {
-      // @ts-ignore
+      // @ts-ignore - nesting limitation
       const result = getPartialValue(base, undefined, [undefined, partial]);
 
       expect(result).toBe(partial);
     });
 
     it('should return base if no partials are defined', () => {
-      // @ts-ignore
+      // @ts-ignore - nesting limitation
       const result = getPartialValue(base, undefined, [undefined, undefined]);
 
       expect(result).toBe(base);

packages/charts/src/utils/fast_deep_equal.ts

@@ -51,16 +51,16 @@ export function deepEqual(a: any, b: any, partial = false): boolean {
     }
     if (a instanceof Map && b instanceof Map) {
       if (a.size !== b.size) return false;
-      // @ts-ignore
+      // @ts-ignore - 3rd party code
       for (i of a.entries()) if (!b.has(i[0])) return false;
-      // @ts-ignore
+      // @ts-ignore - 3rd party code
       for (i of a.entries()) if (!deepEqual(i[1], b.get(i[0]))) return false;
       return true;
     }
 
     if (a instanceof Set && b instanceof Set) {
       if (a.size !== b.size) return false;
-      // @ts-ignore
+      // @ts-ignore - 3rd party code
       for (i of a.entries()) if (!b.has(i[0])) return false;
       return true;
     }