Speed up TypeScript projects (facebook#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 facebook#5820
amplitude · Feb 13, 2020 · 891d93c · 891d93c
1 parent e176921
commit 891d93c
Show file tree

Hide file tree

Showing 11 changed files with 193 additions and 43 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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');
@@ -624,17 +624,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',
@@ -645,7 +638,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
@@ -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
@@ -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
diff --git a/test/fixtures/typescript-typecheck/index.test.js 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
@@ -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
@@ -0,0 +1,13 @@
+import * as React from 'react';
+
+class App extends React.Component {
+  render() {
+    return <div>{format(123)}</div>;
+  }
+}
+
+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
@@ -0,0 +1,5 @@
+import * as React from 'react';
+import * as ReactDOM from 'react-dom';
+import App from './App';
+
+ReactDOM.render(<App />, document.getElementById('root'));