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': minor
---

Added `modular typecheck` command to programatically type check the project
2 changes: 1 addition & 1 deletion .github/workflows/node.js.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
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,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,
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
Original file line number Diff line number Diff line change
@@ -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');
105 changes: 105 additions & 0 deletions packages/modular-scripts/src/__tests__/typecheck.test.ts
Original file line number Diff line number Diff line change
@@ -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');
});
});
});
Original file line number Diff line number Diff line change
@@ -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(
Expand Down
2 changes: 1 addition & 1 deletion packages/modular-scripts/src/buildPackage/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
2 changes: 1 addition & 1 deletion packages/modular-scripts/src/buildPackage/makeBundle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
4 changes: 2 additions & 2 deletions packages/modular-scripts/src/buildPackage/makeTypings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
8 changes: 8 additions & 0 deletions packages/modular-scripts/src/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
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;
}
90 changes: 90 additions & 0 deletions packages/modular-scripts/src/typecheck.ts
Original file line number Diff line number Diff line change
@@ -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();
LukeSheard marked this conversation as resolved.
Show resolved Hide resolved

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));
cangarugula marked this conversation as resolved.
Show resolved Hide resolved
}

// 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'));
}
Loading