Skip to content

Commit 5ce09db

Browse files
deftomatianschmitz
authored andcommitted
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
1 parent 1deb811 commit 5ce09db

File tree

11 files changed

+193
-43
lines changed

11 files changed

+193
-43
lines changed

packages/react-dev-utils/WebpackDevServerUtils.js

+96-20
Original file line numberDiff line numberDiff line change
@@ -17,22 +17,10 @@ const inquirer = require('inquirer');
1717
const clearConsole = require('./clearConsole');
1818
const formatWebpackMessages = require('./formatWebpackMessages');
1919
const getProcessForPort = require('./getProcessForPort');
20+
const typescriptFormatter = require('./typescriptFormatter');
21+
const forkTsCheckerWebpackPlugin = require('fork-ts-checker-webpack-plugin');
2022

2123
const isInteractive = process.stdout.isTTY;
22-
let handleCompile;
23-
24-
// You can safely remove this after ejecting.
25-
// We only use this block for testing of Create React App itself:
26-
const isSmokeTest = process.argv.some(arg => arg.indexOf('--smoke-test') > -1);
27-
if (isSmokeTest) {
28-
handleCompile = (err, stats) => {
29-
if (err || stats.hasErrors() || stats.hasWarnings()) {
30-
process.exit(1);
31-
} else {
32-
process.exit(0);
33-
}
34-
};
35-
}
3624

