From 21efc224a4c6790b2de66f1c7af7497eb0a5f0b7 Mon Sep 17 00:00:00 2001 From: cangarugula <33293642+cangarugula@users.noreply.github.com> Date: Tue, 20 Jul 2021 08:17:54 -0400 Subject: [PATCH] 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 --- .changeset/cyan-moose-judge.md | 5 + .changeset/polite-lizards-thank.md | 5 + .github/workflows/node.js.yml | 2 +- package.json | 3 +- .../react-scripts/config/webpack.config.js | 6 +- .../react-scripts/scripts/start.js | 5 +- .../__fixtures__/typecheck/InvalidTyping.ts | 12 ++ .../src/__tests__/typecheck.test.ts | 105 ++++++++++++++++++ .../src/buildPackage/getPackageEntryPoints.ts | 2 +- .../modular-scripts/src/buildPackage/index.ts | 2 +- .../src/buildPackage/makeBundle.ts | 2 +- .../src/buildPackage/makeTypings.ts | 4 +- packages/modular-scripts/src/cli.ts | 8 ++ .../modular-scripts/src/config/jest/index.ts | 12 ++ packages/modular-scripts/src/typecheck.ts | 90 +++++++++++++++ .../getPackageMetadata.ts | 0 packages/modular-scripts/src/utils/logger.ts | 17 ++- .../reportTSDiagnostics.ts | 2 +- 18 files changed, 267 insertions(+), 15 deletions(-) create mode 100644 .changeset/cyan-moose-judge.md create mode 100644 .changeset/polite-lizards-thank.md create mode 100644 packages/modular-scripts/src/__tests__/__fixtures__/typecheck/InvalidTyping.ts create mode 100644 packages/modular-scripts/src/__tests__/typecheck.test.ts create mode 100644 packages/modular-scripts/src/typecheck.ts rename packages/modular-scripts/src/{buildPackage => utils}/getPackageMetadata.ts (100%) rename packages/modular-scripts/src/{buildPackage => utils}/reportTSDiagnostics.ts (93%) diff --git a/.changeset/cyan-moose-judge.md b/.changeset/cyan-moose-judge.md new file mode 100644 index 000000000..a856e5589 --- /dev/null +++ b/.changeset/cyan-moose-judge.md @@ -0,0 +1,5 @@ +--- +'modular-scripts': minor +--- + +Disable typechecking for modular start, test, build commands in CI environments diff --git a/.changeset/polite-lizards-thank.md b/.changeset/polite-lizards-thank.md new file mode 100644 index 000000000..1f4cef2e5 --- /dev/null +++ b/.changeset/polite-lizards-thank.md @@ -0,0 +1,5 @@ +--- +'modular-scripts': minor +--- + +Added `modular typecheck` command to programatically type check the project diff --git a/.github/workflows/node.js.yml b/.github/workflows/node.js.yml index e13d921cf..1029fbe8f 100644 --- a/.github/workflows/node.js.yml +++ b/.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 diff --git a/package.json b/package.json index dcb53036a..6ed573907 100644 --- a/package.json +++ b/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", diff --git a/packages/modular-scripts/react-scripts/config/webpack.config.js b/packages/modular-scripts/react-scripts/config/webpack.config.js index 531bfcc7b..6e6b5780d 100644 --- a/packages/modular-scripts/react-scripts/config/webpack.config.js +++ b/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, diff --git a/packages/modular-scripts/react-scripts/scripts/start.js b/packages/modular-scripts/react-scripts/scripts/start.js index 229d29d89..88996357d 100644 --- a/packages/modular-scripts/react-scripts/scripts/start.js +++ b/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, diff --git a/packages/modular-scripts/src/__tests__/__fixtures__/typecheck/InvalidTyping.ts b/packages/modular-scripts/src/__tests__/__fixtures__/typecheck/InvalidTyping.ts new file mode 100644 index 000000000..dc670d82d --- /dev/null +++ b/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'); diff --git a/packages/modular-scripts/src/__tests__/typecheck.test.ts b/packages/modular-scripts/src/__tests__/typecheck.test.ts new file mode 100644 index 000000000..7dc113c9c --- /dev/null +++ b/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'); + }); + }); +}); diff --git a/packages/modular-scripts/src/buildPackage/getPackageEntryPoints.ts b/packages/modular-scripts/src/buildPackage/getPackageEntryPoints.ts index 71661fd33..3f2e6fce8 100644 --- a/packages/modular-scripts/src/buildPackage/getPackageEntryPoints.ts +++ b/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( diff --git a/packages/modular-scripts/src/buildPackage/index.ts b/packages/modular-scripts/src/buildPackage/index.ts index f9a2faf11..c840d0377 100644 --- a/packages/modular-scripts/src/buildPackage/index.ts +++ b/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'; diff --git a/packages/modular-scripts/src/buildPackage/makeBundle.ts b/packages/modular-scripts/src/buildPackage/makeBundle.ts index a4680d459..57f16c91b 100644 --- a/packages/modular-scripts/src/buildPackage/makeBundle.ts +++ b/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'; diff --git a/packages/modular-scripts/src/buildPackage/makeTypings.ts b/packages/modular-scripts/src/buildPackage/makeTypings.ts index 8ed0df446..cd37d40c9 100644 --- a/packages/modular-scripts/src/buildPackage/makeTypings.ts +++ b/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'; diff --git a/packages/modular-scripts/src/cli.ts b/packages/modular-scripts/src/cli.ts index 6868ee6e9..6538bfb54 100755 --- a/packages/modular-scripts/src/cli.ts +++ b/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); diff --git a/packages/modular-scripts/src/config/jest/index.ts b/packages/modular-scripts/src/config/jest/index.ts index 6c9367429..721bd7147 100644 --- a/packages/modular-scripts/src/config/jest/index.ts +++ b/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; } diff --git a/packages/modular-scripts/src/typecheck.ts b/packages/modular-scripts/src/typecheck.ts new file mode 100644 index 000000000..584401eec --- /dev/null +++ b/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 { + 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')); +} diff --git a/packages/modular-scripts/src/buildPackage/getPackageMetadata.ts b/packages/modular-scripts/src/utils/getPackageMetadata.ts similarity index 100% rename from packages/modular-scripts/src/buildPackage/getPackageMetadata.ts rename to packages/modular-scripts/src/utils/getPackageMetadata.ts diff --git a/packages/modular-scripts/src/utils/logger.ts b/packages/modular-scripts/src/utils/logger.ts index a5a112092..a00804c8f 100644 --- a/packages/modular-scripts/src/utils/logger.ts +++ b/packages/modular-scripts/src/utils/logger.ts @@ -5,7 +5,7 @@ const prefix = '[modular] '; const DEBUG = process.env.MODULAR_LOGGER_DEBUG; const SILENT = process.env.MODULAR_LOGGER_MUTE; -function print(x: string) { +function printStdErr(x: string) { if (SILENT) { return; } @@ -13,14 +13,23 @@ function print(x: string) { process.stderr.write(chalk.dim(prefix) + l + '\n'); }); } + +function printStdOut(x: string) { + if (SILENT) { + return; + } + x.split('\n').forEach((l) => { + process.stdout.write(chalk.dim(prefix) + l + '\n'); + }); +} export function log(...x: string[]): void { - print(x.join(' ')); + printStdOut(x.join(' ')); } export function warn(...x: string[]): void { - print(chalk.yellow(x.join(' '))); + printStdErr(chalk.yellow(x.join(' '))); } export function error(...x: string[]): void { - print(chalk.red(x.join(' '))); + printStdErr(chalk.red(x.join(' '))); } export function debug(...x: string[]): void { diff --git a/packages/modular-scripts/src/buildPackage/reportTSDiagnostics.ts b/packages/modular-scripts/src/utils/reportTSDiagnostics.ts similarity index 93% rename from packages/modular-scripts/src/buildPackage/reportTSDiagnostics.ts rename to packages/modular-scripts/src/utils/reportTSDiagnostics.ts index b93883a50..537f67f43 100644 --- a/packages/modular-scripts/src/buildPackage/reportTSDiagnostics.ts +++ b/packages/modular-scripts/src/utils/reportTSDiagnostics.ts @@ -1,5 +1,5 @@ import * as ts from 'typescript'; -import { getLogger } from './getLogger'; +import { getLogger } from '../buildPackage/getLogger'; // from https://github.com/Microsoft/TypeScript/issues/6387 // a helper to output a readable message from a ts diagnostics object