Disable type check for test, start, build commands in CI (#606)

* disable type check for test, start, build in CI

* remove check on tscCompileOnError

* add changeset

* tsc api to print flat for non CI or pretty for CI

* add additional changeset

* add silent option

* added comments and moved getPackageMetadata

* moved reporttsdiagnostics into utils

* update changeset to minor

* add typecheck to workflow

* remove unnecessary typecheck step

* add unit tests and fixture and update logger

* Add more comments for emitDiagnostics

* remove silent flag

Co-authored-by: Cang Truong <[email protected]>

.changeset/cyan-moose-judge.md

@@ -0,0 +1,5 @@
+---
+'modular-scripts': minor
+---
+
+Disable typechecking for modular start, test, build commands in CI environments

.changeset/polite-lizards-thank.md

@@ -0,0 +1,5 @@
+---
+'modular-scripts': minor
+---
+
+Added `modular typecheck` command to programatically type check the project

.github/workflows/node.js.yml

@@ -14,7 +14,7 @@ jobs:
     runs-on: ubuntu-latest
     strategy:
       matrix:
-        command: ['prettier --check .', tsc, lint]
+        command: ['prettier --check .', typecheck, lint]
     steps:
       - uses: actions/checkout@v2
       - name: Use Node.js

package.json

@@ -17,7 +17,8 @@
     "build": "yarn workspace create-modular-react-app build && yarn workspace modular-scripts build && yarn workspace modular-views.macro build",
     "release": "yarn build && changeset publish",
     "start": "yarn modular start modular-site",
-    "postinstall": "is-ci || husky install"
+    "postinstall": "is-ci || husky install",
+    "typecheck": "yarn modular typecheck"
   },
   "dependencies": {
     "@babel/cli": "^7.14.5",

packages/modular-scripts/react-scripts/config/webpack.config.js

@@ -25,8 +25,8 @@ const ModuleNotFoundPlugin = require('../../react-dev-utils/ModuleNotFoundPlugin
 const ForkTsCheckerWebpackPlugin = require('fork-ts-checker-webpack-plugin');
 const typescriptFormatter = require('../../react-dev-utils/typescriptFormatter');
 const ReactRefreshWebpackPlugin = require('@pmmmwh/react-refresh-webpack-plugin');
-
 const postcssNormalize = require('postcss-normalize');
+const isCI = require('is-ci');
 
 const appPackageJson = require(paths.appPackageJson);
 
@@ -634,8 +634,10 @@ module.exports = function (webpackEnv) {
           // See https://github.com/cra-template/pwa/issues/13#issuecomment-722667270
           maximumFileSizeToCacheInBytes: 5 * 1024 * 1024,
         }),
-      // TypeScript type checking
+      // TypeScript type checking turned off for CI envs
+      // https://github.com/jpmorganchase/modular/issues/605
       useTypeScript &&
+        !isCI &&
         new ForkTsCheckerWebpackPlugin({
           typescript: resolve.sync('typescript', {
             basedir: paths.appNodeModules,

packages/modular-scripts/react-scripts/scripts/start.js

@@ -28,6 +28,8 @@ const {
 } = require('../../react-dev-utils/WebpackDevServerUtils');
 const openBrowser = require('../../react-dev-utils/openBrowser');
 const semver = require('semver');
+const isCI = require('is-ci');
+
 const paths = require('../config/paths');
 const configFactory = require('../config/webpack.config');
 const createDevServerConfig = require('../config/webpackDevServer.config');
@@ -82,7 +84,7 @@ checkBrowsers(paths.appPath, isInteractive)
     const protocol = process.env.HTTPS === 'true' ? 'https' : 'http';
     const appName = require(paths.appPackageJson).name;
 
-    const useTypeScript = fs.existsSync(paths.appTsConfig);
+    const useTypeScript = !isCI && fs.existsSync(paths.appTsConfig);
     const tscCompileOnError = process.env.TSC_COMPILE_ON_ERROR === 'true';
     const urls = prepareUrls(
       protocol,
@@ -97,6 +99,7 @@ checkBrowsers(paths.appPath, isInteractive)
         devServer.sockWrite(devServer.sockets, 'errors', errors),
     };
     // Create a webpack compiler that is configured with custom messages.
+    // Only run typecheck if not in CI env
     const compiler = createCompiler({
       appName,
       config,

packages/modular-scripts/src/__tests__/__fixtures__/typecheck/InvalidTyping.ts

@@ -0,0 +1,12 @@
+/* eslint-disable */
+//@ts-nocheck
+
+import foo from 'foo';
+
+function convertToCelcius(temp: number): number {
+  const result = (temp - 32) * (5 / 9);
+}
+
+window.__invalid__ = foo.bar;
+
+convertToCelcius('75');

packages/modular-scripts/src/__tests__/typecheck.test.ts

@@ -0,0 +1,105 @@
+import * as path from 'path';
+import * as fs from 'fs-extra';
+import execa from 'execa';
+
+jest.setTimeout(10 * 60 * 1000);
+
+const fixturesFolder = path.join(__dirname, '__fixtures__');
+
+describe('Modular typecheck', () => {
+  describe('when there are type errors', () => {
+    beforeEach(() => {
+      fs.writeFileSync(
+        path.join(fixturesFolder, 'typecheck', 'InvalidTyping.ts'),
+        fs
+          .readFileSync(
+            path.join(fixturesFolder, 'typecheck', 'InvalidTyping.ts'),
+            'utf-8',
+          )
+          .replace('//@ts-nocheck', '//'),
+      );
+    });
+
+    afterEach(() => {
+      fs.writeFileSync(
+        path.join(fixturesFolder, 'typecheck', 'InvalidTyping.ts'),
+        fs
+          .readFileSync(
+            path.join(fixturesFolder, 'typecheck', 'InvalidTyping.ts'),
+            'utf-8',
+          )
+          .replace('//', '//@ts-nocheck'),
+      );
+    });
+
+    describe('when in CI', () => {
+      beforeEach(() => {
+        process.env.CI = 'true';
+      });
+      afterEach(() => {
+        process.env.CI = undefined;
+      });
+      it('should display truncated errors', async () => {
+        let tsc = '';
+        try {
+          await execa('tsc', ['--noEmit', '--pretty', 'false'], {
+            all: true,
+            cleanup: true,
+          });
+        } catch ({ stdout }) {
+          tsc = stdout as string;
+        }
+        let modularStdErr = '';
+        try {
+          await execa('yarnpkg', ['modular', 'typecheck'], {
+            all: true,
+            cleanup: true,
+          });
+        } catch ({ stderr }) {
+          modularStdErr = stderr as string;
+        }
+        const tscErrors = tsc.split('\n');
+        const modularErrors = modularStdErr.split('\n');
+        tscErrors.forEach((errorMessage: string, i: number) => {
+          expect(modularErrors[i]).toMatch(errorMessage);
+        });
+      });
+    });
+    describe('when not in CI', () => {
+      it('should match display full error logs', async () => {
+        let tsc = '';
+        try {
+          await execa('tsc', ['--noEmit'], {
+            all: true,
+            cleanup: true,
+          });
+        } catch ({ stdout }) {
+          tsc = stdout as string;
+        }
+        let modularStdErr = '';
+        try {
+          await execa('yarnpkg', ['modular', 'typecheck'], {
+            all: true,
+            cleanup: true,
+          });
+        } catch ({ stderr }) {
+          modularStdErr = stderr as string;
+        }
+        const tscErrors = tsc.split('\n');
+        const modularErrors = modularStdErr.split('\n');
+        tscErrors.forEach((errorMessage: string, i: number) => {
+          expect(modularErrors[i]).toMatch(errorMessage);
+        });
+      });
+    });
+  });
+  describe('when there are no type errors', () => {
+    it('should print a one line success message', async () => {
+      const result = await execa('yarnpkg', ['modular', 'typecheck'], {
+        all: true,
+        cleanup: true,
+      });
+      expect(result.stdout).toMatch('\u2713 Typecheck passed');
+    });
+  });
+});

packages/modular-scripts/src/buildPackage/getPackageEntryPoints.ts

@@ -1,6 +1,6 @@
+import getPackageMetadata from '../utils/getPackageMetadata';
 import * as path from 'path';
 import * as fse from 'fs-extra';
-import getPackageMetadata from './getPackageMetadata';
 import getModularRoot from '../utils/getModularRoot';
 
 export async function getPackageEntryPoints(

packages/modular-scripts/src/buildPackage/index.ts

@@ -16,7 +16,7 @@ import * as fse from 'fs-extra';
 
 import { getLogger } from './getLogger';
 import { getPackageEntryPoints } from './getPackageEntryPoints';
-import getPackageMetadata from './getPackageMetadata';
+import getPackageMetadata from '../utils/getPackageMetadata';
 import getModularRoot from '../utils/getModularRoot';
 import { makeBundle } from './makeBundle';
 import { makeTypings } from './makeTypings';

packages/modular-scripts/src/buildPackage/makeBundle.ts

@@ -14,7 +14,7 @@ import resolve from '@rollup/plugin-node-resolve';
 
 import { getLogger } from './getLogger';
 import { getPackageEntryPoints } from './getPackageEntryPoints';
-import getPackageMetadata from './getPackageMetadata';
+import getPackageMetadata from '../utils/getPackageMetadata';
 import getModularRoot from '../utils/getModularRoot';
 
 const outputDirectory = 'dist';

packages/modular-scripts/src/buildPackage/makeTypings.ts

@@ -3,8 +3,8 @@ import * as path from 'path';
 import * as ts from 'typescript';
 import * as fse from 'fs-extra';
 import { getLogger } from './getLogger';
-import { reportTSDiagnostics } from './reportTSDiagnostics';
-import getPackageMetadata from './getPackageMetadata';
+import { reportTSDiagnostics } from '../utils/reportTSDiagnostics';
+import getPackageMetadata from '../utils/getPackageMetadata';
 
 const outputDirectory = 'dist';
 const typescriptConfigFilename = 'tsconfig.json';

packages/modular-scripts/src/cli.ts

@@ -226,6 +226,14 @@ program
     await port(relativePath);
   });
 
+program
+  .command('typecheck')
+  .description('Typechecks the entire project')
+  .action(async () => {
+    const { typecheck } = await import('./typecheck');
+    await typecheck();
+  });
+
 void startupCheck()
   .then(() => {
     return program.parseAsync(process.argv);

packages/modular-scripts/src/config/jest/index.ts

@@ -1,6 +1,7 @@
 import * as fs from 'fs-extra';
 import path from 'path';
 import chalk from 'chalk';
+import isCi from 'is-ci';
 import globby from 'globby';
 import type { Config } from '@jest/types';
 import { defaults } from 'jest-config';
@@ -195,5 +196,16 @@ export function createJestConfig(
       moduleNameMapper: mergedMapper,
     });
   }
+
+  // don't typecheck tests in CI
+  if (isCi) {
+    jestConfig.globals = {
+      'ts-jest': {
+        diagnostics: false,
+        isolatedModules: true,
+      },
+    };
+  }
+
   return jestConfig;
 }

packages/modular-scripts/src/typecheck.ts

@@ -0,0 +1,90 @@
+import isCI from 'is-ci';
+import path from 'path';
+import ts from 'typescript';
+import chalk from 'chalk';
+import getPackageMetadata from './utils/getPackageMetadata';
+import * as logger from './utils/logger';
+import getModularRoot from './utils/getModularRoot';
+
+export async function typecheck(): Promise<void> {
+  const { typescriptConfig } = await getPackageMetadata();
+
+  const { _compilerOptions, ...rest } = typescriptConfig;
+
+  const tsConfig = {
+    ...rest,
+    exclude: [
+      'node_modules',
+      'bower_components',
+      'jspm_packages',
+      'tmp',
+      '**/dist-types',
+      '**/dist-cjs',
+      '**/dist-es',
+      'dist',
+    ],
+    compilerOptions: {
+      noEmit: true,
+    },
+  };
+
+  const diagnosticHost = {
+    getCurrentDirectory: (): string => getModularRoot(),
+    getNewLine: (): string => ts.sys.newLine,
+    getCanonicalFileName: (file: string): string =>
+      ts.sys.useCaseSensitiveFileNames ? file : toFileNameLowerCase(file),
+  };
+
+  // Parse all config except for compilerOptions
+  const configParseResult = ts.parseJsonConfigFileContent(
+    tsConfig,
+    ts.sys,
+    path.dirname('tsconfig.json'),
+  );
+
+  if (configParseResult.errors.length > 0) {
+    logger.error('Failed to parse your tsconfig.json');
+    throw new Error(
+      ts.formatDiagnostics(configParseResult.errors, diagnosticHost),
+    );
+  }
+
+  const program = ts.createProgram(
+    configParseResult.fileNames,
+    configParseResult.options,
+  );
+
+  // Pulled from typescript's getCanonicalFileName logic
+  // eslint-disable-next-line  no-useless-escape
+  const fileNameLowerCaseRegExp = /[^\u0130\u0131\u00DFa-z0-9\\/:\-_\. ]+/g;
+
+  function toFileNameLowerCase(x: string) {
+    return fileNameLowerCaseRegExp.test(x)
+      ? x.replace(fileNameLowerCaseRegExp, x.toLowerCase())
+      : x;
+  }
+
+  // Does not emit files or typings but will add declaration diagnostics to our errors
+  // This will ensure that makeTypings will be successful in CI before actually attempting to build
+  const emitResult = program.emit();
+
+  const diagnostics = ts
+    .getPreEmitDiagnostics(program)
+    .concat(emitResult.diagnostics);
+
+  if (diagnostics.length) {
+    if (isCI) {
+      // formatDiagnostics will return a readable list of error messages, each with its own line
+      throw new Error(ts.formatDiagnostics(diagnostics, diagnosticHost));
+    }
+
+    // formatDiagnosticsWithColorAndContext will return a list of errors, each with its own line
+    // and provide an expanded snapshot of the line with the error
+    throw new Error(
+      ts.formatDiagnosticsWithColorAndContext(diagnostics, diagnosticHost),
+    );
+  }
+
+  // "✓ Typecheck passed"
+  logger.log(chalk.green('\u2713 Typecheck passed'));
+}