Skip to content

Make error TS2367 optional #50178

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

Closed
5 tasks done
phuhl opened this issue Aug 4, 2022 · 9 comments
Closed
5 tasks done

Make error TS2367 optional #50178

phuhl opened this issue Aug 4, 2022 · 9 comments
Labels
Declined The issue was declined as something which matches the TypeScript vision Suggestion An idea for TypeScript

Comments

@phuhl
Copy link

phuhl commented Aug 4, 2022

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
#29950

✅ Viability Checklist

My suggestion meets these guidelines:

  • This wouldn't be a breaking change in existing TypeScript/JavaScript code
  • This wouldn't change the runtime behavior of existing JavaScript code
  • This could be implemented without emitting different JS based on the types of the expressions
  • This isn't a runtime feature (e.g. library functionality, non-ECMAScript syntax with JavaScript output, new syntax sugar for JS, etc.)
  • This feature would agree with the rest of TypeScript's Design Goals.

⭐ 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
Copy link
Contributor

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
Copy link
Author

phuhl commented Aug 4, 2022

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
Copy link

fatcerberus commented Aug 4, 2022

Duplicate (more or less) of #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. :-/

@RyanCavanaugh RyanCavanaugh added Suggestion An idea for TypeScript Declined The issue was declined as something which matches the TypeScript vision labels Aug 4, 2022
@Josh-Cena
Copy link
Contributor

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

@RyanCavanaugh
Copy link
Member

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
Copy link

fatcerberus commented Aug 4, 2022

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
Copy link
Contributor

@fatcerberus FYI #13280

@hugobranquinho
Copy link

hugobranquinho commented Apr 14, 2023

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
Copy link
Collaborator

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

@typescript-bot typescript-bot closed this as not planned Won't fix, can't repro, duplicate, stale Jun 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Declined The issue was declined as something which matches the TypeScript vision Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

7 participants