Merge pull request #1073 from spotify/rugvip/jestbump

packages/cli: fix linting and enable TS isolatedModules

packages/backend/src/plugins/auth.ts

@@ -17,6 +17,6 @@
 import { createRouter } from '@backstage/plugin-auth-backend';
 import { PluginEnvironment } from '../types';
 
-export default async function ({ logger }: PluginEnvironment) {
+export default async function createPlugin({ logger }: PluginEnvironment) {
   return await createRouter({ logger });
 }

packages/backend/src/plugins/catalog.ts

@@ -27,7 +27,10 @@ import {
 import { PluginEnvironment } from '../types';
 import { EntityPolicies } from '@backstage/catalog-model';
 
-export default async function ({ logger, database }: PluginEnvironment) {
+export default async function createPlugin({
+  logger,
+  database,
+}: PluginEnvironment) {
   const policy = new EntityPolicies();
   const ingestion = new IngestionModels(
     new LocationReaders(),

packages/backend/src/plugins/identity.ts

@@ -17,6 +17,6 @@
 import { createRouter } from '@backstage/plugin-identity-backend';
 import { PluginEnvironment } from '../types';
 
-export default async function ({ logger }: PluginEnvironment) {
+export default async function createPlugin({ logger }: PluginEnvironment) {
   return await createRouter({ logger });
 }

packages/backend/src/plugins/scaffolder.ts

@@ -21,7 +21,7 @@ import {
 } from '@backstage/plugin-scaffolder-backend';
 import type { PluginEnvironment } from '../types';
 
-export default async function ({ logger }: PluginEnvironment) {
+export default async function createPlugin({ logger }: PluginEnvironment) {
   const storage = new DiskStorage({ logger });
   const templater = new CookieCutter();
 

packages/backend/src/plugins/sentry.ts

@@ -17,6 +17,6 @@
 import { createRouter } from '@backstage/plugin-sentry-backend';
 import { Logger } from 'winston';
 
-export default async function (logger: Logger) {
+export default async function createPlugin(logger: Logger) {
   return await createRouter(logger);
 }

packages/catalog-model/src/kinds/index.ts

@@ -14,7 +14,8 @@
  * limitations under the License.
  */
 
-import type { ComponentV1beta1 } from './ComponentV1beta1';
+export type {
+  ComponentV1beta1,
+  ComponentV1beta1 as Component,
+} from './ComponentV1beta1';
 export { ComponentV1beta1Policy } from './ComponentV1beta1';
-export { ComponentV1beta1 as Component };
-export { ComponentV1beta1 };

packages/catalog-model/src/setupTests.ts

@@ -13,3 +13,5 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
+
+export {};

packages/cli/config/eslint.backend.js

@@ -32,7 +32,7 @@ module.exports = {
     ecmaVersion: 2018,
     sourceType: 'module',
   },
-  ignorePatterns: ['**/dist/**', '**/build/**'],
+  ignorePatterns: ['.eslintrc.js', '**/dist/**'],
   rules: {
     'no-console': 0, // Permitted in console programs
     'new-cap': ['error', { capIsNew: false }], // Because Express constructs things e.g. like 'const r = express.Router()'

packages/cli/config/eslint.js

@@ -39,7 +39,7 @@ module.exports = {
       version: 'detect',
     },
   },
-  ignorePatterns: ['**/dist/**', '**/build/**'],
+  ignorePatterns: ['.eslintrc.js', '**/dist/**'],
   rules: {
     'import/no-duplicates': 'warn',
     'import/no-extraneous-dependencies': [

packages/cli/config/tsconfig.json

@@ -1,17 +1,37 @@
 {
-  "extends": "@spotify/web-scripts/config/tsconfig.json",
-  "exclude": ["**/*.test.*"],
   "compilerOptions": {
     "allowJs": true,
-    "noEmit": false,
+    "declaration": true,
+    "declarationMap": false,
     "emitDeclarationOnly": true,
+    "esModuleInterop": true,
+    "experimentalDecorators": false,
+    "forceConsistentCasingInFileNames": true,
+    "importHelpers": false,
     "incremental": true,
-    "target": "ES2019",
+    "isolatedModules": true,
+    "jsx": "react",
+    "lib": ["DOM", "DOM.Iterable", "ScriptHost", "ES2019"],
     "module": "ESNext",
+    "moduleResolution": "node",
+    "noEmit": false,
+    "noFallthroughCasesInSwitch": true,
+    "noImplicitAny": true,
+    "noImplicitReturns": true,
+    "noImplicitThis": true,
+    "noUnusedLocals": true,
+    "noUnusedParameters": true,
+    "pretty": true,
     "removeComments": false,
     "resolveJsonModule": true,
-    "esModuleInterop": true,
-    "lib": ["DOM", "DOM.Iterable", "ScriptHost", "ES2019"],
+    "sourceMap": false,
+    "strict": true,
+    "strictBindCallApply": true,
+    "strictFunctionTypes": true,
+    "strictNullChecks": true,
+    "strictPropertyInitialization": true,
+    "stripInternal": true,
+    "target": "ES2019",
     "types": ["node", "jest"]
   }
 }

packages/cli/package.json

@@ -35,7 +35,7 @@
     "@rollup/plugin-commonjs": "^11.0.2",
     "@rollup/plugin-json": "^4.0.2",
     "@rollup/plugin-node-resolve": "^7.1.1",
-    "@spotify/web-scripts": "^6.0.0",
+    "@spotify/eslint-config": "^7.0.1",
     "@sucrase/webpack-loader": "^2.0.0",
     "bfj": "^7.0.2",
     "chalk": "^4.0.0",
@@ -44,6 +44,7 @@
     "css-loader": "^3.5.3",
     "dashify": "^2.0.0",
     "diff": "^4.0.2",
+    "eslint": "^7.1.0",
     "eslint-plugin-import": "^2.20.2",
     "eslint-plugin-monorepo": "^0.2.1",
     "fork-ts-checker-webpack-plugin": "^4.0.5",
@@ -74,6 +75,7 @@
     "tar": "^6.0.1",
     "ts-jest": "^26.0.0",
     "ts-loader": "^7.0.4",
+    "typescript": "^3.9.3",
     "url-loader": "^4.1.0",
     "webpack": "^4.41.6",
     "webpack-dev-server": "^3.10.3",

packages/cli/src/commands/lint.ts

@@ -16,12 +16,18 @@
 
 import { Command } from 'commander';
 import { run } from '../lib/run';
+import { paths } from '../lib/paths';
 
 export default async (cmd: Command) => {
-  const args = ['lint', '--max-warnings=0', '--format=codeframe'];
+  const args = [
+    '--ext=js,jsx,ts,tsx',
+    '--max-warnings=0',
+    '--format=codeframe',
+    paths.targetDir,
+  ];
   if (cmd.fix) {
     args.push('--fix');
   }
 
-  await run('web-scripts', args);
+  await run('eslint', args);
 };

packages/core-api/src/app/AppThemeProvider.tsx

@@ -49,10 +49,6 @@ function resolveTheme(
 }
 
 const useShouldPreferDarkTheme = () => {
-  if (!window.matchMedia) {
-    return false;
-  }
-
   const mediaQuery = useMemo(
     () => window.matchMedia('(prefers-color-scheme: dark)'),
     [],
@@ -74,12 +70,16 @@ const useShouldPreferDarkTheme = () => {
 
 export const AppThemeProvider: FC<{}> = ({ children }) => {
   const appThemeApi = useApi(appThemeApiRef);
-  const shouldPreferDark = useShouldPreferDarkTheme();
   const themeId = useObservable(
     appThemeApi.activeThemeId$(),
     appThemeApi.getActiveThemeId(),
   );
 
+  // Browser feature detection won't change over time, so ignore lint rule
+  const shouldPreferDark = Boolean(window.matchMedia)
+    ? useShouldPreferDarkTheme() // eslint-disable-line react-hooks/rules-of-hooks
+    : false;
+
   const appTheme = resolveTheme(
     themeId,
     shouldPreferDark,

packages/core/src/components/FeatureDiscovery/FeatureCalloutCircular.tsx

@@ -141,9 +141,9 @@ export const FeatureCalloutCircular: FC<Props> = ({
       window.removeEventListener('resize', update);
       window.removeEventListener('scroll', update);
     };
-  }, []);
+  }, [update]);
 
-  useLayoutEffect(update, [wrapperRef.current]);
+  useLayoutEffect(update, [wrapperRef.current, update]);
 
   if (!show) {
     return <>{children}</>;

packages/core/src/components/FeatureDiscovery/lib/usePortal.ts

@@ -51,27 +51,30 @@ function addRootElement(rootElem: Element): void {
 export function usePortal(id: string): HTMLElement {
   const rootElemRef = useRef<HTMLElement | null>(null);
 
-  useEffect(function setupElement() {
-    // Look for existing target dom element to append to
-    const existingParent = document.querySelector(`#${id}`);
-    // Parent is either a new root or the existing dom element
-    const parentElem = existingParent || createRootElement(id);
+  useEffect(
+    function setupElement() {
+      // Look for existing target dom element to append to
+      const existingParent = document.querySelector(`#${id}`);
+      // Parent is either a new root or the existing dom element
+      const parentElem = existingParent || createRootElement(id);
 
-    // If there is no existing DOM element, add a new one.
-    if (!existingParent) {
-      addRootElement(parentElem);
-    }
-
-    // Add the detached element to the parent
-    parentElem.appendChild(rootElemRef.current!);
-
-    return function removeElement() {
-      rootElemRef.current!.remove();
-      if (parentElem.childNodes.length === -1) {
-        parentElem.remove();
+      // If there is no existing DOM element, add a new one.
+      if (!existingParent) {
+        addRootElement(parentElem);
       }
-    };
-  }, []);
+
+      // Add the detached element to the parent
+      parentElem.appendChild(rootElemRef.current!);
+
+      return function removeElement() {
+        rootElemRef.current!.remove();
+        if (parentElem.childNodes.length === -1) {
+          parentElem.remove();
+        }
+      };
+    },
+    [id],
+  );
 
   /**
    * It's important we evaluate this lazily:

packages/core/src/components/FeatureDiscovery/lib/useShowCallout.ts

@@ -45,7 +45,7 @@ function useCalloutHasBeenSeen(
 
   const markSeen = useCallback(() => {
     setState(featureId, true);
-  }, [featureId]);
+  }, [setState, featureId]);
 
   return { seen: states[featureId] === true, markSeen };
 }

packages/core/src/components/ProgressBars/HorizontalProgress.tsx

@@ -29,6 +29,7 @@ type Props = {
 };
 
 const HorizontalProgress: FC<Props> = ({ value }) => {
+  const theme = useTheme<BackstageTheme>();
   if (isNaN(value)) {
     return null;
   }
@@ -36,7 +37,6 @@ const HorizontalProgress: FC<Props> = ({ value }) => {
   if (percent > 100) {
     percent = 100;
   }
-  const theme = useTheme<BackstageTheme>();
   const strokeColor = getProgressColor(theme.palette, percent, false, 100);
   return (
     <Tooltip title={`${percent}%`}>

plugins/auth-backend/src/providers/OAuthProvider.test.ts

@@ -1,21 +0,0 @@
-/*
- * Copyright 2020 Spotify AB
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-describe('OAuthProvider', () => {
-  it('unbreak test runner', () => {
-    expect(true).toBeTruthy();
-  });
-});

plugins/auth-backend/src/providers/index.test.ts

@@ -14,8 +14,10 @@
  * limitations under the License.
  */
 
+import { defaultRouter } from '.';
+
 describe('test', () => {
   it('unbreaks the test runner', () => {
-    expect(true).toBeTruthy();
+    expect(defaultRouter).toBeDefined();
   });
 });

plugins/catalog/src/components/ComponentPage/ComponentPage.test.tsx

@@ -18,6 +18,7 @@ import { render } from '@testing-library/react';
 import * as React from 'react';
 import { wrapInTestApp } from '@backstage/test-utils';
 import { ApiProvider, ApiRegistry, errorApiRef } from '@backstage/core';
+import { catalogApiRef, CatalogApi } from '../../api/types';
 
 const getTestProps = (componentName: string) => {
   return {
@@ -39,7 +40,17 @@ describe('ComponentPage', () => {
     const props = getTestProps('');
     await render(
       wrapInTestApp(
-        <ApiProvider apis={ApiRegistry.from([[errorApiRef, errorApi]])}>
+        <ApiProvider
+          apis={ApiRegistry.from([
+            [errorApiRef, errorApi],
+            [
+              catalogApiRef,
+              ({
+                async getEntityByName() {},
+              } as unknown) as CatalogApi,
+            ],
+          ])}
+        >
           <ComponentPage {...props} />
         </ApiProvider>,
       ),

plugins/catalog/src/components/ComponentPage/ComponentPage.tsx

@@ -53,11 +53,6 @@ const ComponentPage: FC<ComponentPageProps> = ({ match, history }) => {
   const componentName = match.params.name;
   const errorApi = useApi<ErrorApi>(errorApiRef);
 
-  if (componentName === '') {
-    history.push('/catalog');
-    return null;
-  }
-
   const catalogApi = useApi(catalogApiRef);
   const catalogRequest = useAsync(() =>
     catalogApi.getEntityByName(match.params.name),
@@ -70,7 +65,12 @@ const ComponentPage: FC<ComponentPageProps> = ({ match, history }) => {
         history.push('/catalog');
       }, REDIRECT_DELAY);
     }
-  }, [catalogRequest.error]);
+  }, [catalogRequest.error, errorApi, history]);
+
+  if (componentName === '') {
+    history.push('/catalog');
+    return null;
+  }
 
   const removeComponent = async () => {
     setConfirmationDialogOpen(false);

plugins/circleci/src/components/Settings/Settings.tsx

@@ -52,7 +52,7 @@ const Settings = () => {
     if (repoFromStore !== repo) {
       setRepo(repoFromStore);
     }
-  }, [ownerFromStore, repoFromStore, tokenFromStore]);
+  }, [ownerFromStore, repoFromStore, tokenFromStore, token, owner, repo]);
 
   const [saved, setSaved] = useState(false);
 

plugins/circleci/src/pages/BuildWithStepsPage/BuildWithStepsPage.tsx

@@ -111,7 +111,7 @@ const BuildWithStepsView: FC<{}> = () => {
   useEffect(() => {
     startPolling();
     return () => stopPolling();
-  }, [buildId, settings]);
+  }, [buildId, settings, startPolling, stopPolling]);
 
   return (
     <>

plugins/circleci/src/state/useBuildWithSteps.ts

@@ -46,7 +46,7 @@ export function useBuildWithSteps(buildId: number) {
       errorApi.post(e);
       return Promise.reject(e);
     }
-  }, [token, owner, repo, buildId]);
+  }, [token, owner, repo, buildId, api, errorApi]);
 
   const restartBuild = async () => {
     try {

plugins/circleci/src/state/useBuilds.ts

@@ -101,7 +101,7 @@ export function useBuilds() {
         return Promise.reject(e);
       }
     },
-    [repo, token, owner],
+    [repo, token, owner, api, errorApi],
   );
 
   const restartBuild = async (buildId: number) => {
@@ -121,7 +121,7 @@ export function useBuilds() {
 
   useEffect(() => {
     getBuilds({ limit: 1, offset: 0 }).then(b => setTotal(b?.[0].build_num!));
-  }, [repo]);
+  }, [repo, getBuilds]);
 
   const { loading, value, retry } = useAsyncRetry(
     () =>

plugins/circleci/src/state/useSettings.ts

@@ -23,27 +23,29 @@ export function useSettings() {
 
   const errorApi = useApi(errorApiRef);
 
-  const rehydrate = () => {
-    try {
-      const stateFromStorage = JSON.parse(sessionStorage.getItem(STORAGE_KEY)!);
-      if (
-        stateFromStorage &&
-        Object.keys(stateFromStorage).some(
-          k => (settings as any)[k] !== stateFromStorage[k],
-        )
-      )
-        dispatch({
-          type: 'setCredentials',
-          payload: stateFromStorage,
-        });
-    } catch (error) {
-      errorApi.post(error);
-    }
-  };
-
   useEffect(() => {
+    const rehydrate = () => {
+      try {
+        const stateFromStorage = JSON.parse(
+          sessionStorage.getItem(STORAGE_KEY)!,
+        );
+        if (
+          stateFromStorage &&
+          Object.keys(stateFromStorage).some(
+            k => (settings as any)[k] !== stateFromStorage[k],
+          )
+        )
+          dispatch({
+            type: 'setCredentials',
+            payload: stateFromStorage,
+          });
+      } catch (error) {
+        errorApi.post(error);
+      }
+    };
+
     rehydrate();
-  }, []);
+  }, [dispatch, errorApi, settings]);
 
   const persist = (state: Settings) => {
     sessionStorage.setItem(STORAGE_KEY, JSON.stringify(state));

plugins/lighthouse/src/components/AuditList/index.tsx

@@ -63,7 +63,7 @@ const AuditList: FC<{}> = () => {
     if (value?.total && value?.limit)
       return Math.ceil(value?.total / value?.limit);
     return 0;
-  }, [value]);
+  }, [value?.total, value?.limit]);
 
   const history = useHistory();
 

plugins/sentry/src/components/SentryPluginWidget/SentryPluginWidget.tsx

@@ -43,7 +43,7 @@ export const SentryPluginWidget: FC<{
     if (error) {
       errorApi.post(error);
     }
-  }, [error]);
+  }, [error, errorApi]);
 
   if (loading) {
     return (

plugins/tech-radar/src/components/RadarComponent.tsx

@@ -21,6 +21,7 @@ import { TechRadarComponentProps, TechRadarLoaderResponse } from '../api';
 import getSampleData from '../sampleData';
 
 const useTechRadarLoader = (props: TechRadarComponentProps) => {
+  const errorApi = useApi<ErrorApi>(errorApiRef);
   const [state, setState] = useState<{
     loading: boolean;
     error?: Error;
@@ -31,38 +32,33 @@ const useTechRadarLoader = (props: TechRadarComponentProps) => {
     data: undefined,
   });
 
+  const { getData } = props;
+
   useEffect(() => {
-    if (!props.getData) {
+    if (!getData) {
       return;
     }
 
-    props
-      .getData()
+    getData()
       .then((payload: TechRadarLoaderResponse) => {
         setState({ loading: false, error: undefined, data: payload });
       })
       .catch((err: Error) => {
+        errorApi.post(err);
         setState({
           loading: false,
           error: err,
           data: undefined,
         });
       });
-  }, []);
+  }, [getData, errorApi]);
 
   return state;
 };
 
-const RadarComponent: FC<TechRadarComponentProps> = (props) => {
-  const errorApi = useApi<ErrorApi>(errorApiRef);
+const RadarComponent: FC<TechRadarComponentProps> = props => {
   const { loading, error, data } = useTechRadarLoader(props);
 
-  useEffect(() => {
-    if (error) {
-      errorApi.post(error);
-    }
-  }, [error && error.message]);
-
   return (
     <>
       {loading && <Progress />}

tsconfig.json

@@ -1,7 +1,6 @@
 {
   "extends": "@backstage/cli/config/tsconfig.json",
   "include": ["packages/*/src", "plugins/*/src", "plugins/*/dev"],
-  "exclude": ["**/node_modules"],
   "compilerOptions": {
     "outDir": "dist"
   }