chore: eslint cleanup - eqeqeq, unused experession, index key (flyteo…

…rg#349) * chore: eslint cleanup - eqeqeq, unused experession, index key * chore: update rules no-undef and import/no-unresolved Signed-off-by: Nastya Rusina <[email protected]>
eapolinario · Mar 31, 2022 · ce1f113 · ce1f113
1 parent 5885d18
commit ce1f113
Show file tree

Hide file tree

Showing 36 changed files with 114 additions and 97 deletions.
diff --git a/.eslintrc.js b/.eslintrc.js
@@ -3,6 +3,13 @@ module.exports = {
   env: {
     browser: true,
     es2021: true,
+    jest: true,
+  },
+  globals: {
+    // global variables, that should be assumed by eslint as defined
+    JSX: true,
+    RequiredNonNullable: true,
+    Dictionary: true,
   },
   parser: '@typescript-eslint/parser',
   parserOptions: {
@@ -28,6 +35,7 @@ module.exports = {
      */
     'arrow-body-style': 'off',
     'import/extensions': 'off',
+    'import/no-unresolved': 'off',
     'import/prefer-default-export': 'off',
     'react/jsx-boolean-value': 'off',
     'react/jsx-filename-extension': [2, { extensions: ['.jsx', '.tsx'] }],
@@ -49,20 +57,17 @@ module.exports = {
      * temporarily off or warn
      * */
     // some setup of eslint or prettier needed
-    'no-undef': 'off', // 1066
-    'import/no-unresolved': 'off', // 1275
     'import/no-extraneous-dependencies': 'off', // 715 - !important
     'react/jsx-props-no-spreading': 'off', // 119
 
     // classic
     'no-use-before-define': 'off', // 49
     'no-shadow': 'off', // 104
     'no-param-reassign': 'off', // 28
-    'no-unused-expressions': 'warn', // 6
+    'no-unused-expressions': 'warn', // 5
     'prefer-destructuring': 'off', // 34
-    'max-classes-per-file': 'warn', // 2
     'no-empty-function': 'off',
-    'no-useless-constructor': 'warn',
+    'no-useless-constructor': 'warn', // 1
     'no-useless-computed-key': 'off',
     'no-restricted-syntax': 'off',
     'no-else-return': 'off',
@@ -96,7 +101,7 @@ module.exports = {
     'prefer-promise-reject-errors': 'off',
     'prefer-arrow-callback': 'off',
     'func-names': 'off',
-    eqeqeq: 'warn',
+    eqeqeq: 'warn', // 12
 
     // import
     'import/no-dynamic-require': 'warn', // 1
@@ -113,10 +118,10 @@ module.exports = {
     'react/no-unescaped-entities': 'off',
     'react/require-default-props': 'off',
     'react/no-unused-prop-types': 'off', // 15
-    'react/no-array-index-key': 'warn', // 5
-    'react/static-property-placement': 'off', // 1
-    'react/state-in-constructor': 'off', // 2
-    'react/no-children-prop': 'off', // 1
+    'react/no-array-index-key': 'warn', // 2
+    'react/static-property-placement': 'warn', // 1
+    'react/state-in-constructor': 'warn', // 2
+    'react/no-children-prop': 'warn', // 1
 
     // jsx-a11y
     'jsx-a11y/anchor-is-valid': 'off', // 4
@@ -127,15 +132,23 @@ module.exports = {
   overrides: [
     {
       // overrides for test files
-      files: ['*.spec.*', '*.test.*', 'src/**/test/*'],
+      files: ['*.spec.*', '*.test.*', '*.stories.*', 'src/**/test/*', 'src/**/mocks/*'],
       rules: {
         camelcase: 'off',
         '@typescript-eslint/no-explicit-any': 'off',
         'import/no-extraneous-dependencies': 'off',
+        'no-console': 'off',
 
         'jsx-a11y/aria-role': 'off',
         'jsx-a11y/control-has-associated-label': 'off',
       },
     },
+    {
+      // rules which not make sense for TS files
+      files: ['*.ts', '*.tsx'],
+      rules: {
+        'no-undef': 'off',
+      },
+    },
   ],
 };
diff --git a/index.js b/index.js
@@ -3,6 +3,7 @@
  * This file is NOT compiled and is run directly by Node.js. Make sure you are not using JavaScript features that
  * does not exist in Node.js runtime.
  */
+/* eslint-disable no-console */
 const morgan = require('morgan');
 const express = require('express');
 const env = require('./env');
@@ -80,7 +81,7 @@ if (process.env.NODE_ENV === 'production') {
 /* Set ADMIN_API_USE_SSL to https for CORS support */
 let server;
 const port = process.env.PORT || 3000;
-if (env.ADMIN_API_USE_SSL == 'https') {
+if (env.ADMIN_API_USE_SSL === 'https') {
   const fs = require('fs');
   const https = require('https');
   var privateKey = fs.readFileSync('script/server.key');

diff --git a/plugins.js b/plugins.js
@@ -1,3 +1,4 @@
+/* eslint import/no-dynamic-require: 'off', no-console: 'off' */
 const env = require('./env');
 
 const { middleware } = env.PLUGINS_MODULE ? require(env.PLUGINS_MODULE) : {};

diff --git a/src/basics/LocalCache/index.tsx b/src/basics/LocalCache/index.tsx
@@ -1,4 +1,5 @@
 // More info on Local storage: https://developer.mozilla.org/en-US/docs/Web/API/Window/localStorage
+import { log } from 'common/log';
 import { useState } from 'react';
 import { defaultLocalCacheConfig, LocalCacheItem } from './defaultConfig';
 
@@ -11,7 +12,7 @@ export function ClearLocalCache() {
 const getDefault = (setting: LocalCacheItem) => {
   const result = defaultLocalCacheConfig[setting];
   if (!result) {
-    console.error(
+    log.error(
       `ERROR: LocalCacheItem ${setting} doesn't have default value provided in defaultLocalCacheConfig`,
     );
     return null;

diff --git a/src/client.tsx b/src/client.tsx
@@ -14,7 +14,7 @@ const initializeApp = () => {
 
   const { ENABLE_GA, GA_TRACKING_ID } = env;
 
-  if (ENABLE_GA != 'false') {
+  if (ENABLE_GA !== 'false') {
     ReactGA.initialize(GA_TRACKING_ID as string);
   }
 

diff --git a/src/components/Entities/EntityExecutionsBarChart.tsx b/src/components/Entities/EntityExecutionsBarChart.tsx
@@ -3,7 +3,6 @@ import Typography from '@material-ui/core/Typography';
 import { makeStyles, Theme } from '@material-ui/core/styles';
 import { formatDateUTC, millisecondsToHMS } from 'common/formatters';
 import { timestampToDate } from 'common/utils';
-import { fetchStates } from 'components/hooks/types';
 import { BarChart } from 'components/common/BarChart';
 import { WaitForData } from 'components/common/WaitForData';
 import { useWorkflowExecutionFiltersState } from 'components/Executions/filters/useExecutionFiltersState';

diff --git a/src/components/Executions/ExecutionDetails/ExecutionMetadata.tsx b/src/components/Executions/ExecutionDetails/ExecutionMetadata.tsx
@@ -55,7 +55,7 @@ const useStyles = makeStyles((theme: Theme) => {
 
 interface DetailItem {
   className?: string;
-  label: React.ReactNode;
+  label: ExecutionMetadataLabels;
   value: React.ReactNode;
 }
 
@@ -109,8 +109,8 @@ export const ExecutionMetadata: React.FC<{
   return (
     <div className={styles.container}>
       <div className={styles.detailsContainer}>
-        {details.map(({ className, label, value }, idx) => (
-          <div className={classnames(styles.detailItem, className)} key={idx}>
+        {details.map(({ className, label, value }) => (
+          <div className={classnames(styles.detailItem, className)} key={label}>
             <Typography className={commonStyles.truncateText} variant="subtitle1">
               {label}
             </Typography>

diff --git a/src/components/Executions/ExecutionDetails/ExecutionMetadataExtra.tsx b/src/components/Executions/ExecutionDetails/ExecutionMetadataExtra.tsx
@@ -19,7 +19,7 @@ const useStyles = makeStyles((theme: Theme) => {
 
 interface DetailItem {
   className?: string;
-  label: React.ReactNode;
+  label: ExecutionMetadataLabels;
   value: React.ReactNode;
 }
 
@@ -34,7 +34,12 @@ export const ExecutionMetadataExtra: React.FC<{
   const commonStyles = useCommonStyles();
   const styles = useStyles();
 
-  const { launchPlan: launchPlanId, maxParallelism, rawOutputDataConfig, authRole } = execution.spec;
+  const {
+    launchPlan: launchPlanId,
+    maxParallelism,
+    rawOutputDataConfig,
+    authRole,
+  } = execution.spec;
 
   const [launchPlanSpec, setLaunchPlanSpec] = React.useState<Partial<LaunchPlanSpec>>({});
 
@@ -53,7 +58,9 @@ export const ExecutionMetadataExtra: React.FC<{
     },
     {
       label: ExecutionMetadataLabels.rawOutputPrefix,
-      value: rawOutputDataConfig?.outputLocationPrefix || launchPlanSpec?.rawOutputDataConfig?.outputLocationPrefix,
+      value:
+        rawOutputDataConfig?.outputLocationPrefix ||
+        launchPlanSpec?.rawOutputDataConfig?.outputLocationPrefix,
     },
     {
       label: ExecutionMetadataLabels.parallelism,
@@ -63,8 +70,8 @@ export const ExecutionMetadataExtra: React.FC<{
 
   return (
     <>
-      {details.map(({ className, label, value }, idx) => (
-        <div className={classnames(styles.detailItem, className)} key={idx}>
+      {details.map(({ className, label, value }) => (
+        <div className={classnames(styles.detailItem, className)} key={label}>
           <Typography className={commonStyles.truncateText} variant="subtitle1">
             {label}
           </Typography>

diff --git a/src/components/Executions/ExecutionDetails/NodeExecutionDetailsPanelContent.tsx b/src/components/Executions/ExecutionDetails/NodeExecutionDetailsPanelContent.tsx
@@ -1,4 +1,4 @@
-import { useEffect, useRef, useState } from 'react';
+import { useEffect, useRef } from 'react';
 import { IconButton, Typography } from '@material-ui/core';
 import { makeStyles, Theme } from '@material-ui/core/styles';
 import Tab from '@material-ui/core/Tab';

diff --git a/src/components/Executions/ExecutionDetails/Timeline/scaleContext.tsx b/src/components/Executions/ExecutionDetails/Timeline/scaleContext.tsx
@@ -1,4 +1,5 @@
 import { Mark } from '@material-ui/core/Slider';
+import { log } from 'common/log';
 import * as React from 'react';
 import { createContext, useContext } from 'react';
 import { formatSecondsToHmsFormat } from './BarChart/utils';
@@ -27,10 +28,10 @@ export const ScaleContext = createContext<TimelineScaleState>({
   chartInterval: 20,
   marks: [],
   setMaxValue: () => {
-    console.error('ERROR: No ScaleContextProvider was found in parent components.');
+    log.error('ERROR: No ScaleContextProvider was found in parent components.');
   },
   setScaleFactor: () => {
-    console.error('ERROR: No ScaleContextProvider was found in parent components.');
+    log.error('ERROR: No ScaleContextProvider was found in parent components.');
   },
 });
 

diff --git a/src/components/Executions/ExecutionDetails/test/BarChart.test.tsx b/src/components/Executions/ExecutionDetails/test/BarChart.test.tsx
@@ -1,12 +1,11 @@
-import { getChartDurationData } from '../Timeline/BarChart/chartData';
 import {
   CASHED_GREEN,
   formatSecondsToHmsFormat,
   generateChartData,
   getOffsetColor,
   TRANSPARENT,
 } from '../Timeline/BarChart/utils';
-import { getMockExecutionsForBarChart, mockbarItems } from './__mocks__/NodeExecution.mock';
+import { mockbarItems } from './__mocks__/NodeExecution.mock';
 
 describe('ExecutionDetails > Timeline > BarChart', () => {
   it('formatSecondsToHmsFormat works as expected', () => {
@@ -39,22 +38,6 @@ describe('ExecutionDetails > Timeline > BarChart', () => {
     expect(offsetColors[1]).toEqual(CASHED_GREEN);
   });
 
-  // Mock bars used below
-  // const mockbarItems = [
-  //   { phase: NodeExecutionPhase.FAILED, startOffsetSec: 0, durationSec: 15, isFromCache: false },
-  //   { phase: NodeExecutionPhase.SUCCEEDED, startOffsetSec: 5, durationSec: 11, isFromCache: true },
-  //   { phase: NodeExecutionPhase.RUNNING, startOffsetSec: 17, durationSec: 23, isFromCache: false },
-  //   { phase: NodeExecutionPhase.QUEUED, startOffsetSec: 39, durationSec: 0, isFromCache: false },
-  // ];
-  // it('getChartDurationData is properly generated from Node[] items', () => {
-  //   /** */
-  //   const startTime = 1642627611;
-  //   const mockData = getMockExecutionsForBarChart(startTime);
-  //   const chartItems = getChartDurationData(mockData, new Date(1642627611 * 1000));
-
-  //   expect(chartItems[0]).toEqual(mockbarItems[0]);
-  // });
-
   it('generateChartData properly generates map of data for ChartBars', () => {
     const chartData = generateChartData(mockbarItems);
     expect(chartData.durations).toEqual([15, 11, 23, 0]);

diff --git a/src/components/Executions/ExecutionDetails/test/ExecutionDetailsAppBarContent.test.tsx b/src/components/Executions/ExecutionDetails/test/ExecutionDetailsAppBarContent.test.tsx
@@ -58,14 +58,13 @@ describe('ExecutionDetailsAppBarContent', () => {
     describe('in overflow menu', () => {
       let renderResult: RenderResult;
       let buttonEl: HTMLElement;
-      let menuEl: HTMLElement;
 
       beforeEach(async () => {
         renderResult = renderContent();
         const { getByLabelText } = renderResult;
         buttonEl = await waitFor(() => getByLabelText(commonLabels.moreOptionsButton));
         fireEvent.click(buttonEl);
-        menuEl = await waitFor(() => getByLabelText(commonLabels.moreOptionsMenu));
+        await waitFor(() => getByLabelText(commonLabels.moreOptionsMenu));
       });
 
       it('renders a clone option', () => {

diff --git a/src/components/Executions/Tables/WorkflowExecutionLink.tsx b/src/components/Executions/Tables/WorkflowExecutionLink.tsx
@@ -18,7 +18,7 @@ export const WorkflowExecutionLink: React.FC<{
   } = history;
   const fromExecutionNav = pathname.split('/').pop() === 'executions';
 
-  const linkColor = color == 'disabled' ? commonStyles.secondaryLink : commonStyles.primaryLink;
+  const linkColor = color === 'disabled' ? commonStyles.secondaryLink : commonStyles.primaryLink;
   return (
     <RouterLink
       className={classnames(linkColor, className)}

diff --git a/src/components/Executions/contextProvider/NodeExecutionDetails/index.tsx b/src/components/Executions/contextProvider/NodeExecutionDetails/index.tsx
@@ -1,5 +1,6 @@
 import * as React from 'react';
 import { createContext, useContext, useEffect, useRef, useState } from 'react';
+import { log } from 'common/log';
 import { Identifier } from 'models/Common/types';
 import { NodeExecution } from 'models/Execution/types';
 import { CompiledWorkflowClosure } from 'models/Workflow/types';
@@ -21,7 +22,7 @@ const NOT_AVAILABLE = 'NotAvailable';
 export const NodeExecutionDetailsContext = createContext<NodeExecutionDetailsState>({
   /** Default values used if ContextProvider wasn't initialized. */
   getNodeExecutionDetails: async () => {
-    console.error('ERROR: No NodeExecutionDetailsContextProvider was found in parent components.');
+    log.error('ERROR: No NodeExecutionDetailsContextProvider was found in parent components.');
     return UNKNOWN_DETAILS;
   },
   workflowId: {

diff --git a/src/components/Launch/LaunchForm/CollectionInput.tsx b/src/components/Launch/LaunchForm/CollectionInput.tsx
@@ -1,5 +1,6 @@
 import { TextField } from '@material-ui/core';
 import * as React from 'react';
+import { log } from 'common/log';
 import { makeStringChangeHandler } from './handlers';
 import { InputProps, InputType } from './types';
 import { UnsupportedInput } from './UnsupportedInput';
@@ -16,7 +17,7 @@ export const CollectionInput: React.FC<InputProps> = (props) => {
     value = '',
   } = props;
   if (!subtype) {
-    console.error('Unexpected missing subtype for collection input', props.typeDefinition);
+    log.warn('Unexpected missing subtype for collection input', props.typeDefinition);
     return <UnsupportedInput {...props} />;
   }
   const hasError = !!error;

diff --git a/src/components/Launch/LaunchForm/LaunchFormAdvancedInputs.tsx b/src/components/Launch/LaunchForm/LaunchFormAdvancedInputs.tsx
@@ -1,4 +1,5 @@
 import * as React from 'react';
+import { Admin } from 'flyteidl';
 import { createMuiTheme, MuiThemeProvider } from '@material-ui/core/styles';
 import Accordion from '@material-ui/core/Accordion';
 import AccordionSummary from '@material-ui/core/AccordionSummary';
@@ -11,7 +12,6 @@ import TextField from '@material-ui/core/TextField';
 import Typography from '@material-ui/core/Typography';
 import ExpandMoreIcon from '@material-ui/icons/ExpandMore';
 import Form from '@rjsf/material-ui';
-import { flyteidl } from '@flyteorg/flyteidl/gen/pb-js/flyteidl';
 import { State } from 'xstate';
 import { LaunchAdvancedOptionsRef } from './types';
 import {
@@ -21,8 +21,6 @@ import {
 } from './launchMachine';
 import { useStyles } from './styles';
 
-import IExecutionSpec = flyteidl.admin.IExecutionSpec;
-
 const muiTheme = createMuiTheme({
   props: {
     MuiTextField: {
@@ -86,7 +84,7 @@ export const LaunchFormAdvancedInputs = React.forwardRef<
       }
       if (
         other?.rawOutputDataConfig?.outputLocationPrefix !== undefined &&
-          other.rawOutputDataConfig.outputLocationPrefix !== null
+        other.rawOutputDataConfig.outputLocationPrefix !== null
       ) {
         setRawOutputDataConfig(other.rawOutputDataConfig.outputLocationPrefix);
       }
@@ -100,7 +98,14 @@ export const LaunchFormAdvancedInputs = React.forwardRef<
       };
       setLabelsParamData(newLabels);
       setAnnotationsParamData(newAnnotations);
-    }, [other.disableAll, other.maxParallelism, other.rawOutputDataConfig, other.labels, other.annotations, launchPlan?.spec]);
+    }, [
+      other.disableAll,
+      other.maxParallelism,
+      other.rawOutputDataConfig,
+      other.labels,
+      other.annotations,
+      launchPlan?.spec,
+    ]);
 
     React.useImperativeHandle(
       ref,
@@ -109,7 +114,7 @@ export const LaunchFormAdvancedInputs = React.forwardRef<
           return {
             disableAll,
             rawOutputDataConfig: {
-              outputLocationPrefix: rawOutputDataConfig
+              outputLocationPrefix: rawOutputDataConfig,
             },
             maxParallelism: parseInt(maxParallelism || '', 10),
             labels: {
@@ -118,7 +123,7 @@ export const LaunchFormAdvancedInputs = React.forwardRef<
             annotations: {
               values: annotationsParamData,
             },
-          } as IExecutionSpec;
+          } as Admin.IExecutionSpec;
         },
         validate: () => {
           return true;