microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
101.09k stars 12.5k forks source link

Make error TS2367 optional #50178

Closed phuhl closed 1 year ago

phuhl commented 2 years ago

Suggestion

When developing, you sometimes produce unfinished code, that you want to compile. In these cases, certain errors are hindering compilation unnecessarily. E.g. error TS2367 "This condition will always return 'false' since the types '"abc"' and '"def"' have no overlap."

While I certainly want that error to throw in a CI run, I don't want it to block my dev flow.

🔍 Search Terms

TS2367 disable error globaly https://github.com/Microsoft/TypeScript/issues/29950

✅ Viability Checklist

My suggestion meets these guidelines:

⭐ Suggestion

Either a way to disable errors like you can e.g. with eslint,

or more specifically to this issue, a compiler flag to disable TS2367.

📃 Motivating Example

Concider this (react) example

const PaymentWallet = (props: { walletType: "googlepay" }) => {
  switch (props.walletType) {
    case "googlepay":
      return <GooglepayComponent/>;
    case "applepay":
      return <ApplepayComponent/>;
  }
}

Let's assume, the switch case is a bit more complex and maybe I copied it from somewhere else. It is clear, that the applepay case will never be called according to the functions type signature. I want that error when building for deploy as it catches an obvious error.

But right now I am just developing happily. I don't want to remove code, that I will later need anyways. I just want to ignore the warning without adding something like / @ts-ignore / (as I might forget to remove it later).

Instead, I want a separate tsconfig for dev, which is much more relaxed and shows me a warning instead of an error and still lets me compile. Why nicer to work with.

💻 Use Cases

This is mainly a feature for improving development. Current workaround would be, to make sure that all you intermediate code does not produce warnings. That is tedious as often times in dev you don't know for sure which code will make it into prod. Why cleaning all that code, if we might throw it away later?

MartinJohns commented 2 years ago

Either a way to disable errors like you can e.g. with eslint,

You can use // @ts-ignore or // @ts-expect-error. The latter will ensure you remove the comment again once everything is complete.

I don't want it to block my dev flow.

Why would it? The error does not stop the rest of the type checking or the emit.

phuhl commented 2 years ago

Why would it? The error does not stop the rest of the type checking or the emit.

With react scripts it does produce a overlay that pops over the application every time it builds. That is pretty intrusive. For many errors that makes sense, but for these "non-braking" things, that really should be warnings, it is too much.

You can use // @ts-ignore or // @ts-expect-error. The latter will ensure you remove the comment again once everything is complete.

If I use the latter and forget to finish my implementation, there will be no warning later on. Also, both of them disable any ts error, not specifically the one I am facing.

fatcerberus commented 2 years ago

Duplicate (more or less) of https://github.com/microsoft/TypeScript/issues/19139.

For many errors that makes sense, but for these "non-[breaking]" things, that really should be warnings, it is too much.

TS makes no distinction between errors and warnings (likely because almost all errors are non-breaking under type erasure; tsc is effectively a linter), but I do wish the compiler could be made to stop returning a nonzero exit code when noEmitOnError is set to false. :-/

Josh-Cena commented 2 years ago

Slightly off-topic: react-scripts making TS errors block the build is the last thing you want a build tool to do...

RyanCavanaugh commented 2 years ago

Things have been going pretty OK without a true warning/error distinction so far and opening up the pandora's box of "well this is an error and this is a warning and this is a warning under these conditions otherwise an error and these are the six thousand tsc flags you can use to toggle this" is just not something that seems worthwhile.

Even in this specific instance it's hard to reason about whether this is an "error" or a "warning". If you wrote something like

function formatHardDrive(mode: "test" | "actually-do-it") {
  if (mode === "tst") {
    mockFormatHardDrive();
  } else {
    actuallyFormatHardDrive();
  }
}

Well now your code is extremely wrong in a way that's probably very dangerous. But there's nothing in principle different about this case and the one in the OP.

Our longstanding philosophy is that dev toolchains should not block execution on typecheck.

fatcerberus commented 2 years ago

Our longstanding philosophy is that dev toolchains should not block execution on typecheck.

I agree in theory, though the problem is that currently tsc returns a nonzero exit code on any error (i.e. tsc && postbuild will short-circuit) and it’s impossible to know if that’s just because of an ignorable type error vs. something genuinely fatal that even prevented emit. So if I’m writing a toolchain, it’s safest right now to treat all nonzero exit codes from tsc as blocking, which indeed seems to be what most toolchains do today.

MartinJohns commented 2 years ago

@fatcerberus FYI #13280

hugobranquinho commented 1 year ago

Just to give an example of something that's valid and we have the TS2367

function updateObj(obj: { id: number }) {
  obj.id = 2;
}

function run(obj: { id: number }) {
  if (obj.id !== 0) {
    return;
  }

  updateObj(obj);

  if (obj.id === 2) {
    // 
  }
}

obj.id === 2 is marked as TS2367

typescript-bot commented 1 year ago

This issue has been marked as "Declined" and has seen no recent activity. It has been automatically closed for house-keeping purposes.