Provide option to ignore semantic compile errors

[masaeedu]

I keep getting errors about xyz is declared but not used, which I want to keep because they are useful in the editor during development, but I don't want to see a blank page with this error instead of my app.

TypeScript does not require all semantic errors to be fixed in order to emit JS, so there should be a corresponding option for create-react-app-typescript to ignore semantic errors during live development.

[ianschmitz]

You can edit the tsconfig.json that is in your project root. I believe noUnusedLocals is the setting you're after.

[masaeedu]

@ianschmitz I do not want to disable noUnusedLocals. As I said:

I want to keep [these errors] because they are useful in the editor during development

I just want to ignore the errors and go ahead with viewing the result of the emit.

[ianschmitz]

The problem is that noUnusedLocals throws a compiler error. Any compile errors will trigger the error overlay in CRA.

I don't know of any other way to accomplish what you're after, without either disabling noUnusedLocals or waiting for something like microsoft/TypeScript#13408 to land.

[masaeedu]

@ianschmitz I understand that this is a compiler error. If you've used TypeScript before, you'll know that compiler errors do not prevent emit; it is an explicit goal of the TypeScript project to maintain a distinction between semantic errors (e.g. "you are adding a potato to a number, this makes no sense"), and syntactic errors ("I can't even parse your code as valid TypeScript"). Only the latter prevent your code from being transpiled to JS.

I want an option to ignore semantic errors (such as noUnusedLocals) and simply hot reload my app in the event that there are no syntactic errors and emit succeeds.

There are no additional features needed in TypeScript to implement this: there's already a transpileModule function that turns syntactically valid TypeScript to syntactically valid <whatever ES target you're pointing at> while completely ignoring any semantic errors. E.g. you can do const module = ts.transpileModule('const a: string = 2; a = 0;') and it'll happily transpile it for you, ignoring the const a: string = 2 issue.

[nico1510]

This would be really important...

[geekflyer]

i agree. Its super annoying and slowing me down that i always have to make everything type compliant when I'm just hacking around with some experimental changes in my code etc..
I'm actually surprised this is not yet possible with CRA-TS, because with normal tsc workflows that's the standard behaviour.

I think CRA-TS ideally should respect the noEmitOnError and only show semantic errors in the overlay if "noEmitOnError": true in the tsconfig.

[DorianGrey]

[...] when I'm just hacking around with some experimental changes in my code [...]

I've seen a bit too many of these "experimental changes" going into production just because someone felt himself "slowed down" by a transpiler or type-checker arguing that the provided code does not make sense or is semantically wrong to think it is a good idea to suppress errors from popping up strictly, esp. for "quick changes".
The setup is intended to help you find, prevent and fix potential errors before you even think about pushing a commit, i.e. enforce you to deal with them as early as possible.

[masaeedu]

@DorianGrey If your safeguard against broken code is someone checking the create-react-app UI I'm not surprised you're seeing half baked stuff going into production. Your CI should be preventing merge of code that has type errors.

This is precisely why I don't want to touch settings like noUnusedLocals in my tsconfig.json, which is used during CI to type check the code. I instead expect that the development server, i.e. a development time tool to help me develop stuff, will show me the goods as I edit despite the type errors.

In my workflow at least, and I would imagine in any sensible CI workflow, the developer will be forced to clean up any type errors before they're actually allowed to merge anything.

[DorianGrey]

Personally, I consider "preventing from merge" as too late in the development chain. I definitely favor "preventing from commit" resp. "preventing from push/review/pr".
CI builds may take quite long, especially when additional backend code is involved. Consider a CI build of 15-30 minutes failing due to a failed type check, occurring just because somehow considered strict options to "slow him down in development flow" and thus ignoring the type checkers output. Compare this build time wasted with a couple of seconds it takes to fix a type error. The latter sounds better to me.

[...] I instead expect my development server, which, you know, is a development time tool to help me develop stuff, to show me the goods as I edit despite the type errors. [...]

Seems we have quite different requirements here - I'd always expect that I get confronted with every kind of error immediately, regardless of its category (syntax, typing...) since all of them indicate that something is wrong with the code and (potentially) whatever comes out of it.
Or, the other way round: What exactly are transpilers, type checkers and code linters for if not for immediate feedback and forced actions?

[masaeedu]

Consider a CI build of 15-30 minutes failing due to a failed type check

This is also broken, because your code should be typechecked as soon as it's pushed to a branch, and it should take the CI the same amount of time to typecheck it as it takes your local machine. Regardless of how long the rest of the CI process is, if there's problems at this stage, you should find out right away, or fix your CI process. Anyway, this is all besides your original point of "experimental changes going into production", which as I've clarified, is a process issue in your environment.

I don't think it's a good idea to preemptively police user's access to features like unhindered emit (which is a default in the TypeScript compiler itself) because you know better or don't think it's best practices.

What exactly are transpilers, type checkers and code linters for if not for immediate feedback and forced actions?

