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

Adds TSC_COMPILE_ON_ERROR env var... #6931

Merged
merged 7 commits into from Oct 1, 2019
4 changes: 3 additions & 1 deletion docusaurus/docs/adding-typescript.md
Expand Up @@ -31,7 +31,9 @@ yarn add typescript @types/node @types/react @types/react-dom @types/jest

kylebebak marked this conversation as resolved.
Show resolved Hide resolved
Next, rename any file to be a TypeScript file (e.g. `src/index.js` to `src/index.tsx`) and **restart your development server**!

Type errors will show up in the same console as the build one.
Type errors will show up in the same console as the build one. By default, Create React App prevents you from running the dev server if your code has type errors. If you introduce type errors to your project, you have to fix or ignore them before you continue development.
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd rather we omit this for now, as we don't want users to do this unless they absolutely must. Perhaps just as small snippet like "for advanced configuration, see (advanced config)" - as a terrible example.

That way, people can find it if they need, but they won't assume it's a normal practice to use this.

Ideally, this should never be used unless you're migrating a lot of legacy code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey @mrmckeb

I've changed the above to the following:

Type errors will show up in the same console as the build one. You'll have to fix these type errors before you continue development or build your project. For advanced configuration, see here.

Now it doesn't explicitly or even implicitly encourage anyone to do this =)

Thanks for your feedback!


You can remove this restriction by running your app with the `TSC_COMPILE_ON_ERROR` environment variable set to `true`, for example, by adding `TSC_COMPILE_ON_ERROR=true` as documented in our [advanced configuration](advanced-configuration.md).
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd like to say that we don't recommend this. @ianschmitz, what are your thoughts?

Copy link
Contributor

Choose a reason for hiding this comment

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

Good point. I agree this is likely an advanced scenario for those porting legacy code. New code bases really shouldn't be using this option.


To learn more about TypeScript, check out [its documentation](https://www.typescriptlang.org/).

Expand Down
1 change: 1 addition & 0 deletions docusaurus/docs/advanced-configuration.md
Expand Up @@ -23,3 +23,4 @@ You can adjust various development and production settings by setting environmen
| INLINE_RUNTIME_CHUNK | 🚫 Ignored | ✅ Used | By default, Create React App will embed the runtime script into `index.html` during the production build. When set to `false`, the script will not be embedded and will be imported as usual. This is normally required when dealing with CSP. |
| IMAGE_INLINE_SIZE_LIMIT | 🚫 Ignored | ✅ Used | By default, images smaller than 10,000 bytes are encoded as a data URI in base64 and inlined in the CSS or JS build artifact. Set this to control the size limit in bytes. Setting it to 0 will disable the inlining of images. |
| EXTEND_ESLINT | ✅ Used | ✅ Used | When set to `true`, ESLint configs that extend `eslint-config-react-app` will be used by `eslint-loader`. Any rules that are set to `"error"` will stop the application from building. |
| TSC_COMPILE_ON_ERROR | ✅ Used | ✅ Used | When set to `true`, you can run and properly build TypeScript projects even if there are TypeScript type check errors. These errors are printed as warnings in the terminal and/or browser console. |
1 change: 1 addition & 0 deletions packages/react-dev-utils/README.md
Expand Up @@ -337,6 +337,7 @@ The `args` object accepts a number of properties:
- **urls** `Object`: To provide the `urls` argument, use `prepareUrls()` described below.
- **useYarn** `boolean`: If `true`, yarn instructions will be emitted in the terminal instead of npm.
- **useTypeScript** `boolean`: If `true`, TypeScript type checking will be enabled. Be sure to provide the `devSocket` argument above if this is set to `true`.
- **tscCompileOnError** `boolean`: If `true`, errors in TypeScript type checking will not prevent start script from running app, and will not cause build script to exit unsuccessfully. Also downgrades all TypeScript type checking error messages to warning messages.
ianschmitz marked this conversation as resolved.
Show resolved Hide resolved
- **webpack** `function`: A reference to the webpack constructor.

##### `prepareProxy(proxySetting: string, appPublicFolder: string): Object`
Expand Down
19 changes: 16 additions & 3 deletions packages/react-dev-utils/WebpackDevServerUtils.js
Expand Up @@ -108,6 +108,7 @@ function createCompiler({
urls,
useYarn,
useTypeScript,
tscCompileOnError,
webpack,
}) {
// "Compiler" is a low-level interface to Webpack.
Expand Down Expand Up @@ -190,16 +191,28 @@ function createCompiler({

const messages = await tsMessagesPromise;
clearTimeout(delayedMsg);
statsData.errors.push(...messages.errors);
if (tscCompileOnError) {
statsData.warnings.push(...messages.errors);
} else {
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);
if (tscCompileOnError) {
stats.compilation.warnings.push(...messages.errors);
} else {
stats.compilation.errors.push(...messages.errors);
}
stats.compilation.warnings.push(...messages.warnings);

if (messages.errors.length > 0) {
devSocket.errors(messages.errors);
if (tscCompileOnError) {
devSocket.warnings(messages.errors);
} else {
devSocket.errors(messages.errors);
}
} else if (messages.warnings.length > 0) {
devSocket.warnings(messages.warnings);
}
Expand Down
14 changes: 11 additions & 3 deletions packages/react-scripts/scripts/build.js
Expand Up @@ -122,9 +122,17 @@ checkBrowsers(paths.appPath, isInteractive)
);
},
err => {
console.log(chalk.red('Failed to compile.\n'));
printBuildError(err);
process.exit(1);
const tscCompileOnError = process.env.TSC_COMPILE_ON_ERROR === 'true';
if (tscCompileOnError) {
console.log(chalk.yellow(
'Compiled with the following type errors (you may want to check these before deploying your app):\n'
));
printBuildError(err);
} else {
console.log(chalk.red('Failed to compile.\n'));
printBuildError(err);
process.exit(1);
}
}
)
.catch(err => {
Expand Down
2 changes: 2 additions & 0 deletions packages/react-scripts/scripts/start.js
Expand Up @@ -95,6 +95,7 @@ checkBrowsers(paths.appPath, isInteractive)
const protocol = process.env.HTTPS === 'true' ? 'https' : 'http';
const appName = require(paths.appPackageJson).name;
const useTypeScript = fs.existsSync(paths.appTsConfig);
const tscCompileOnError = process.env.TSC_COMPILE_ON_ERROR === 'true';
const urls = prepareUrls(protocol, HOST, port);
const devSocket = {
warnings: warnings =>
Expand All @@ -110,6 +111,7 @@ checkBrowsers(paths.appPath, isInteractive)
urls,
useYarn,
useTypeScript,
tscCompileOnError,
webpack,
});
// Load proxy config
Expand Down