From 5ce09db2b353f48138060a1e0ef9947c6c9c6556 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1=C5=A1=20Szabo?= Date: Fri, 8 Feb 2019 04:34:52 +0100 Subject: [PATCH] Speed up TypeScript projects (#5903) As a lot of [people](https://hackernoon.com/why-i-no-longer-use-typescript-with-react-and-why-you-shouldnt-either-e744d27452b4) is complaining about TypeScript performance in CRA, I decided to enable `async` mode in TypeScript checker. These changes basically brings the JS compilation times to TS projects. So, recompilation took less than 1 second instead of 3 seconds in medium size project. The problem with async mode is that type-errors are reported after Webpack ends up recompilation as TypeScript could be slower than Babel. PR allows to emit files compiled by Babel immediately and then wait for TS and show type errors in terminal later. Also, if there was no compilation errors and any type error occurs, we trigger a hot-reload with new errors to show error overlay in browser. Also, I wanted to start a discussion about `skipLibCheck: false` option in default `tsconfig.json`. This makes recompilations really slow and we should consider to set it to `true` or at least give users a big warning to let them know that it could be really slow. The following video is showing the updated workflow with a forced 2.5 second delay for type-check to give you an idea how it works. ![nov-26-2018 15-47-01](https://user-images.githubusercontent.com/5549148/49021284-9446fe80-f192-11e8-952b-8f83d77d5fbc.gif) I'm pretty sure that PR needs some polishing and improvements but it should works as it is. Especially a "hack" with reloading the browser after type-check looks ugly to me. cc @brunolemos as he is an initiator of an original TypeScript PR. Should fix https://github.com/facebook/create-react-app/issues/5820 --- .../react-dev-utils/WebpackDevServerUtils.js | 116 +++++++++++++++--- .../react-dev-utils/typescriptFormatter.js | 7 +- .../react-dev-utils/webpackHotDevClient.js | 18 +-- .../react-scripts/config/webpack.config.js | 16 +-- packages/react-scripts/package.json | 2 +- packages/react-scripts/scripts/start.js | 17 ++- .../typescript-typecheck/.disable-pnp | 0 .../typescript-typecheck/index.test.js | 33 +++++ .../typescript-typecheck/package.json | 9 ++ .../fixtures/typescript-typecheck/src/App.tsx | 13 ++ .../typescript-typecheck/src/index.tsx | 5 + 11 files changed, 193 insertions(+), 43 deletions(-) create mode 100644 test/fixtures/typescript-typecheck/.disable-pnp create mode 100644 test/fixtures/typescript-typecheck/index.test.js create mode 100644 test/fixtures/typescript-typecheck/package.json create mode 100644 test/fixtures/typescript-typecheck/src/App.tsx create mode 100644 test/fixtures/typescript-typecheck/src/index.tsx diff --git a/packages/react-dev-utils/WebpackDevServerUtils.js b/packages/react-dev-utils/WebpackDevServerUtils.js index 67a1a45e3a6..67f6d4164f7 100644 --- a/packages/react-dev-utils/WebpackDevServerUtils.js +++ b/packages/react-dev-utils/WebpackDevServerUtils.js @@ -17,22 +17,10 @@ const inquirer = require('inquirer'); const clearConsole = require('./clearConsole'); const formatWebpackMessages = require('./formatWebpackMessages'); const getProcessForPort = require('./getProcessForPort'); +const typescriptFormatter = require('./typescriptFormatter'); +const forkTsCheckerWebpackPlugin = require('fork-ts-checker-webpack-plugin'); const isInteractive = process.stdout.isTTY; -let handleCompile; - -// You can safely remove this after ejecting. -// We only use this block for testing of Create React App itself: -const isSmokeTest = process.argv.some(arg => arg.indexOf('--smoke-test') > -1); -if (isSmokeTest) { - handleCompile = (err, stats) => { - if (err || stats.hasErrors() || stats.hasWarnings()) { - process.exit(1); - } else { - process.exit(0); - } - }; -} function prepareUrls(protocol, host, port) { const formatUrl = hostname => @@ -113,12 +101,20 @@ function printInstructions(appName, urls, useYarn) { console.log(); } -function createCompiler(webpack, config, appName, urls, useYarn) { +function createCompiler( + webpack, + config, + appName, + urls, + useYarn, + useTypeScript, + devSocket +) { // "Compiler" is a low-level interface to Webpack. // It lets us listen to some events and provide our own custom messages. let compiler; try { - compiler = webpack(config, handleCompile); + compiler = webpack(config); } catch (err) { console.log(chalk.red('Failed to compile.')); console.log(); @@ -139,10 +135,35 @@ function createCompiler(webpack, config, appName, urls, useYarn) { }); let isFirstCompile = true; + let tsMessagesPromise; + let tsMessagesResolver; + + if (useTypeScript) { + compiler.hooks.beforeCompile.tap('beforeCompile', () => { + tsMessagesPromise = new Promise(resolve => { + tsMessagesResolver = msgs => resolve(msgs); + }); + }); + + forkTsCheckerWebpackPlugin + .getCompilerHooks(compiler) + .receive.tap('afterTypeScriptCheck', (diagnostics, lints) => { + const allMsgs = [...diagnostics, ...lints]; + const format = message => + `${message.file}\n${typescriptFormatter(message, true)}`; + + tsMessagesResolver({ + errors: allMsgs.filter(msg => msg.severity === 'error').map(format), + warnings: allMsgs + .filter(msg => msg.severity === 'warning') + .map(format), + }); + }); + } // "done" event fires when Webpack has finished recompiling the bundle. // Whether or not you have warnings or errors, you will get this event. - compiler.hooks.done.tap('done', stats => { + compiler.hooks.done.tap('done', async stats => { if (isInteractive) { clearConsole(); } @@ -152,9 +173,43 @@ function createCompiler(webpack, config, appName, urls, useYarn) { // them in a readable focused way. // We only construct the warnings and errors for speed: // https://github.com/facebook/create-react-app/issues/4492#issuecomment-421959548 - const messages = formatWebpackMessages( - stats.toJson({ all: false, warnings: true, errors: true }) - ); + const statsData = stats.toJson({ + all: false, + warnings: true, + errors: true, + }); + + if (useTypeScript && statsData.errors.length === 0) { + const delayedMsg = setTimeout(() => { + console.log( + chalk.yellow( + 'Files successfully emitted, waiting for typecheck results...' + ) + ); + }, 100); + + const messages = await tsMessagesPromise; + clearTimeout(delayedMsg); + statsData.errors.push(...messages.errors); + statsData.warnings.push(...messages.warnings); + + // Push errors and warnings into compilation result + // to show them after page refresh triggered by user. + stats.compilation.errors.push(...messages.errors); + stats.compilation.warnings.push(...messages.warnings); + + if (messages.errors.length > 0) { + devSocket.errors(messages.errors); + } else if (messages.warnings.length > 0) { + devSocket.warnings(messages.warnings); + } + + if (isInteractive) { + clearConsole(); + } + } + + const messages = formatWebpackMessages(statsData); const isSuccessful = !messages.errors.length && !messages.warnings.length; if (isSuccessful) { console.log(chalk.green('Compiled successfully!')); @@ -194,6 +249,27 @@ function createCompiler(webpack, config, appName, urls, useYarn) { ); } }); + + // You can safely remove this after ejecting. + // We only use this block for testing of Create React App itself: + const isSmokeTest = process.argv.some( + arg => arg.indexOf('--smoke-test') > -1 + ); + if (isSmokeTest) { + compiler.hooks.failed.tap('smokeTest', async () => { + await tsMessagesPromise; + process.exit(1); + }); + compiler.hooks.done.tap('smokeTest', async stats => { + await tsMessagesPromise; + if (stats.hasErrors() || stats.hasWarnings()) { + process.exit(1); + } else { + process.exit(0); + } + }); + } + return compiler; } diff --git a/packages/react-dev-utils/typescriptFormatter.js b/packages/react-dev-utils/typescriptFormatter.js index 2d011fc7e71..3a33b37a427 100644 --- a/packages/react-dev-utils/typescriptFormatter.js +++ b/packages/react-dev-utils/typescriptFormatter.js @@ -45,12 +45,15 @@ function formatter(message, useColors) { } const severity = hasGetters ? message.getSeverity() : message.severity; + const types = { diagnostic: 'TypeScript', lint: 'TSLint' }; return [ - messageColor.bold(`Type ${severity.toLowerCase()}: `) + + messageColor.bold(`${types[message.type]} ${severity.toLowerCase()}: `) + (hasGetters ? message.getContent() : message.content) + ' ' + - messageColor.underline(`TS${message.code}`), + messageColor.underline( + (message.type === 'lint' ? 'Rule: ' : 'TS') + message.code + ), '', frame, ].join(os.EOL); diff --git a/packages/react-dev-utils/webpackHotDevClient.js b/packages/react-dev-utils/webpackHotDevClient.js index cc7dc61684b..050ce2a6351 100644 --- a/packages/react-dev-utils/webpackHotDevClient.js +++ b/packages/react-dev-utils/webpackHotDevClient.js @@ -106,7 +106,7 @@ function handleSuccess() { tryApplyUpdates(function onHotUpdateSuccess() { // Only dismiss it when we're sure it's a hot update. // Otherwise it would flicker right before the reload. - ErrorOverlay.dismissBuildError(); + tryDismissErrorOverlay(); }); } } @@ -140,19 +140,15 @@ function handleWarnings(warnings) { } } + printWarnings(); + // Attempt to apply hot updates or reload. if (isHotUpdate) { tryApplyUpdates(function onSuccessfulHotUpdate() { - // Only print warnings if we aren't refreshing the page. - // Otherwise they'll disappear right away anyway. - printWarnings(); // Only dismiss it when we're sure it's a hot update. // Otherwise it would flicker right before the reload. - ErrorOverlay.dismissBuildError(); + tryDismissErrorOverlay(); }); - } else { - // Print initial warnings immediately. - printWarnings(); } } @@ -183,6 +179,12 @@ function handleErrors(errors) { // We will reload on next success instead. } +function tryDismissErrorOverlay() { + if (!hasCompileErrors) { + ErrorOverlay.dismissBuildError(); + } +} + // There is a newer version of the code available. function handleAvailableHash(hash) { // Update last known compilation hash. diff --git a/packages/react-scripts/config/webpack.config.js b/packages/react-scripts/config/webpack.config.js index 942c22a2cc7..1e304d76046 100644 --- a/packages/react-scripts/config/webpack.config.js +++ b/packages/react-scripts/config/webpack.config.js @@ -29,7 +29,7 @@ const getCSSModuleLocalIdent = require('react-dev-utils/getCSSModuleLocalIdent') const paths = require('./paths'); const getClientEnvironment = require('./env'); const ModuleNotFoundPlugin = require('react-dev-utils/ModuleNotFoundPlugin'); -const ForkTsCheckerWebpackPlugin = require('fork-ts-checker-webpack-plugin-alt'); +const ForkTsCheckerWebpackPlugin = require('fork-ts-checker-webpack-plugin'); const typescriptFormatter = require('react-dev-utils/typescriptFormatter'); // @remove-on-eject-begin const getCacheIdentifier = require('react-dev-utils/getCacheIdentifier'); @@ -617,17 +617,10 @@ module.exports = function(webpackEnv) { typescript: resolve.sync('typescript', { basedir: paths.appNodeModules, }), - async: false, + async: isEnvDevelopment, + useTypescriptIncrementalApi: true, checkSyntacticErrors: true, tsconfig: paths.appTsConfig, - compilerOptions: { - module: 'esnext', - moduleResolution: 'node', - resolveJsonModule: true, - isolatedModules: true, - noEmit: true, - jsx: 'preserve', - }, reportFiles: [ '**', '!**/*.json', @@ -638,7 +631,8 @@ module.exports = function(webpackEnv) { ], watch: paths.appSrc, silent: true, - formatter: typescriptFormatter, + // The formatter is invoked directly in WebpackDevServerUtils during development + formatter: isEnvProduction ? typescriptFormatter : undefined, }), ].filter(Boolean), // Some libraries import Node modules but don't use them in the browser. diff --git a/packages/react-scripts/package.json b/packages/react-scripts/package.json index a9ed5e4c835..d393b73190b 100644 --- a/packages/react-scripts/package.json +++ b/packages/react-scripts/package.json @@ -45,7 +45,7 @@ "eslint-plugin-jsx-a11y": "6.1.2", "eslint-plugin-react": "7.12.3", "file-loader": "2.0.0", - "fork-ts-checker-webpack-plugin-alt": "0.4.14", + "fork-ts-checker-webpack-plugin": "1.0.0-alpha.6", "fs-extra": "7.0.1", "html-webpack-plugin": "4.0.0-alpha.2", "identity-obj-proxy": "3.0.0", diff --git a/packages/react-scripts/scripts/start.js b/packages/react-scripts/scripts/start.js index 852e6b8fb44..6ad020c9d46 100644 --- a/packages/react-scripts/scripts/start.js +++ b/packages/react-scripts/scripts/start.js @@ -94,9 +94,24 @@ checkBrowsers(paths.appPath, isInteractive) const config = configFactory('development'); const protocol = process.env.HTTPS === 'true' ? 'https' : 'http'; const appName = require(paths.appPackageJson).name; + const useTypeScript = fs.existsSync(paths.appTsConfig); const urls = prepareUrls(protocol, HOST, port); + const devSocket = { + warnings: warnings => + devServer.sockWrite(devServer.sockets, 'warnings', warnings), + errors: errors => + devServer.sockWrite(devServer.sockets, 'errors', errors), + }; // Create a webpack compiler that is configured with custom messages. - const compiler = createCompiler(webpack, config, appName, urls, useYarn); + const compiler = createCompiler( + webpack, + config, + appName, + urls, + useYarn, + useTypeScript, + devSocket + ); // Load proxy config const proxySetting = require(paths.appPackageJson).proxy; const proxyConfig = prepareProxy(proxySetting, paths.appPublic); diff --git a/test/fixtures/typescript-typecheck/.disable-pnp b/test/fixtures/typescript-typecheck/.disable-pnp new file mode 100644 index 00000000000..e69de29bb2d diff --git a/test/fixtures/typescript-typecheck/index.test.js b/test/fixtures/typescript-typecheck/index.test.js new file mode 100644 index 00000000000..c4978c735e9 --- /dev/null +++ b/test/fixtures/typescript-typecheck/index.test.js @@ -0,0 +1,33 @@ +const testSetup = require('../__shared__/test-setup'); +const puppeteer = require('puppeteer'); + +const expectedErrorMsg = `Argument of type '123' is not assignable to parameter of type 'string'`; + +test('shows error overlay in browser', async () => { + const { port, done } = await testSetup.scripts.start(); + + const browser = await puppeteer.launch({ headless: true }); + try { + const page = await browser.newPage(); + await page.goto(`http://localhost:${port}/`); + await page.waitForSelector('iframe', { timeout: 5000 }); + const overlayMsg = await page.evaluate(() => { + const overlay = document.querySelector('iframe').contentWindow; + return overlay.document.body.innerHTML; + }); + expect(overlayMsg).toContain(expectedErrorMsg); + } finally { + browser.close(); + done(); + } +}); + +test('shows error in console (dev mode)', async () => { + const { stderr } = await testSetup.scripts.start({ smoke: true }); + expect(stderr).toContain(expectedErrorMsg); +}); + +test('shows error in console (prod mode)', async () => { + const { stderr } = await testSetup.scripts.build(); + expect(stderr).toContain(expectedErrorMsg); +}); diff --git a/test/fixtures/typescript-typecheck/package.json b/test/fixtures/typescript-typecheck/package.json new file mode 100644 index 00000000000..a6c00267c54 --- /dev/null +++ b/test/fixtures/typescript-typecheck/package.json @@ -0,0 +1,9 @@ +{ + "dependencies": { + "@types/react": "*", + "@types/react-dom": "*", + "react": "*", + "react-dom": "*", + "typescript": "3.1.3" + } +} diff --git a/test/fixtures/typescript-typecheck/src/App.tsx b/test/fixtures/typescript-typecheck/src/App.tsx new file mode 100644 index 00000000000..75924d78b0e --- /dev/null +++ b/test/fixtures/typescript-typecheck/src/App.tsx @@ -0,0 +1,13 @@ +import * as React from 'react'; + +class App extends React.Component { + render() { + return
{format(123)}
; + } +} + +function format(value: string) { + return value; +} + +export default App; diff --git a/test/fixtures/typescript-typecheck/src/index.tsx b/test/fixtures/typescript-typecheck/src/index.tsx new file mode 100644 index 00000000000..bea6ed52237 --- /dev/null +++ b/test/fixtures/typescript-typecheck/src/index.tsx @@ -0,0 +1,5 @@ +import * as React from 'react'; +import * as ReactDOM from 'react-dom'; +import App from './App'; + +ReactDOM.render(, document.getElementById('root'));