refactor: scale improvements and TS 4.4 (#1383)

* switch to TS 4.4 and bump other deps * type strength increase via removing many `any` occurrences * domain oriented generic typing for scales * replacement and future prevention of coercive `==` and `!=` ops * refactoring: simplification and shortening of numerous functions, about 300 runtime lines gone * removal of numerous let, if/then, optional parameters etc. BREAKING CHANGE: The public type varieties for domains are discontinued, in favor of retaining the single `DomainRange` export, which now has a mandatory `{min: number, max: number}`. The developer can supply `NaN` where a finite min, max or both aren't defined (ie. in place of former effective `undefined`). In addition, some console.warn punctuations changed Co-authored-by: Marco Vettorello <[email protected]> Co-authored-by: Nick Partridge <[email protected]>
elastic · Sep 22, 2021 · 0003bc1 · 0003bc1
1 parent b6645d3
commit 0003bc1
Show file tree

Hide file tree

Showing 67 changed files with 625 additions and 892 deletions.
diff --git a/.eslintrc.js b/.eslintrc.js
@@ -90,11 +90,12 @@ module.exports = {
     'jsx-a11y/mouse-events-have-key-events': 1,
     'jsx-a11y/click-events-have-key-events': 1,
     '@typescript-eslint/member-ordering': 1,
-    eqeqeq: 1,
+    eqeqeq: 2,
 
     /*
      * Standard rules
      */
+    '@typescript-eslint/object-curly-spacing': 0,
     'no-restricted-syntax': 0, // this is a good rule, for-of is good
     'no-console': process.env.NODE_ENV === 'production' ? 2 : 1,
     'no-debugger': process.env.NODE_ENV === 'production' ? 2 : 1,
@@ -112,7 +113,7 @@ module.exports = {
     'no-continue': 0,
     'no-lonely-if': 0,
     'no-return-assign': 0,
-    'no-underscore-dangle': 0,
+    'no-underscore-dangle': ['error', { allow: ['__REDUX_DEVTOOLS_EXTENSION_COMPOSE__'] }],
     'no-confusing-arrow': 0,
     'prefer-destructuring': 0,
     'function-paren-newline': 0,

diff --git a/integration/page_objects/common.ts b/integration/page_objects/common.ts
@@ -321,27 +321,23 @@ class CommonPage {
     selector: string = 'body',
     options?: ScreenshotElementAtUrlOptions,
   ) {
-    try {
-      await this.loadElementFromURL(url, options?.waitSelector ?? selector, options?.timeout);
+    await this.loadElementFromURL(url, options?.waitSelector ?? selector, options?.timeout);
 
-      if (options?.action) {
-        await options.action();
-      }
-
-      if (options?.delay) {
-        await page.waitFor(options.delay);
-      }
+    if (options?.action) {
+      await options.action();
+    }
 
-      const element = await this.screenshotDOMElement(options?.screenshotSelector ?? selector, options);
+    if (options?.delay) {
+      await page.waitFor(options.delay);
+    }
 
-      if (!element) {
-        throw new Error(`Error: Unable to find element\n\n\t${url}`);
-      }
+    const element = await this.screenshotDOMElement(options?.screenshotSelector ?? selector, options);
 
-      expect(element).toMatchImageSnapshot();
-    } catch (error) {
-      throw new Error(error);
+    if (!element) {
+      throw new Error(`Error: Unable to find element\n\n\t${url}`);
     }
+
+    expect(element).toMatchImageSnapshot();
   }
 
   /**

diff --git a/integration/server/mocks/@storybook/addon-knobs/index.ts b/integration/server/mocks/@storybook/addon-knobs/index.ts
@@ -15,13 +15,8 @@ function getKnobKey(name: string, groupId?: string) {
 }
 
 export function boolean(name: string, dftValue: boolean, groupId?: string) {
-  const params = getParams();
-  const key = getKnobKey(name, groupId);
-  const param = params.get(key);
-  if (param === '' || param == null) {
-    return dftValue;
-  }
-  return params.get(key) === 'true';
+  const param = getParams().get(getKnobKey(name, groupId));
+  return param ? param === 'true' : dftValue;
 }
 
 export function number(name: string, dftValue: number, options?: any, groupId?: string) {
@@ -43,19 +38,15 @@ export function select(name: string, b: unknown, dftValue: string, groupId?: str
 }
 
 export function text(name: string, dftValue: string, groupId?: string) {
-  const params = getParams();
-  const key = getKnobKey(name, groupId);
-  const value = params.get(key);
-  if (value != null) {
-    // the # used for the color knob needs to be escaped on the URL and unescaped here
-    return unescape(value);
-  }
-  return dftValue;
+  const value = getParams().get(getKnobKey(name, groupId));
+  // the # used for the color knob needs to be escaped on the URL and unescaped here
+  return value === null ? dftValue : unescape(value);
 }
 
 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) {
     if (key.startsWith(`${getKnobKey(name, groupId)}[`)) {

diff --git a/package.json b/package.json
@@ -68,7 +68,7 @@
     "@elastic/eui": "^38.0.0",
     "@mdx-js/loader": "^1.6.6",
     "@microsoft/api-documenter": "^7.12.7",
-    "@microsoft/api-extractor": "^7.13.1",
+    "@microsoft/api-extractor": "^7.18.9",
     "@semantic-release/changelog": "^5.0.1",
     "@semantic-release/commit-analyzer": "^8.0.1",
     "@semantic-release/exec": "^5.0.0",
@@ -102,8 +102,8 @@
     "@types/seedrandom": "^2.4.28",
     "@types/url-parse": "^1.4.3",
     "@types/uuid": "^3.4.4",
-    "@typescript-eslint/eslint-plugin": "^4.12.0",
-    "@typescript-eslint/parser": "^4.12.0",
+    "@typescript-eslint/eslint-plugin": "^4.31.1",
+    "@typescript-eslint/parser": "^4.31.1",
     "autoprefixer": "^9.0.0",
     "backport": "^5.6.6",
     "canvas": "^2.8.0",
@@ -113,7 +113,8 @@
     "enzyme": "^3.11.0",
     "enzyme-adapter-react-16": "^1.15.5",
     "eslint": "^7.17.0",
-    "eslint-config-airbnb-typescript": "^12.0.0",
+    "eslint-config-airbnb": "^18.2.1",
+    "eslint-config-airbnb-typescript": "^14.0.0",
     "eslint-config-prettier": "^7.1.0",
     "eslint-plugin-elastic-charts": "link:./packages/eslint-plugin-elastic-charts",
     "eslint-plugin-eslint-comments": "^3.2.0",
@@ -167,7 +168,7 @@
     "speed-measure-webpack-plugin": "^1.5.0",
     "ts-jest": "^26.5.5",
     "ts-prune": "^0.8.4",
-    "typescript": "^4.1.3",
+    "typescript": "^4.4.3",
     "webpack": "^4.46.0",
     "webpack-cli": "^4.8.0",
     "webpack-dev-server": "^4.1.0"

diff --git a/packages/charts/api/charts.api.md b/packages/charts/api/charts.api.md
@@ -410,7 +410,7 @@ export class Chart extends React_2.Component<ChartProps, ChartState> {
     } | null;
     // (undocumented)
     render(): JSX.Element;
-    }
+}
 
 // @public (undocumented)
 export type ChartSize = number | string | ChartSizeArray | ChartSizeObject;
@@ -473,13 +473,6 @@ export const ColorVariant: Readonly<{
 // @public (undocumented)
 export type ColorVariant = $Values<typeof ColorVariant>;
 
-// Warning: (ae-forgotten-export) The symbol "DomainBase" needs to be exported by the entry point index.d.ts
-// Warning: (ae-forgotten-export) The symbol "LowerBound" needs to be exported by the entry point index.d.ts
-// Warning: (ae-forgotten-export) The symbol "UpperBound" needs to be exported by the entry point index.d.ts
-//
-// @public (undocumented)
-export type CompleteBoundedDomain = DomainBase & LowerBound & UpperBound;
-
 // @public
 export type ComponentWithAnnotationDatum = ComponentType<LineAnnotationDatum>;
 
@@ -564,7 +557,7 @@ export class DataGenerator {
         y: number;
         g: string;
     }[];
-    }
+}
 
 // @public (undocumented)
 export type DataName = CategoryKey;
@@ -706,7 +699,13 @@ export const DomainPaddingUnit: Readonly<{
 export type DomainPaddingUnit = $Values<typeof DomainPaddingUnit>;
 
 // @public (undocumented)
-export type DomainRange = LowerBoundedDomain | UpperBoundedDomain | CompleteBoundedDomain | UnboundedDomainWithInterval;
+export interface DomainRange {
+    // (undocumented)
+    max: number;
+    min: number;
+    // (undocumented)
+    minInterval?: number;
+}
 
 // @public (undocumented)
 export type ElementClickListener = (elements: Array<XYChartElementEvent | PartitionElementEvent | HeatmapElementEvent | WordCloudElementEvent>) => void;
@@ -1109,9 +1108,9 @@ export type HistogramModeAlignment = 'start' | 'center' | 'end';
 
 // @public (undocumented)
 export const HistogramModeAlignments: Readonly<{
-    Start: LineAlignSetting;
-    Center: LineAlignSetting;
-    End: LineAlignSetting;
+    Start: HistogramModeAlignment;
+    Center: HistogramModeAlignment;
+    End: HistogramModeAlignment;
 }>;
 
 // @public (undocumented)
@@ -1359,9 +1358,6 @@ export interface LogScaleOptions {
     logMinLimit?: number;
 }
 
-// @public (undocumented)
-export type LowerBoundedDomain = DomainBase & LowerBound;
-
 // @public
 export type MarkBuffer = number | ((radius: number) => number);
 
@@ -1534,7 +1530,6 @@ export type Placement = $Values<typeof Placement>;
 
 // @public (undocumented)
 type PointerEvent_2 = PointerOverEvent | PointerOutEvent;
-
 export { PointerEvent_2 as PointerEvent }
 
 // @public (undocumented)
@@ -2256,12 +2251,6 @@ export interface UnaryAccessorFn<Return = any> {
     fieldName?: string;
 }
 
-// @public (undocumented)
-export type UnboundedDomainWithInterval = DomainBase;
-
-// @public (undocumented)
-export type UpperBoundedDomain = DomainBase & UpperBound;
-
 // @public (undocumented)
 export type ValueAccessor = (d: Datum) => AdditiveNumber;
 
@@ -2433,7 +2422,6 @@ export interface YDomainBase {
 // @public (undocumented)
 export type YDomainRange = YDomainBase & DomainRange & LogScaleOptions;
 
-
 // Warnings were encountered during analysis:
 //
 // src/chart_types/heatmap/layout/types/config_types.ts:19:13 - (ae-forgotten-export) The symbol "SizeRatio" needs to be exported by the entry point index.d.ts

diff --git a/packages/charts/src/chart_types/heatmap/layout/viewmodel/viewmodel.ts b/packages/charts/src/chart_types/heatmap/layout/viewmodel/viewmodel.ts
@@ -107,7 +107,7 @@ export function shapeViewModel(
       ? new ScaleContinuous(
           {
             type: ScaleType.Time,
-            domain: xDomain.domain,
+            domain: xDomain.domain as number[],
             range: [0, chartDimensions.width],
             nice: false,
           },

diff --git a/packages/charts/src/chart_types/heatmap/state/selectors/get_legend_items_labels.ts b/packages/charts/src/chart_types/heatmap/state/selectors/get_legend_items_labels.ts
@@ -15,10 +15,8 @@ import { computeLegendSelector } from './compute_legend';
 export const getLegendItemsLabelsSelector = createCustomCachedSelector(
   [computeLegendSelector, getSettingsSpecSelector],
   (legendItems, { showLegendExtra }): LegendItemLabel[] =>
-    legendItems.map(({ label, defaultExtra }) => {
-      if (defaultExtra?.formatted != null) {
-        return { label: `${label}${showLegendExtra ? defaultExtra.formatted : ''}`, depth: 0 };
-      }
-      return { label, depth: 0 };
-    }),
+    legendItems.map(({ label, defaultExtra }) => ({
+      label: `${label}${showLegendExtra ? defaultExtra?.formatted ?? '' : ''}`,
+      depth: 0,
+    })),
 );
diff --git a/packages/charts/src/chart_types/heatmap/state/selectors/get_x_axis_right_overflow.ts b/packages/charts/src/chart_types/heatmap/state/selectors/get_x_axis_right_overflow.ts
@@ -30,7 +30,7 @@ export const getXAxisRightOverflow = createCustomCachedSelector(
     const timeScale = new ScaleContinuous(
       {
         type: ScaleType.Time,
-        domain: xDomain.domain,
+        domain: xDomain.domain as number[],
         range: [0, 1],
       },
       {

diff --git a/packages/charts/src/chart_types/partition_chart/layout/utils/legend_labels.ts b/packages/charts/src/chart_types/partition_chart/layout/utils/legend_labels.ts
@@ -29,12 +29,9 @@ function flatSlicesNames(
     // format the key with the layer formatter
     const layer = layers[depth - 1];
     const formatter = layer?.nodeLabel;
-    let formattedValue = '';
-    if (key != null) {
-      formattedValue = formatter ? formatter(key) : `${key}`;
-    }
+    const formattedValue = formatter ? formatter(key) : `${key}`;
     // preventing errors from external formatters
-    if (formattedValue != null && formattedValue !== '' && formattedValue !== HIERARCHY_ROOT_KEY) {
+    if (formattedValue && formattedValue !== HIERARCHY_ROOT_KEY) {
       // save only the max depth, so we can compute the the max extension of the legend
       keys.set(formattedValue, Math.max(depth, keys.get(formattedValue) ?? 0));
     }

diff --git a/packages/charts/src/chart_types/partition_chart/layout/viewmodel/hierarchy_of_arrays.ts b/packages/charts/src/chart_types/partition_chart/layout/viewmodel/hierarchy_of_arrays.ts
@@ -115,18 +115,11 @@ export function getExtraValueMap(
     const branch = tree[i];
     const [key, arrayNode] = branch;
     const { value, path, [CHILDREN_KEY]: children } = arrayNode;
-
-    if (key != null) {
-      const values: LegendItemExtraValues = new Map();
-      const formattedValue = valueFormatter ? valueFormatter(value) : value;
-
-      values.set(key, formattedValue);
-      keys.set(path.map(({ index }) => index).join('__'), values);
-    }
-
-    if (depth < maxDepth) {
-      getExtraValueMap(layers, valueFormatter, children, maxDepth, depth + 1, keys);
-    }
+    const values: LegendItemExtraValues = new Map();
+    const formattedValue = valueFormatter ? valueFormatter(value) : value;
+    values.set(key, formattedValue);
+    keys.set(path.map(({ index }) => index).join('__'), values);
+    if (depth < maxDepth) getExtraValueMap(layers, valueFormatter, children, maxDepth, depth + 1, keys);
   }
   return keys;
 }
diff --git a/packages/charts/src/chart_types/partition_chart/layout/viewmodel/scenegraph.ts b/packages/charts/src/chart_types/partition_chart/layout/viewmodel/scenegraph.ts
@@ -8,7 +8,7 @@
 
 import { measureText } from '../../../../common/text_utils';
 import { SmallMultiplesStyle } from '../../../../specs';
-import { Color, identity, mergePartial, RecursivePartial } from '../../../../utils/common';
+import { Color, mergePartial, RecursivePartial } from '../../../../utils/common';
 import { Dimensions } from '../../../../utils/dimensions';
 import { Layer, PartitionSpec } from '../../specs';
 import { config as defaultConfig, VALUE_GETTERS } from '../config';
@@ -26,7 +26,7 @@ import { shapeViewModel } from './viewmodel';
 
 function rawTextGetter(layers: Layer[]): RawTextGetter {
   return (node: ShapeTreeNode) => {
-    const accessorFn = layers[node[DEPTH_KEY] - 1].nodeLabel || identity;
+    const accessorFn = layers[node[DEPTH_KEY] - 1].nodeLabel || ((d) => d);
     return `${accessorFn(node.dataName)}`;
   };
 }

diff --git a/packages/charts/src/chart_types/xy_chart/annotations/line/dimensions.ts b/packages/charts/src/chart_types/xy_chart/annotations/line/dimensions.ts
@@ -8,7 +8,7 @@
 
 import { Line } from '../../../../geoms/types';
 import { Scale } from '../../../../scales';
-import { isContinuousScale, isBandScale } from '../../../../scales/types';
+import { isBandScale, isContinuousScale } from '../../../../scales/types';
 import { isNil, Position, Rotation } from '../../../../utils/common';
 import { Dimensions, Size } from '../../../../utils/dimensions';
 import { GroupId } from '../../../../utils/ids';
@@ -17,7 +17,7 @@ import { SmallMultipleScales } from '../../state/selectors/compute_small_multipl
 import { isHorizontalRotation, isVerticalRotation } from '../../state/utils/common';
 import { computeXScaleOffset } from '../../state/utils/utils';
 import { getPanelSize } from '../../utils/panel';
-import { AnnotationDomainType, LineAnnotationSpec, LineAnnotationDatum } from '../../utils/specs';
+import { AnnotationDomainType, LineAnnotationDatum, LineAnnotationSpec } from '../../utils/specs';
 import { AnnotationLineProps } from './types';
 
 function computeYDomainLineAnnotationDimensions(
@@ -155,32 +155,27 @@ function computeXDomainLineAnnotationDimensions(
       if (isHistogramMode) {
         const offset = computeXScaleOffset(xScale, true);
         const pureScaledValue = xScale.pureScale(dataValue);
-        if (pureScaledValue == null) {
-          return;
+        if (typeof pureScaledValue === 'number') {
+          // Number.isFinite is regrettably not a type guard yet https://github.com/microsoft/TypeScript/issues/10038#issuecomment-924115831
+          annotationValueXPosition = pureScaledValue - offset;
         }
-        annotationValueXPosition = pureScaledValue - offset;
       } else {
         annotationValueXPosition += (xScale.bandwidth * xScale.totalBarsInCluster) / 2;
       }
     } else if (isBandScale(xScale)) {
-      if (isHistogramMode) {
-        const padding = (xScale.step - xScale.originalBandwidth) / 2;
-        annotationValueXPosition -= padding;
-      } else {
-        annotationValueXPosition += xScale.originalBandwidth / 2;
-      }
+      annotationValueXPosition += isHistogramMode
+        ? -(xScale.step - xScale.originalBandwidth) / 2
+        : xScale.originalBandwidth / 2;
     } else {
       return;
     }
-    if (isNaN(annotationValueXPosition) || annotationValueXPosition == null) {
+    if (!isFinite(annotationValueXPosition)) {
       return;
     }
 
     vertical.domain.forEach((verticalValue) => {
       horizontal.domain.forEach((horizontalValue) => {
-        if (annotationValueXPosition == null) {
-          return;
-        }
+        if (annotationValueXPosition === null) return;
 
         const top = vertical.scaleOrThrow(verticalValue);
         const left = horizontal.scaleOrThrow(horizontalValue);

diff --git a/packages/charts/src/chart_types/xy_chart/annotations/rect/dimensions.integration.test.ts b/packages/charts/src/chart_types/xy_chart/annotations/rect/dimensions.integration.test.ts
@@ -267,7 +267,7 @@ describe('Render rect annotation within', () => {
     const heightFromStore = 200;
     const store = MockStore.default({ top: 0, left: 0, width: 20, height: heightFromStore });
     const settings = MockGlobalSpec.settingsNoMargins();
-    const yDomainFitted = MockGlobalSpec.axis({ domain: { fit: true } });
+    const yDomainFitted = MockGlobalSpec.axis({ domain: { min: NaN, max: NaN, fit: true } });
     const annotation = MockAnnotationSpec.rect({
       dataValues: [{ coordinates: { x0: 2, x1: 4 } }],
     });