Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Close #605: Disable type check for test, start, build commands in CI #606

Merged
merged 16 commits into from
Jul 20, 2021
Merged
5 changes: 5 additions & 0 deletions .changeset/cyan-moose-judge.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'modular-scripts': minor
---

Disable typechecking for modular start, test, build commands in CI environments
5 changes: 5 additions & 0 deletions .changeset/polite-lizards-thank.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'modular-scripts': patch
---

Added `modular typecheck` command to programatically type check the project
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -634,8 +634,9 @@ 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 for non CI envs
LukeSheard marked this conversation as resolved.
Show resolved Hide resolved
useTypeScript &&
!isCI &&
new ForkTsCheckerWebpackPlugin({
typescript: resolve.sync('typescript', {
basedir: paths.appNodeModules,
Expand Down
5 changes: 4 additions & 1 deletion packages/modular-scripts/react-scripts/scripts/start.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down
16 changes: 16 additions & 0 deletions packages/modular-scripts/src/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,22 @@ program
await port(relativePath);
});

interface TypecheckOptions {
silent: boolean;
}

program
.command('typecheck')
.option(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the use case for this? I would avoid adding any unnecessary APIs to this until we've got a better idea of when we'll need them.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added this as a way to silence error messages and provide a green/red light on typechecking. I'm impartial so I will remove for simplicity's sake.

'--silent',
'silences the error messages gathered during the type check',
)
.description('Typechecks the entire project')
.action(async (options: TypecheckOptions) => {
const { typecheck } = await import('./typecheck');
await typecheck(options.silent);
});

void startupCheck()
.then(() => {
return program.parseAsync(process.argv);
Expand Down
12 changes: 12 additions & 0 deletions packages/modular-scripts/src/config/jest/index.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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,
LukeSheard marked this conversation as resolved.
Show resolved Hide resolved
},
};
}

return jestConfig;
}
65 changes: 65 additions & 0 deletions packages/modular-scripts/src/typecheck.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
import isCI from 'is-ci';
import path from 'path';
import ts from 'typescript';
import chalk from 'chalk';
import getPackageMetadata from './buildPackage/getPackageMetadata';
import { reportTSDiagnostics } from './buildPackage/reportTSDiagnostics';
LukeSheard marked this conversation as resolved.
Show resolved Hide resolved
import * as logger from './utils/logger';

export async function typecheck(silent: boolean): Promise<void> {
const { typescriptConfig } = await getPackageMetadata();
LukeSheard marked this conversation as resolved.
Show resolved Hide resolved

const { _compilerOptions, ...tsConfig } = typescriptConfig;

// Parse all config except for compilerOptions
const configParseResult = ts.parseJsonConfigFileContent(
tsConfig,
ts.sys,
path.dirname('tsconfig.json'),
);

if (configParseResult.errors.length > 0) {
reportTSDiagnostics(':root', configParseResult.errors);
throw new Error('Could not parse Typescript configuration');
}

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;
}

const diagnostics = ts.getPreEmitDiagnostics(program) as ts.Diagnostic[];

const diagnosticHost = {
getCurrentDirectory: (): string => ts.sys.getCurrentDirectory(),
LukeSheard marked this conversation as resolved.
Show resolved Hide resolved
getNewLine: (): string => ts.sys.newLine,
getCanonicalFileName: (file: string): string =>
ts.sys.useCaseSensitiveFileNames ? file : toFileNameLowerCase(file),
};

if (diagnostics.length) {
if (silent) {
throw new Error('\u0078 Typecheck did not pass');
LukeSheard marked this conversation as resolved.
Show resolved Hide resolved
}

if (isCI) {
throw new Error(ts.formatDiagnostics(diagnostics, diagnosticHost));
cangarugula marked this conversation as resolved.
Show resolved Hide resolved
}

throw new Error(
ts.formatDiagnosticsWithColorAndContext(diagnostics, diagnosticHost),
);
}

logger.log(chalk.green('\u2713 Typecheck passed'));
}