3725
function prepareUrls(protocol, host, port) {
3826
const formatUrl = hostname =>
@@ -113,12 +101,20 @@ function printInstructions(appName, urls, useYarn) {
113101
console.log();
114102
}
115103

116-
function createCompiler(webpack, config, appName, urls, useYarn) {
104+
function createCompiler(
105+
webpack,
106+
config,
107+
appName,
108+
urls,
109+
useYarn,
110+
useTypeScript,
111+
devSocket
112+
) {
117113
// "Compiler" is a low-level interface to Webpack.
118114
// It lets us listen to some events and provide our own custom messages.
119115
let compiler;
120116
try {
121-
compiler = webpack(config, handleCompile);
117+
compiler = webpack(config);
122118
} catch (err) {
123119
console.log(chalk.red('Failed to compile.'));
124120
console.log();
@@ -139,10 +135,35 @@ function createCompiler(webpack, config, appName, urls, useYarn) {
139135
});
140136

141137
let isFirstCompile = true;
138+
let tsMessagesPromise;
139+
let tsMessagesResolver;
140+
141+
if (useTypeScript) {
142+
compiler.hooks.beforeCompile.tap('beforeCompile', () => {
143+
tsMessagesPromise = new Promise(resolve => {
144+
tsMessagesResolver = msgs => resolve(msgs);
145+
});
146+
});
147+
148+
forkTsCheckerWebpackPlugin
149+
.getCompilerHooks(compiler)
150+
.receive.tap('afterTypeScriptCheck', (diagnostics, lints) => {
151+
const allMsgs = [...diagnostics, ...lints];
152+
const format = message =>
153+
`${message.file}\n${typescriptFormatter(message, true)}`;
154+
155+
tsMessagesResolver({
156+
errors: allMsgs.filter(msg => msg.severity === 'error').map(format),
157+
warnings: allMsgs
158+
.filter(msg => msg.severity === 'warning')
159+
.map(format),
160+
});
161+
});
162+
}
142163

143164
// "done" event fires when Webpack has finished recompiling the bundle.
144165
// Whether or not you have warnings or errors, you will get this event.
145-
compiler.hooks.done.tap('done', stats => {
166+
compiler.hooks.done.tap('done', async stats => {
146167
if (isInteractive) {
147168
clearConsole();
148169
}
@@ -152,9 +173,43 @@ function createCompiler(webpack, config, appName, urls, useYarn) {
152173
// them in a readable focused way.
153174
// We only construct the warnings and errors for speed:
154175
// https://github.com/facebook/create-react-app/issues/4492#issuecomment-421959548
155-
const messages = formatWebpackMessages(
156-
stats.toJson({ all: false, warnings: true, errors: true })
157-
);
176+
const statsData = stats.toJson({
177+
all: false,
178+
warnings: true,
179+
errors: true,
180+
});
181+
182+
if (useTypeScript && statsData.errors.length === 0) {
183+
const delayedMsg = setTimeout(() => {
184+
console.log(
185+
chalk.yellow(
186+
'Files successfully emitted, waiting for typecheck results...'
187+
)
188+
);
189+
}, 100);
190+
191+
const messages = await tsMessagesPromise;
192+
clearTimeout(delayedMsg);
193+
statsData.errors.push(...messages.errors);
194+
statsData.warnings.push(...messages.warnings);
195+
196+
// Push errors and warnings into compilation result
197+
// to show them after page refresh triggered by user.
198+
stats.compilation.errors.push(...messages.errors);
199+
stats.compilation.warnings.push(...messages.warnings);
200+
201+
if (messages.errors.length > 0) {
202+
devSocket.errors(messages.errors);
203+
} else if (messages.warnings.length > 0) {
204+
devSocket.warnings(messages.warnings);
205+
}
206+
207+
if (isInteractive) {
208+
clearConsole();
209+
}
210+
}
211+
212+
const messages = formatWebpackMessages(statsData);
158213
const isSuccessful = !messages.errors.length && !messages.warnings.length;
159214
if (isSuccessful) {
160215
console.log(chalk.green('Compiled successfully!'));
@@ -194,6 +249,27 @@ function createCompiler(webpack, config, appName, urls, useYarn) {
194249
);
195250
}
196251
});
252+
253+
// You can safely remove this after ejecting.
254+
// We only use this block for testing of Create React App itself:
255+
const isSmokeTest = process.argv.some(
256+
arg => arg.indexOf('--smoke-test') > -1
257+
);
258+
if (isSmokeTest) {
259+
compiler.hooks.failed.tap('smokeTest', async () => {
260+
await tsMessagesPromise;
261+
process.exit(1);
262+
});
263+
compiler.hooks.done.tap('smokeTest', async stats => {
264+
await tsMessagesPromise;
265+
if (stats.hasErrors() || stats.hasWarnings()) {
266+
process.exit(1);
267+
} else {
268+
process.exit(0);
269+
}
270+
});
271+
}
272+
197273
return compiler;
198274
}
199275

packages/react-dev-utils/typescriptFormatter.js

+5-2
Original file line numberDiff line numberDiff line change
@@ -45,12 +45,15 @@ function formatter(message, useColors) {
4545
}
4646

4747
const severity = hasGetters ? message.getSeverity() : message.severity;
48+
const types = { diagnostic: 'TypeScript', lint: 'TSLint' };
4849

4950
return [
50-
messageColor.bold(`Type ${severity.toLowerCase()}: `) +
51+
messageColor.bold(`${types[message.type]} ${severity.toLowerCase()}: `) +
5152
(hasGetters ? message.getContent() : message.content) +
5253
' ' +
53-
messageColor.underline(`TS${message.code}`),
54+
messageColor.underline(
55+
(message.type === 'lint' ? 'Rule: ' : 'TS') + message.code
56+
),
5457
'',
5558
frame,
5659
].join(os.EOL);

packages/react-dev-utils/webpackHotDevClient.js

+10-8
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ function handleSuccess() {
106106
tryApplyUpdates(function onHotUpdateSuccess() {
107107
// Only dismiss it when we're sure it's a hot update.
108108
// Otherwise it would flicker right before the reload.
109-
ErrorOverlay.dismissBuildError();
109+
tryDismissErrorOverlay();
110110
});
111111
}
112112
}
@@ -140,19 +140,15 @@ function handleWarnings(warnings) {
140140
}
141141
}
142142

143+
printWarnings();
144+
143145
// Attempt to apply hot updates or reload.
144146
if (isHotUpdate) {
145147
tryApplyUpdates(function onSuccessfulHotUpdate() {
146-
// Only print warnings if we aren't refreshing the page.
147-
// Otherwise they'll disappear right away anyway.
148-
printWarnings();
149148
// Only dismiss it when we're sure it's a hot update.
150149
// Otherwise it would flicker right before the reload.
151-
ErrorOverlay.dismissBuildError();
150+
tryDismissErrorOverlay();
152151
});
153-
} else {
154-
// Print initial warnings immediately.
155-
printWarnings();
156152
}
157153
}
158154

@@ -183,6 +179,12 @@ function handleErrors(errors) {
183179
// We will reload on next success instead.
184180
}
185181

182+
function tryDismissErrorOverlay() {
183+
if (!hasCompileErrors) {
184+
ErrorOverlay.dismissBuildError();
185+
}
186+
}
187+
186188
// There is a newer version of the code available.
187189
function handleAvailableHash(hash) {
188190
// Update last known compilation hash.

packages/react-scripts/config/webpack.config.js

+5-11
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ const getCSSModuleLocalIdent = require('react-dev-utils/getCSSModuleLocalIdent')
2929
const paths = require('./paths');
3030
const getClientEnvironment = require('./env');
3131
const ModuleNotFoundPlugin = require('react-dev-utils/ModuleNotFoundPlugin');
32-
const ForkTsCheckerWebpackPlugin = require('fork-ts-checker-webpack-plugin-alt');
32+
const ForkTsCheckerWebpackPlugin = require('fork-ts-checker-webpack-plugin');
3333
const typescriptFormatter = require('react-dev-utils/typescriptFormatter');
3434
// @remove-on-eject-begin
3535
const getCacheIdentifier = require('react-dev-utils/getCacheIdentifier');
@@ -617,17 +617,10 @@ module.exports = function(webpackEnv) {
617617
typescript: resolve.sync('typescript', {
618618
basedir: paths.appNodeModules,
619619
}),
620-
async: false,
620+
async: isEnvDevelopment,
621+
useTypescriptIncrementalApi: true,
621622
checkSyntacticErrors: true,
622623
tsconfig: paths.appTsConfig,
623-
compilerOptions: {
624-
module: 'esnext',
625-
moduleResolution: 'node',
626-
resolveJsonModule: true,
627-
isolatedModules: true,
628-
noEmit: true,
629-
jsx: 'preserve',
630-
},
631624
reportFiles: [
632625
'**',
633626
'!**/*.json',
@@ -638,7 +631,8 @@ module.exports = function(webpackEnv) {
638631
],
639632
watch: paths.appSrc,
640633
silent: true,
641-
formatter: typescriptFormatter,
634+
// The formatter is invoked directly in WebpackDevServerUtils during development
635+
formatter: isEnvProduction ? typescriptFormatter : undefined,
642636
}),
643637
].filter(Boolean),
644638
// Some libraries import Node modules but don't use them in the browser.

packages/react-scripts/package.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@
4545
"eslint-plugin-jsx-a11y": "6.1.2",
4646
"eslint-plugin-react": "7.12.3",
4747
"file-loader": "2.0.0",
48-
"fork-ts-checker-webpack-plugin-alt": "0.4.14",
48+
"fork-ts-checker-webpack-plugin": "1.0.0-alpha.6",
4949
"fs-extra": "7.0.1",
5050
"html-webpack-plugin": "4.0.0-alpha.2",
5151
"identity-obj-proxy": "3.0.0",

packages/react-scripts/scripts/start.js

+16-1
Original file line numberDiff line numberDiff line change
@@ -94,9 +94,24 @@ checkBrowsers(paths.appPath, isInteractive)
9494
const config = configFactory('development');
9595
const protocol = process.env.HTTPS === 'true' ? 'https' : 'http';
9696
const appName = require(paths.appPackageJson).name;
97+
const useTypeScript = fs.existsSync(paths.appTsConfig);
9798
const urls = prepareUrls(protocol, HOST, port);
99+
const devSocket = {
100+
warnings: warnings =>
101+
devServer.sockWrite(devServer.sockets, 'warnings', warnings),
102+
errors: errors =>
103+
devServer.sockWrite(devServer.sockets, 'errors', errors),
104+
};
98105
// Create a webpack compiler that is configured with custom messages.
99-
const compiler = createCompiler(webpack, config, appName, urls, useYarn);
106+
const compiler = createCompiler(
107+
webpack,
108+
config,
109+
appName,
110+
urls,
111+
useYarn,
112+
useTypeScript,
113+
devSocket
114+
);
100115
// Load proxy config
101116
const proxySetting = require(paths.appPackageJson).proxy;
102117
const proxyConfig = prepareProxy(proxySetting, paths.appPublic);

test/fixtures/typescript-typecheck/.disable-pnp

Whitespace-only changes.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
const testSetup = require('../__shared__/test-setup');
2+
const puppeteer = require('puppeteer');
3+
4+
const expectedErrorMsg = `Argument of type '123' is not assignable to parameter of type 'string'`;
5+
6+
test('shows error overlay in browser', async () => {
7+
const { port, done } = await testSetup.scripts.start();
8+
9+
const browser = await puppeteer.launch({ headless: true });
10+
try {
11+
const page = await browser.newPage();
12+
await page.goto(`http://localhost:${port}/`);
13+
await page.waitForSelector('iframe', { timeout: 5000 });
14+
const overlayMsg = await page.evaluate(() => {
15+
const overlay = document.querySelector('iframe').contentWindow;
16+
return overlay.document.body.innerHTML;
17+
});
18+
expect(overlayMsg).toContain(expectedErrorMsg);
19+
} finally {
20+
browser.close();
21+
done();
22+
}
23+
});
24+
25+
test('shows error in console (dev mode)', async () => {
26+
const { stderr } = await testSetup.scripts.start({ smoke: true });
27+
expect(stderr).toContain(expectedErrorMsg);
28+
});
29+
30+
test('shows error in console (prod mode)', async () => {
31+
const { stderr } = await testSetup.scripts.build();
32+
expect(stderr).toContain(expectedErrorMsg);
33+
});
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
{
2+
"dependencies": {
3+
"@types/react": "*",
4+
"@types/react-dom": "*",
5+
"react": "*",
6+
"react-dom": "*",
7+
"typescript": "3.1.3"
8+
}
9+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
import * as React from 'react';
2+
3+
class App extends React.Component {
4+
render() {
5+
return <div>{format(123)}</div>;
6+
}
7+
}
8+
9+
function format(value: string) {
10+
return value;
11+
}
12+
13+
export default App;
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
import * as React from 'react';
2+
import * as ReactDOM from 'react-dom';
3+
import App from './App';
4+
5+
ReactDOM.render(<App />, document.getElementById('root'));

0 commit comments

Comments
 (0)