You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Feature idea/improvement: allow dev server and build script to run in presence of TypeScript errors #6930
This isn't a bug report, but rather an idea to make CRA's TypeScript setup more user-friendly, and more aligned with what I understand to be the "spirit" of TypeScript.
Currently, for projects built with npx create-react-app <app> --typescript, or migrated to TypeScript as per the docs, both npm start and npm build fail in the presence of TypeScript type errors.
The dev server refuses to serve the app if non-fatal type errors are present. The build script builds it just fine, but exits with a non-zero status. In other words, npm build outputs valid code to /build that can deployed to a production server, even as it tells the user that the app "Failed to compile".
Devs migrating a moderately sized CRA app to TypeScript may literally be faced with thousands of type errors initially. None of these prevents the app from running, either locally or in production, but devs have to correct or ignore every last error (not that easy, given the limitations of @ts-ignore) before they can continue developing the app.
IMO this goes against TypeScript's philosophy of making the migration path from JS -> TS as easy as possible, and allowing devs to "progressively" add type annotations to their code.
TypeScript devs already have many options at their disposal to prevent deployment of code that has type errors, if they want to impose this constraint on themselves. CRA definitely shouldn't impose it on them.
At the very least, I think there should be an option to enable or disable this constraint. I understand that CRA's philosophy is to avoid config wherever possible. If adding an env var, e.g. TSC_COMPILE_ON_ERROR, is not desirable because it means more config, then I think the default behavior should be for CRA to function normally in the presence of type errors, while printing them as warnings to the terminal/browser console.
Reacted by Robert Cooper, Zach, Biel Simon Rojas, ericbiewener, Александр Коровин, Tanya Tsvetkova, Arutyunyan Artem, Maksim Kononov, Anastasiia Lopatina, Andrey Melnikov and 11 more
It sounds like it's actually triggering an error because the TS compile failed (in which case you don't actually have your latest code in your build yet) but by your description, I'm not sure.
I haven't used TS w/CRA yet, but I'd imagine they are building from JS, rather than building from TS then compiling, but I could be wrong.
in which case you don't actually have your latest code in your build yet...
I definitely do.
You can write code that has type errors, save the code, and even see the updated app in the browser momentarily before the "Failed to compile" screen takes over.
And no, this isn't something that can be avoided by modifying tsconfig.json. Check out the PR if you want to, you'll see there's code, for example in react-dev-utils/WebpackDevServerUtils.js, that intercepts type errors and shows the "Failed to compile" screen if any are present.
If you change this code to not show the "Failed to compile" screen, you can still see the type errors, but the code compiles just fine (which means the "Failed to compile" message isn't really accurate anyway, but that's another, much less important issue).
in which case you don't actually have your latest code in your build yet...
I definitely do.
You can write code that has type errors, save the code, and even see the updated app in the browser momentarily before the "Failed to compile" screen takes over.
And no, this isn't something that can be avoided by modifying tsconfig.json. Check out the PR if you want to, you'll see there's code, for example in react-dev-utils/WebpackDevServerUtils.js, that intercepts type errors and shows the "Failed to compile" screen if any are present.
If you change this code to not show the "Failed to compile" screen, you can still see the type errors, but the code compiles just fine (which means the "Failed to compile" message isn't really accurate anyway, but that's another, much less important issue).
I'm going to try this myself, just because I'm curious, but have you actually tried this by creating a new app using the command in the docs, instead of npx create-react-app cra-typescript --typescript?
The recommended method is npx create-react-app my-app --typescript.
Edit - I tested this and it does in fact throw a proper error in dev mode (npm start) - as I would expect it to since there is a TS error; ultimately CRA is referencing the tsconfig / the compiler for this error, then deciding to quit.
You can prove this by placing a // @ts-ignore before the offending line. For example, I used this to test:
In both the case of npm start and npm run build it worked fine. This indicates the compiler is being referenced for the error.
Edit - I see that your PR code makes this an optional boolean, which I like. Personally I'd rather it did not ignore the error, unless I had acknowledged it and chosen to do so as the potential for side effects would not necessarily be clear.
There may be non-fatal errors that still introduce bugs, even security issues.
I would like to express my support for some more user-friendly way to deal with ts compilation errors. We have a very large typescript codebase so the experience we have right now is:
cra compiles js and app loads
10-15 seconds later, the screen gets covered by a non-dismissible overlay that shows a type error that could be as trivial as "unused import"
We fix the issue and have to wait for recompilation.
In addition to that scenario, we also get the issue of type errors that have already been fixed appearing as errors because the compiler is behind.
Any method to either allow the overlay to be dismissible or disable compilation failure on type error would be really great
Your command relies on bash (we can't assume someone won't be using sh or csh and echo doesn't always have the same behavior in different shells) and it also rewrites the .env file every time that command is run.
Writing echo > anything will overwrite the entire file with just anything, so each time you're rewriting the entire file, which means destroying what is there.
Since it's not appending (or checking if the .env parameter already exists), and because there is always a possibility of additional config in a user's environment being overwritten, I feel like that's a non-starter as its destructive and unpredictable.
Generally, that type of a command shouldn't be associated with a startup script. If it's not your intention to include that as a prereq to the PR, maybe that should be clarified in the PR.
I'd like to see an approach that's as simple as npm start --ts-ignore-errors that doesn't require modifications to the .env to run on a per-case basis, and maybe allows a modification to .env to make "permanent". A simple flag argument shouldn't require modifying the .env, much less a destructive file write.
To test it out, you can just create a bare TypeScript app with npx create-react-app cra-typescript --typescript, then change the react-scripts dep as follows.
Sounds like it's a way to test the code in the PR.
As for "permanently" modifying .env, you could always create your own .env file, add whatever you want to it, then run npm start.
There’s no need to be a jerk when also arguing semantics.
I was clearly referring to the “PR” including the comments.
My suggestion is that if you intend to make a change that requires an edit to the .env then:
This particular case seems like it would be a good place for a command line arg, since many will want the option to bypass as needed, but not necessarily configure an environment variable for this.
Maybe give that example of editing the .env in your PR COMMENTS (just want to be clear here) instead of the current PR COMMENTS which, while an example of how to test this, still include bad and possibly destructive practice that might be terrible advice for someone wanting to test it out who doesn’t realize the repercussions of overwriting a potentially vital .env config.
This is really needed. Trying to migrate a .js app to .ts is a hard as it is, and cra overwrites compilerOptions is tsconfig.json if you try to edit them, so there is no solution going that way.
Personally I'd rather it did not ignore the error, unless I had acknowledged it and chosen to do so
That's what the option would do. We are acknowledging the errors are there. Our editors still show them, the console output will still show them, but we will be able to develop our apps. Developing is not a road without errors, forcing the overlay and preventing the build without an option to not do so is not helpful.
Just to clarify, the PR linked to in this issue doesn't ignore any errors. It does what you suggest -- it prints the errors to the terminal and to the browser console, but doesn't prevent the dev server from serving the app in the presence of TypeScript errors.
and cra overwrites compilerOptions is tsconfig.json if you try to edit them, so there is no solution going that way.
Also worth clarifying: even if CRA didn't overwrite compilerOptions, you wouldn't be able to solve this problem by writing your own.
There's nothing in CRA's compilerOptions that prevents TypeScript from compiling your TS files to JS. It compiles the files just fine, it just refuses to serve them. If you make a change to your source code and save it, you can even see the updated app in the browser momentarily before the "Failed to compile" screen takes over.
To solve this problem, you have to modify react-dev-utils/WebpackDevServerUtils.js, which is what is done in the PR.
2. 10-15 seconds later, the screen gets covered by a non-dismissible overlay that shows a type error that could be as trivial as "unused import"
This is the annoying part. You might want 'noImplicitAny' or 'noUnusedImports' to be strictly enforced during prod builds or code reviews, but having it show the error overlay during development with no way to dismiss it despite it being perfectly valid emitted javascript is frustrating.
It seems like the error overlay used to be dismissible at some point, or maybe its just not dismissible for typescript errors, but its a pain point either way.
The errors are already being printed in the terminal and the browser console, so having the option to dismiss or opt out of the overlay errors would be awesome.
It would be great to get some insight from a contributor on what the current thought process is for not making errors dismissible and what would be required to change that.
I’m a contributor but I’m certainly not a maintainer or owner of this package, but I’ll give my two cents, though I don’t think you’ll like it ;).
I’m wondering how exactly the process would go, just in matter of order of processing, to build the app but also have type errors.
The JS that is used for the build is first compiled from the associated TS files.
If you’re just writing TS (not inside a CRA project) and have the watch TS build task running, essentially what happens in CRA, you don’t get any JS files if your TS fails type checking unless you’ve configured your tsconfig in such a way to allow it. Even then, you don’t always get it to compile depending on the error.
My thought would be (as I said before) relaxing the tsconfig, and on the CRA side allowing certain errors, but the tsc or babel compilers aren’t going to care what CRA wants; they are going to fail to compile in instances where they would fail otherwise, unless excepted by tsconfig or some argument on the CLI.
CRA ultimately fails because files required to make the build, the JS not the TS fail to be generated, which is up to the compiler.
I’m still not clear why the OP would not be able to see TS errors before compile, since they are linted in the TS already; it would be obvious this is going to fail to compile.
The argument that migrating a project to TS is going to result in, basically, a lot of work fixing type errors begs the question why you would want to build the app from that instance vs the JS instance when you haven’t fixed any of the type issues?
In my experience, moving an app from JS to TS, the most common occurrence of type errors stems from two things: you haven’t installed the applicable @types package(s) and/or you haven’t imported the necessary types from those packages (less common as it’s usually detected).
I just migrated 8 screens and an API from RN to TS and these are the only issues I faced beyond some ambiguity which is (and shouldn’t be) allowed in JS and not in TS (like using a global which isn’t locally defined - think “ref” on the instance).
I'm not sure @jineshshah36 and I have the same concern as the OP actually.
Let's say you quickly prototype some function: const func = (x) => doThing(x) and then save. After a few seconds, CRA will have emitted the new perfectly valid javascript bundles. Now you're trying to test out the new function in the browser after your live-reload kicks in. Imagine you're clicking around on the page, and you're about to get to the point where you do whatever is necessary to trigger your function - you've already invested some time and effort. Suddenly the error overlay appears out of nowhere to complain that x in your function is implicitly typed as any (because the async type-checking process has just finished). The critical issue here is that there is no way to dismiss the error overlay.
The error overlay says "This error occurred during the build time and cannot be dismissed.", but it didn't occur during build time (since babel just strips the types before compiling/handing off to webpack - it actually happens during the async type checking), and it clearly can be dismissed (you can delete the overlay iframe using devtools, and the app would work fine in this case).
It's easy to say "just give x a type", but there are plenty of situations where its just not that simple (or you might come back to add types later after sanity testing your prototype).
IMO the overlay should only be non-dismissible if the actual babel compilation / webpack bundling process fails.
@methodbox I appreciate the thorough response! And yes, @MattMorrisDev is correct that is essentially the gist of the problem. We literally just want an "x" in the corner. If you think it makes more sense to open a separate issue and explain that, I'm more then happy to do that
Currently it is very difficult and frustrating to develop CRA app with typescript because of this problem. I think that OP approach with adding optional env var is just fine. I even think that by default TS compilation errors should be warnings.
Reacted by Tanya Tsvetkova, Maksim Kononov, Sergey Gavshin, Egor Ogarkov, Anastasiia Lopatina, Artyom Resh, Andrey Melnikov, Vyacheslav Popov, Vladislav Grigorzhevskii, Dmitry Petrov and 3 more
Guys, how is it going? I'm looking forward to use CRA without any hacking. The inability to start compiling forces me to write my own unmaintainable hacks every time again and again. I throw myself on your mercy.)
Reacted by Andrey Melnikov, Elizaveta Yanovskaya, Andrey Yankovsky and Maksim Kononov
Our team now really need way to start server if we have TypeScript errors. This improvement will help us. I think way using some argument flag will be useful, something like --ignore-typescript-errors
Reacted by Maksim Kononov, Andrey Melnikov, Andrey Yankovsky and Maxim Vislogurov
I encountered a similar problem. I have some JS codebase I want translate to TS, but I cant do this step by step - I have to translate WHOLE component before I can see results on my screen. I think it is nice to have an option to ignore TS errors on translate period.
Activity
methodbox commentedon Apr 27, 2019
Couldn't this be solved by setting your
tsconfigwith a more passive setting (like allowing compile on errors) and doing the compile before the build?https://www.typescriptlang.org/docs/handbook/compiler-options.html
It sounds like it's actually triggering an error because the TS compile failed (in which case you don't actually have your latest code in your build yet) but by your description, I'm not sure.
I haven't used TS w/CRA yet, but I'd imagine they are building from JS, rather than building from TS then compiling, but I could be wrong.
kylebebak commentedon Apr 27, 2019
I definitely do.
You can write code that has type errors, save the code, and even see the updated app in the browser momentarily before the "Failed to compile" screen takes over.
And no, this isn't something that can be avoided by modifying
tsconfig.json. Check out the PR if you want to, you'll see there's code, for example inreact-dev-utils/WebpackDevServerUtils.js, that intercepts type errors and shows the "Failed to compile" screen if any are present.If you change this code to not show the "Failed to compile" screen, you can still see the type errors, but the code compiles just fine (which means the "Failed to compile" message isn't really accurate anyway, but that's another, much less important issue).
methodbox commentedon Apr 27, 2019
I'm going to try this myself, just because I'm curious, but have you actually tried this by creating a new app using the command in the docs, instead of
npx create-react-app cra-typescript --typescript?The recommended method is
npx create-react-app my-app --typescript.https://facebook.github.io/create-react-app/docs/adding-typescript#docsNav
Edit - I tested this and it does in fact throw a proper error in dev mode (
npm start) - as I would expect it to since there is a TS error; ultimately CRA is referencing the tsconfig / the compiler for this error, then deciding to quit.You can prove this by placing a
// @ts-ignorebefore the offending line. For example, I used this to test:In both the case of
npm startandnpm run buildit worked fine. This indicates the compiler is being referenced for the error.Edit - I see that your PR code makes this an optional boolean, which I like. Personally I'd rather it did not ignore the error, unless I had acknowledged it and chosen to do so as the potential for side effects would not necessarily be clear.
There may be non-fatal errors that still introduce bugs, even security issues.
jineshshah36 commentedon May 1, 2019
I would like to express my support for some more user-friendly way to deal with ts compilation errors. We have a very large typescript codebase so the experience we have right now is:
In addition to that scenario, we also get the issue of type errors that have already been fixed appearing as errors because the compiler is behind.
Any method to either allow the overlay to be dismissible or disable compilation failure on type error would be really great
methodbox commentedon May 1, 2019
I agree with most of this, in theory, but I question this approach (from the PR):
Your command relies on bash (we can't assume someone won't be using sh or csh and echo doesn't always have the same behavior in different shells) and it also rewrites the
.envfile every time that command is run.Writing
echo > anythingwill overwrite the entire file with justanything, so each time you're rewriting the entire file, which means destroying what is there.Since it's not appending (or checking if the
.envparameter already exists), and because there is always a possibility of additional config in a user's environment being overwritten, I feel like that's a non-starter as its destructive and unpredictable.Generally, that type of a command shouldn't be associated with a startup script. If it's not your intention to include that as a prereq to the PR, maybe that should be clarified in the PR.
I'd like to see an approach that's as simple as
npm start --ts-ignore-errorsthat doesn't require modifications to the.envto run on a per-case basis, and maybe allows a modification to.envto make "permanent". A simple flag argument shouldn't require modifying the.env, much less a destructive file write.kylebebak commentedon May 2, 2019
@methodbox
The code you put in your comment above is obviously not "from the PR".
It's from a comment in the discussion thread. Right before said code, the comment says this:
Sounds like it's a way to test the code in the PR.
As for "permanently" modifying
.env, you could always create your own.envfile, add whatever you want to it, then runnpm start.methodbox commentedon May 2, 2019
@kylebebak
There’s no need to be a jerk when also arguing semantics.
I was clearly referring to the “PR” including the comments.
My suggestion is that if you intend to make a change that requires an edit to the
.envthen:This particular case seems like it would be a good place for a command line arg, since many will want the option to bypass as needed, but not necessarily configure an environment variable for this.
Maybe give that example of editing the
.envin your PR COMMENTS (just want to be clear here) instead of the current PR COMMENTS which, while an example of how to test this, still include bad and possibly destructive practice that might be terrible advice for someone wanting to test it out who doesn’t realize the repercussions of overwriting a potentially vital.envconfig.cristianfraser commentedon May 7, 2019
This is really needed. Trying to migrate a .js app to .ts is a hard as it is, and
craoverwritescompilerOptionsistsconfig.jsonif you try to edit them, so there is no solution going that way.That's what the option would do. We are acknowledging the errors are there. Our editors still show them, the console output will still show them, but we will be able to develop our apps. Developing is not a road without errors, forcing the overlay and preventing the build without an option to not do so is not helpful.
kylebebak commentedon May 7, 2019
@cristianfraser
Just to clarify, the PR linked to in this issue doesn't ignore any errors. It does what you suggest -- it prints the errors to the terminal and to the browser console, but doesn't prevent the dev server from serving the app in the presence of TypeScript errors.
Also worth clarifying: even if CRA didn't overwrite
compilerOptions, you wouldn't be able to solve this problem by writing your own.There's nothing in CRA's
compilerOptionsthat prevents TypeScript from compiling your TS files to JS. It compiles the files just fine, it just refuses to serve them. If you make a change to your source code and save it, you can even see the updated app in the browser momentarily before the "Failed to compile" screen takes over.To solve this problem, you have to modify
react-dev-utils/WebpackDevServerUtils.js, which is what is done in the PR.MattMorrisDev commentedon May 21, 2019
This is the annoying part. You might want 'noImplicitAny' or 'noUnusedImports' to be strictly enforced during prod builds or code reviews, but having it show the error overlay during development with no way to dismiss it despite it being perfectly valid emitted javascript is frustrating.
It seems like the error overlay used to be dismissible at some point, or maybe its just not dismissible for typescript errors, but its a pain point either way.
The errors are already being printed in the terminal and the browser console, so having the option to dismiss or opt out of the overlay errors would be awesome.
jineshshah36 commentedon May 22, 2019
@methodbox
It would be great to get some insight from a contributor on what the current thought process is for not making errors dismissible and what would be required to change that.
methodbox commentedon May 22, 2019
@jineshshah36
I’m a contributor but I’m certainly not a maintainer or owner of this package, but I’ll give my two cents, though I don’t think you’ll like it ;).
I’m wondering how exactly the process would go, just in matter of order of processing, to build the app but also have type errors.
The JS that is used for the build is first compiled from the associated TS files.
If you’re just writing TS (not inside a CRA project) and have the
watchTS build task running, essentially what happens in CRA, you don’t get any JS files if your TS fails type checking unless you’ve configured yourtsconfigin such a way to allow it. Even then, you don’t always get it to compile depending on the error.My thought would be (as I said before) relaxing the
tsconfig, and on the CRA side allowing certain errors, but thetscorbabelcompilers aren’t going to care what CRA wants; they are going to fail to compile in instances where they would fail otherwise, unless excepted bytsconfigor some argument on the CLI.CRA ultimately fails because files required to make the build, the JS not the TS fail to be generated, which is up to the compiler.
I’m still not clear why the OP would not be able to see TS errors before compile, since they are linted in the TS already; it would be obvious this is going to fail to compile.
The argument that migrating a project to TS is going to result in, basically, a lot of work fixing type errors begs the question why you would want to build the app from that instance vs the JS instance when you haven’t fixed any of the type issues?
In my experience, moving an app from JS to TS, the most common occurrence of type errors stems from two things: you haven’t installed the applicable @types package(s) and/or you haven’t imported the necessary types from those packages (less common as it’s usually detected).
I just migrated 8 screens and an API from RN to TS and these are the only issues I faced beyond some ambiguity which is (and shouldn’t be) allowed in JS and not in TS (like using a global which isn’t locally defined - think “ref” on the instance).
MattMorrisDev commentedon May 22, 2019
I'm not sure @jineshshah36 and I have the same concern as the OP actually.
Let's say you quickly prototype some function:
const func = (x) => doThing(x)and then save. After a few seconds, CRA will have emitted the new perfectly valid javascript bundles. Now you're trying to test out the new function in the browser after your live-reload kicks in. Imagine you're clicking around on the page, and you're about to get to the point where you do whatever is necessary to trigger your function - you've already invested some time and effort. Suddenly the error overlay appears out of nowhere to complain thatxin your function is implicitly typed asany(because the async type-checking process has just finished). The critical issue here is that there is no way to dismiss the error overlay.The error overlay says "This error occurred during the build time and cannot be dismissed.", but it didn't occur during build time (since babel just strips the types before compiling/handing off to webpack - it actually happens during the async type checking), and it clearly can be dismissed (you can delete the overlay iframe using devtools, and the app would work fine in this case).
It's easy to say "just give x a type", but there are plenty of situations where its just not that simple (or you might come back to add types later after sanity testing your prototype).
IMO the overlay should only be non-dismissible if the actual babel compilation / webpack bundling process fails.
jineshshah36 commentedon May 22, 2019
@methodbox I appreciate the thorough response! And yes, @MattMorrisDev is correct that is essentially the gist of the problem. We literally just want an "x" in the corner. If you think it makes more sense to open a separate issue and explain that, I'm more then happy to do that
7 remaining items
methodbox commentedon Jun 17, 2019
@tomduncalf Would you mind sharing the
tsconfigyou’re using along with Parcel?tomduncalf commentedon Jun 17, 2019
When I'm doing a quick prototype I don't usually use one @methodbox, it has a default which seems to work okay
Yankovsky commentedon Jul 9, 2019
Currently it is very difficult and frustrating to develop CRA app with typescript because of this problem. I think that OP approach with adding optional env var is just fine. I even think that by default TS compilation errors should be warnings.
ArtyomResh commentedon Jul 9, 2019
It'd be be really helpful and great if this feature was implemented
vislogurov commentedon Jul 9, 2019
Guys, how is it going? I'm looking forward to use CRA without any hacking. The inability to start compiling forces me to write my own unmaintainable hacks every time again and again. I throw myself on your mercy.)
lise-yy commentedon Jul 9, 2019
Could you please please fix it?
Windrushfarer commentedon Jul 9, 2019
Our team now really need way to start server if we have TypeScript errors. This improvement will help us. I think way using some argument flag will be useful, something like
--ignore-typescript-errorsmaksim-kononov-csssr commentedon Jul 9, 2019
I encountered a similar problem. I have some JS codebase I want translate to TS, but I cant do this step by step - I have to translate WHOLE component before I can see results on my screen. I think it is nice to have an option to ignore TS errors on translate period.
socksrust commentedon Jul 24, 2019
Still a problem :/
sibelius commentedon Jul 24, 2019
Fixed here #6421
jischein commentedon Aug 2, 2019
+1
artalar commentedon Aug 24, 2019
Why it still does not fix? Error overlay for types error in development mode is an awful idea!
jineshshah36 commentedon Aug 24, 2019
This issue can now easily be fixed. Let's imagine you don't want an overlay to show for unused variables. Do the following:
noUnusedLocalstofalseintsconfig.json{ "root": true, "parser": "@typescript-eslint/parser", "parserOptions": { "project": "./tsconfig.json", "sourceType": "module", "jsx": true, "ecmaFeatures": { "jsx": true } }, "extends": ["react-app"], "rules": { "no-unused-vars": ["warn"], }, "overrides": [ { "files": ["**/*.ts?(x)"], "plugins": ["@typescript-eslint"], "extends": [ "plugin:@typescript-eslint/eslint-recommended", "plugin:@typescript-eslint/recommended", ], "rules": { "@typescript-eslint/no-unused-vars": [ "warn", { "ignoreRestSiblings": true, "varsIgnorePattern": "^_", "argsIgnorePattern": "^_", "caughtErrors": "all" } ] } } ] }You will no longer get an overlay but will get a warning in the console. You can also set it to off if you don't want to see the console warning.