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

Speed up TypeScript projects #5903

Merged
merged 18 commits into from
Feb 8, 2019
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
89 changes: 69 additions & 20 deletions packages/react-dev-utils/WebpackDevServerUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,22 +17,9 @@ const inquirer = require('inquirer');
const clearConsole = require('./clearConsole');
const formatWebpackMessages = require('./formatWebpackMessages');
const getProcessForPort = require('./getProcessForPort');
const typescriptFormatter = require('./typescriptFormatter');

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 =>
Expand Down Expand Up @@ -113,12 +100,12 @@ 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();
Expand All @@ -139,10 +126,30 @@ 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);
});
});

compiler.hooks.forkTsCheckerReceive.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();
}
Expand All @@ -152,9 +159,32 @@ 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) {
process.stdout.write(
ianschmitz marked this conversation as resolved.
Show resolved Hide resolved
chalk.yellow('Files successfully emitted, waiting for typecheck results...')
ianschmitz marked this conversation as resolved.
Show resolved Hide resolved
);

const messages = await tsMessagesPromise;
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);
}

process.stdout.clearLine();
process.stdout.cursorTo(0);
}

const messages = formatWebpackMessages(statsData);
const isSuccessful = !messages.errors.length && !messages.warnings.length;
if (isSuccessful) {
console.log(chalk.green('Compiled successfully!'));
Expand Down Expand Up @@ -194,6 +224,25 @@ 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)
ianschmitz marked this conversation as resolved.
Show resolved Hide resolved
});
compiler.hooks.done.tap('smokeTest', async stats => {
await tsMessagesPromise;
if (stats.hasErrors() || stats.hasWarnings()) {
process.exit(1);
} else {
process.exit(0);
}
});
}

return compiler;
}

Expand Down
7 changes: 5 additions & 2 deletions packages/react-dev-utils/typescriptFormatter.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
8 changes: 2 additions & 6 deletions packages/react-dev-utils/webpackHotDevClient.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
ianschmitz marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the only thing I'm on the fence about, what's the reason for this change? Can you please add the reasoning in the code?

Copy link
Contributor Author

@deftomat deftomat Jan 10, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem is that we need to print warnings even when there is not update in code.
Without this change,

if (!isUpdateAvailable() || !canApplyUpdates()) {
return;
}
will cancel it as __webpack_hash__ didn't change.

So, the idea is that we want to print warnings every time they occur.

Yes, we can figure out a different way, but this change tries to keep it as simple as possible.

// 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();
});
} else {
// Print initial warnings immediately.
printWarnings();
}
}

Expand Down
4 changes: 2 additions & 2 deletions packages/react-scripts/config/webpack.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -619,7 +619,7 @@ module.exports = function(webpackEnv) {
typescript: resolve.sync('typescript', {
basedir: paths.appNodeModules,
}),
async: false,
async: isEnvDevelopment,
checkSyntacticErrors: true,
tsconfig: paths.appTsConfig,
compilerOptions: {
Expand All @@ -640,7 +640,7 @@ module.exports = function(webpackEnv) {
],
watch: paths.appSrc,
silent: true,
formatter: typescriptFormatter,
formatter: isEnvProduction ? typescriptFormatter : undefined,
deftomat marked this conversation as resolved.
Show resolved Hide resolved
}),
].filter(Boolean),
// Some libraries import Node modules but don't use them in the browser.
Expand Down
7 changes: 6 additions & 1 deletion packages/react-scripts/scripts/start.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,9 +94,14 @@ 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);
ianschmitz marked this conversation as resolved.
Show resolved Hide resolved
// Load proxy config
const proxySetting = require(paths.appPackageJson).proxy;
const proxyConfig = prepareProxy(proxySetting, paths.appPublic);
Expand Down