Neither TypeScript nor any linter I've ever used worked with works this way. The feedback from both the type checker and the linter is available to me, and I can proceed to edit, build, even run transpiled code without addressing issues like noUnusedLocals or naming conventions the linter doesn't like.

[DorianGrey]

[...] is a process issue in your environment. [...]

It's less of a process issue than a human one... but that's a different topic.

[...] I don't think it's a good idea to preemptively police user's access to features like unhindered emit (which is a default in the TypeScript compiler itself) because you know better or don't think it's best practices. [...]

Almost every template, boilerplate or project generator I know of has an opinionated setup, because the people working on it consider a specific set of features or a particular behavior to be best practice. It's almost impossible to fit everyone's requirements without messing up the code with tons of configuration options to be taken care of.

[...] Neither TypeScript nor any linter I've ever used worked with works this way [...]

At least if you don't configure them to do so, yes. I've always configured my tools to yell as loud as possible as early as possible.

[...] The feedback from both the type checker and the linter is available to me, and I can proceed [...]

In my experience, people tend to ignore those information as long as they are not pushed to deal with them. Yes, the CI build will blame them, but especially on larger projects, that may take some time, and might raise when they are already dealing with a different issue or feature, forcing them to turn back and forth again. Had some extremely bad experiences with this kind of workflow, especially in larger or more complex projects, thus I'd recommend to avoid this whenever possible.

If its particularly about noUnusedLocals, you may still mark variables for "to be used later, don't blame me for their existence" with a leading _. Those variables are ignored regarding the unused rules available.

[masaeedu]

If its particularly about noUnusedLocals, you may still mark variables for "to be used later, don't blame me for their existence" with a leading _. Those variables are ignored regarding the unused rules available.

🤦‍♂️ This kind of hack is what leads to broken code sneaking past CI. You're saying you're concerned about unpolished code getting into production, then taking precisely the reverse of what approach should be taken to correct the problem.

A developer shouldn't add any hacks to turn off linter rules, type errors, etc. in their code or configuration. Any failing type checks or linter errors should always remain enabled, to be displayed to the developer in their editor and to be caught by CI if the developer for whatever reason tries to push broken code.

At the same time, my live reload tool, which is simply responsible for reloading the page with the transpiled result, should reload the page if emit succeeds. It should not force me to actually bake hacks into the actual code, or to change the linter/typechecker config to disable valuable errors, because then you're relying on a fallible human to remember to take out those hacks and turn those checks back on.

I already have a perfectly good editor notifying me of the typechecking and linter errors in the code I'm working on: I just need an option to view the rendered result despite semantic errors, without playing fast and loose with annotations or config that bypass linter and type checker rules that are there for a reason.

[Flux159]

So this was slowing me down quite a bit since it was causing valid Javascript to be marked as a compiler error. I decided to just hack the webpack configs (dev and prod) for create-react-app-typescript and remove the ForkTsCheckerWebpackPlugin by default.

Example of what I did in packages/react-scripts/config/webpack.config.dev.js:

(process.env.NO_EMIT_ON_ERROR ? new ForkTsCheckerWebpackPlugin({
      async: false,
      watch: paths.appSrc,
      tsconfig: paths.appTsConfig,
      tslint: paths.appTsLint,
    }) : null),
  ].filter(Boolean),

I just wanted to make minimal changes so its super hacky and I just used an environment variable to toggle between no emit on error. I think it would be more ideal if ForkTsCheckerWebpackPlugin offered this as an option itself. Alternatively, just read directly from the file at paths.appTsConfig and check the tsconfig.json's noEmitOnError property.

My repo is here for reference.

I still get compiler errors appearing during development when I write invalid javascript (syntactic errors), but I don't get semantic errors breaking my development flow. I can also still see semantic errors in my editor since I made no changes to tsconfig.json (Visual Studio Code reads tsconfig.json automatically). If I want to force errors to break the build (or for CI), I can simply do this:

NO_EMIT_ON_ERROR=1 yarn start

This workflow is much closer to create-react-native-app's typescript generator which is super elegant to use during development in my opinion.

I also published my fork as @flux159/react-scripts-ts, if you want to use it / test it right now you could simply do:

create-react-app <NAME> --scripts-version=@flux159/react-scripts-ts

Personally, I think the ideal solution for this would be a configurable option in create-react-app-typescript using tsconfig.json's "noEmitOnError" value with the default being do not break the app on semantic errors (similar to the Typescript compiler's default).

[zheeeng]

@DorianGrey I encountered a big trouble with this setup. I import in a third module which is written in loose mode typescript, it contains unused locals, parameters, no fallback switch statement, implicit any and no-full-checked condition branch. And I triggered on all strict checkings on my own project. No matter how bad the 3rd lib it is, it works. At presant, have a way to bypass these 'errors' is necessary. Maybe change errors to warnings cloud save me and let me go forward.

[whut]

There is an important comment from @nickmccurdy posted on #360:

I believe you can use "defaultSeverity": "warning" in tslint.json for now.

I can only add that this should be added by default to tslint.json generated by create-react-app-typescript. Alternatively it could add "defaultSeverity": "error" as this is the default of tslint --init.