Revert "Revert "Speed up TypeScript projects (#5903)""

This reverts commit 544a594.
facebook · Feb 12, 2019 · 9581519 · 9581519
1 parent c6eca6e
commit 9581519
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');
@@ -625,17 +625,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',
@@ -646,7 +639,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.4",
     "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'));