The Wayback Machine - https://web.archive.org/web/20210328034734/https://github.com/microsoft/TypeScript/issues/19139
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

Support @ts-ignore for specific errors #19139

Open
andy-ms opened this issue Oct 12, 2017 · 81 comments
Open

Support @ts-ignore for specific errors #19139

andy-ms opened this issue Oct 12, 2017 · 81 comments

Comments

@andy-ms
Copy link
Contributor

@andy-ms andy-ms commented Oct 12, 2017

TypeScript Version: 2.6.0-dev.20171011

Code

function countDown(n: number): void {
    switch (n) {
        // @ts-ignore
        case 1:
            console.log("1");
            // intentional fall through
        case 0:
            console.log("0");
    }
}

Expected behavior:

Ability to make ts-ignore apply to the --noFallthroughCasesInSwitch error but not to other errors.

Actual behavior:

case "1": would also compile.

@megan-starr9
Copy link

@megan-starr9 megan-starr9 commented Oct 12, 2017

Seconding (and wanting to track this)

This would be super helpful in our case as we have a class with a bunch of variables, and a function that accesses them by key value (vs direct getter function). As a result, they are never called directly within the class and are throwing the error despite being necessary to the functionality. I don't want to have to disable the "noUnusedLocals" check across the board, just in the two classes where this is the case.

Edit -
It's been 2 years and a job change, but I wanted to clarify something bc I'm looking back at old issues and realized this description is slightly unhelpful
My use case for this request was more along the lines of this introduced feature
#33383
But I imagine I did not want to ignore every rule in the file, so wanted to be able to specifically turn off the rule that wasn't preventing working code in an edge case, but we didn't want becoming common practice through the code base

@Ristaaf
Copy link

@Ristaaf Ristaaf commented Nov 28, 2017

I would also love this, more specifically for the noUnusedLocals rule, we get errors on this:

class NumberEvaluator {
   private expression;
   ...
   public evaluate(){
      this[this.expression.type]();
   }

   private BinaryExpression() {
      ...
   }
   ...
}

Saying that BinaryExpression is not used, but it might be, depeding on the value of this.expression.type. I could make BinaryExpresison public, but I really think it is a private method.

@alexburner
Copy link

@alexburner alexburner commented Dec 13, 2017

This would also be very useful for our team, we are slowly migrating a large JS app to TS. We'd like to turn on strict compilation flags (especially --noImplicitAny and --strictNullChecks) but have lots of deviant files that need fixing.

Specific error suppression would allow us to gradually migrate everything to strict compilation.

@stweedie
Copy link

@stweedie stweedie commented Jan 24, 2018

It's actually absurd that this is even still an issue. I've experienced a problem related to this. The underlying cause was due to a typescript change in 2.4, as discussed here:

ag-grid/ag-grid#1708

We were then left with three options

  1. pin typescript to some version < 2.4
  2. update our code to allow updating ag grid to a compliant version
  3. fork our own version of ag-grid until we can upgrade

option 1 is not preferable, as it means we can't leverage any new typescript features and can result in having mismatched dependencies

options 2 and 3 are possible (although difficult), and they really only handle one such specific 'error' preventing successful compilation.

Why is there not an option for us to 'ignore' TS2559 (as an example)?

@Busyrev
Copy link
Contributor

@Busyrev Busyrev commented Feb 15, 2018

@stweedie I made pull request with ability to specify error code to ignore, #21602 but nothing happens,
@andy-ms What you think about?

@thw0rted
Copy link

@thw0rted thw0rted commented Apr 25, 2018

@Ristaaf I know it's been a while, but I think your use case (private but only referenced dynamically) could use the @internal decorator, like /** @internal */ BinaryExpression() { ... }. Technically your method is public so tsc won't complain but it doesn't show up in the docs as being part of your API. (I can't seem to find any docs on @internal but I know it's used extensively in Angular...)

@wosevision
Copy link

@wosevision wosevision commented Jun 13, 2018

Here's another little doozy that I don't believe is covered by any "normal" TS methods. Consider the following:

You're dynamically creating a web worker, i.e. by using an ObjectURL created from a Blob containing the .toString()-ified contents of a function:

const objectUrl = URL.createObjectURL(
  new Blob(
    [`(${
      function() { ... }.toString()
    })()`],
    { type: 'application/javascript' }
  )
);

...and the body of that function contains the single most important thing a web worker can do, postMessage():

// inside worker function
setInterval(() => postMessage('from worker!'), 2000);
                              // ^^^ this causes TS error! 😿

Oh no! TypeScript's implementation of postMessage has a required target parameter in it's function signature. Problem is, we're not using window.postMessage, we're using **self**.postMessage, i.e. the web worker's context! Furthermore, trying to spoof it with a null value makes it worse:

// inside worker function
setInterval(() => postMessage('from worker!', null), 2000);
                              // ^^^ this causes worker exception! ☠️

The web worker postMessage won't accept another parameter! Woe is me.

Since it doesn't make sense to change the [correct] type definition for the window context's interface, and web workers can't use TypeScript [natively], it seems this would be a perfect opportunity to tell the TS compiler "Hey dude, that's a stringified version of some arbitrary Javascript that has nothing to do with you, your execution context even. Back off, get your own sandwich".

@HolgerJeromin
Copy link
Contributor

@HolgerJeromin HolgerJeromin commented Jun 14, 2018

@wosevision
Webworker have a special API which is covered by lib.webworker.d.ts
After adding the target Webworker to your tsconfig you should be able to have a fully typed code like this:
setInterval((this: BroadcastChannel) => this.postMessage('from worker!'), 2000);

disclaimer: I never worked with Webworker till now.
For further question opening a stackoverflow question is probably the best.

@wosevision
Copy link

@wosevision wosevision commented Jun 14, 2018

@HolgerJeromin Thanks, I appreciate the tip. I was aware of the TS webworker lib and target, but this situation is slightly different. The linchpin point is here:

dynamically creating a web worker, i.e. by using an ObjectURL created from a Blob

...That is to say, the target is not a webworker. It's just regular browser JS that happens to be building the webworker. The lib should still reflect the regular lib.dom.d.ts because, for all intents and purposes, we are not inside a webworker and therefore lib.webworker.d.ts typings are useless everywhere else.

It's such an edge case that it would feel silly to do anything other than ignore a line or two.

@jlengrand
Copy link

@jlengrand jlengrand commented Jul 12, 2018

Is there a reason this issue is still open? I depend on a library that has an incorrect documentation, and it causes unsilencable errors!

See esteban-uo/picasa#27 for more info. This feels a bit silly.

@mjomble
Copy link

@mjomble mjomble commented Jul 12, 2018

@jlengrand you could use a generic // @ts-ignore comment.
It already works and ignores all TypeScript errors on the next line.
This issue is only about enhancing the feature to enable targeting more specific errors.

@jlengrand
Copy link

@jlengrand jlengrand commented Jul 13, 2018

Ha nice, I didn't understand that! I saw the ts-ignore issue closed, thinking it had been dismissed. Thanks for the tip!

@centigrade-thomas-becker

Without fine control about how legacy files are handled it is really hard to introduce new typescript rules into a project.

Please give this use case some more consideration.

@ilyakatz
Copy link

@ilyakatz ilyakatz commented Oct 19, 2018

👍

@joebowbeer
Copy link

@joebowbeer joebowbeer commented Jun 9, 2020

@RyanCavanaugh If I had hundreds of ts-ignores in my code, I'd definitely want them to have TScodes associated with them, to restrict their error-hiding radius.

But in the few(?) cases where the.TScode kept changing, I might revert to a simple comment to avoid churn.

The situation seems analogous to eslint, where users are already accustomed to updating their ignores as the error types subtly change. Are ts-ignores fundamentally different from eslint-ignores in this regard?

@vpanta
Copy link

@vpanta vpanta commented Jun 9, 2020

This sounds largely like the need for a perfect solution is preventing a good solution. I can think of multiple responses to your specific argument, but I don't really want to get bogged down in arguing on one specific thing.

Offhand, I've been through multiple migrations where they're much simpler this sort of one-off error squashing. Start with tooling to ignore the current errors so we can move forward writing new error-free code with the codes enabled. Is there a better path to enabling "strict" mode? Or say enabling new error codes in the future?

@falsandtru
Copy link
Contributor

@falsandtru falsandtru commented Jun 9, 2020

Probably this feature isn't implemented because of TS team's test process. TS team frequently tests TypeScript using DefinitelyTyped and some projects but if @ts-ignore for specific errors is used in them, TS team has to maintain these @ts-ignore comments. TS team hates this maintenance cost. This would be why @ts-ignore for specific errors isn't implemented.

FYI, I needs @ts-ignore for specific errors to ignore some language bugs in most cases:

https://github.com/falsandtru/spica/search?q=ts-ignore&unscoped_q=ts-ignore

A lot of bugs have still not been fixed. TypeScript hasn't provided any suitable solution to care these bugs.

@NetOperatorWibby
Copy link

@NetOperatorWibby NetOperatorWibby commented Jun 9, 2020

Would it be possible for the team to ignore ignore comments? 🤔

@doberkofler
Copy link

@doberkofler doberkofler commented Jun 9, 2020

@RyanCavanaugh I completely understand that there is no perfect solution (as @vpanta called it) but nevertheless I would very much prefer the option to only ignore specific errors.
Like in eslint, a user can still decide if he wants to ignore all errors or just specific ones.
I also believe that there most likely should be a reasonable compromise between completely changing all errors codes in each minor revision and not being able to change them at all.
I believe that by trying to rather add new error codes instead of changing existing ones where possible, trying to combine the breaking changes in error codes to bigger releases and just documenting the changes should very much alleviate this problem.

@RyanCavanaugh
Copy link
Member

@RyanCavanaugh RyanCavanaugh commented Jun 9, 2020

Today: "Why let perfect stand in the way of the good? Just do something right away, I'm sure it'll be fine"

Tomorrow: "Yes, that new proposal would be more ideal, but we can't change the behavior now" / "Well why did you rush the feature instead of figuring it out first?"

The day after: "Why is this language so full of inconsistencies? It's like the designers just rushed something out the door to satisfy people. Pretty disappointing that they weren't willing to take their time to do it right."

@doberkofler
Copy link

@doberkofler doberkofler commented Jun 9, 2020

@RyanCavanaugh I would agree with you on most design decisions but I think this is different because I don't think there is a "perfect" solution and silently ignoring possible errors does also not seem like the perfect solution. For this problem I would rather aim for a practical compromise.

@NetOperatorWibby
Copy link

@NetOperatorWibby NetOperatorWibby commented Jun 9, 2020

Today: "Why let perfect stand in the way of the good? Just do something right away, I'm sure it'll be fine"

In all fairness, any decision made today or tomorrow wouldn't be "right away" considering this issue was opened in 2017...

The day after: "Why is this language so full of inconsistencies? It's like the designers just rushed something out the door to satisfy people. Pretty disappointing that they weren't willing to take their time to do it right."

Full? Are there really people here who sound like this?

@cantiero
Copy link

@cantiero cantiero commented Jun 9, 2020

@RyanCavanaugh > there's a 1:1 relationship between error codes and messages.

It seems to me like this is the source of our problems. Is this something that can be changed?

@thw0rted
Copy link

@thw0rted thw0rted commented Jun 10, 2020

Has anybody proposed solving Ryan's problem by maintaining a mapping of "deprecated" / "refined" / "OBE" error codes? (Remember folks, naming things is one of the Hard Problems. Sorry.)

In his example, before TS7632 existed, the code would have been marked with TS2339. So, capture TS2339 -> TS7632 somewhere, once, and in future if there's code with @ts-ignore TS2339 and the compiler spits out TS7632, it walks up the tree and finds an equivalence, so the line is still ignored. There might be cases where this doesn't work out exactly, but it could at least address some of the concern.

@cantiero
Copy link

@cantiero cantiero commented Jun 10, 2020

My proposal is that an error dictionary is created. All errors would have an immutable number and mutable description. Any change in an error description would never break anything. No need for extra maintenance. The text editor would be able to use this dictionary to show a tooltip with the current error description on hover and that is it.

@thw0rted
Copy link

@thw0rted thw0rted commented Jun 10, 2020

Ryan's example above isn't actually a change to an error, it's the introduction of a new, more detailed check that catches a subset of the errors that would previously have been caught. The critical thing is that the hypothetical TS7632 would not mean that TS2339 is completely obsolete. Consider

type Foo = { x?: number; }

type Foo2 = { y?: string; }

const f: Foo = {};
f.y = "hey";
f.z = true;

I assume Ryan is suggesting that, maybe in the future, the compiler will be smart enough to do string-similarity checks and give us a "did you mean?" style error. In that case, the f.y line could be marked with "Did you mean Foo2?", but the f.z line would still get the old error (TS2339) because you're simply referring to an unknown property.

That's why I suggested a mapping that captures this information. The new error message (TS7632) would have shown up as a more general message (TS2339) in a previous TS version, so knowing that means that @ts-ignore TS2339 should also always ignore TS7632. Of course, such a system could also work if a new error message number / string did completely replace an older one.

@cantiero
Copy link

@cantiero cantiero commented Jun 10, 2020

@thw0rted My impression is: if a more recent TS version is really catching a new kind of error, that was previously no cought because of an oversight or language limitation, it should never be suppressed by old error codes being ignored. IMO that is the whole point of this feature rquest.

Your example is not really a new error, it is an augumented way of dealing with an old error. I undersntand that, the way things work now, it would be required to use the new error code TS7632, thecnically making it a new error. But, this is not the true nature of this thing.

Would it make sense to separate them into what is an error and what is a smart suggestion to solve this error? In that case, we would have preserved the original error TS2339, which is not in fact obsolete. And there would be a list of possible smart suggestions for each error code.

@ethanresnick
Copy link
Contributor

@ethanresnick ethanresnick commented Jun 11, 2020

@thw0rted I believe what you’re describing is what I proposed here, though I confess I haven’t followed this thread super closely of late. I think it addresses @RyanCavanaugh’s upgrade issue

@thw0rted
Copy link

@thw0rted thw0rted commented Jun 12, 2020

Pretty much, Ethan. Hah, I even thumbs-up'd the old post back in October! The only difference is that I was imagining storing the "parent" or "alt" information as a tree -- it could be thought of as "ancestry".

I think this still makes the most sense. If all current errors are leaf nodes, then you can walk up to the root of the tree and each visited node is an error that the same line of code would have gotten under some previous version of the compiler. Storing it this way instead of as a flat array might be adding complexity for very little benefit, though.

@nojvek
Copy link
Contributor

@nojvek nojvek commented Sep 9, 2020

@RyanCavanaugh if I understand correctly, your main issue is with the fact that TS could give a different error code for an issue in the future causing upgrade pains? However every TS version is already assumed to be a breaking change, so why not give users the ability to choose how much the ignore radius should be?

AFAIK the proposal is the following

@ts-ignore(?<comment> .*)? - Behavior remains as is Ignores the error on next line, if there is no error, tsc doesn't complain.
@ts-expect-error(?<comment> .*)? - Behavior remains as is. If there is any class of error on next line, error is ignored. If no error, then tsc says Unused '@ts-expect-error' directive.

New proposal @ts-expect-error (TS\d+): (?<comment>.+)? - Expecting a specific error and a reason why it's ignored. If in future there is a different error code, then user has to explicitly acknowledge that and change it.

Basically we're saying, let us decide how strict we want our ignore radius to be. In some projects, going yolo @ts-ignore makes sense, in some projects we want to tightly control ignored errors and slowly work on reducing the number of ignored errors. Having a smaller ignore radius is a big boon.

Every TS version upgrade is already a breaking change. There has been exactly 0 times, where a TS upgrade has just worked for me for our 100k+ lines codebase. So if you're fear is introducing more errors for a version upgrade, this hasn't been a big concern for us personally. Fixing @ts-expect-error issues with a new TS error code is a simple find replace. That's an easy class of errors to fix. I actually want to know when that happens and explicitly fix each one rather than it happening transparently.

Also this issue is something I feel strongly about and would be happy to champion a PR if Typescript core members agree it's something they are willing to accept.

@vegerot
Copy link

@vegerot vegerot commented Sep 9, 2020

This is something that would highly valuable to us. Having a ts-expect-error should always be a last resort, but if it comes to that then I would at least like to limit the scope of it as much as possible.

@RyanCavanaugh
Copy link
Member

@RyanCavanaugh RyanCavanaugh commented Sep 9, 2020

@nojvek These things are all true qualitatively, but there is a quantitative aspect that needs consideration. We totally expect that most very large codebases will see a dozenish breaks during version migrations, and that the maintainers of those codebases look at the breaks and mostly say "Yeah I can see how that was sketchy".

So if you're fear is introducing more errors for a version upgrade, this hasn't been a big concern for us personally

The reason you haven't had this problem is precisely because we've avoided adding features (like this one) that would create that scenario. I don't know what more to say on that; not getting wet in a rainstorm is a bad reason to throw away one's umbrella.

Here's a scenario that is entirely foreseeable:

  • Engineers at a large company with a TypeScript monorepo see this new feature for explicitly ignoring specific errors and "upgrade" their ts-ignores (which were inserted by an automated upgrade tool) to ts-specific-ignores in the name of "Now we know what's going on!"
  • This continues for a few years, and because TS generally issues either "X not assignable to Y" or "Can't call Z" errors at the top-level, those two error codes are everywhere
  • We find a neat way to be more specific in half of these errors, which necessitates a new error message and message number
  • Feedback on TS 5.7 is they're blocked on upgrading because there are so many new errors in this version that it's impossible to tell if anything bad is getting through

You're acknowledging the danger of this feature, saying that you accept the danger, and therefore there's no problem with the feature existing. The problem is, we're not just adding the feature for you, we're adding it for everyone, and there's no way to put a EULA sticker on this that says "FYI, version to version upgrades are totally YOLO if you use this". They're going to see it on Stack Overflow, stick 40 different ts-specific-ignores (which feel "safer" than a blanket ts-ignore) in places where they should used a tactical any instead, get burned on version N+1, and complain about how TS sucks at versioning and broke their app with no warning.

The downstream effects of "I accept the danger" thinking can already be seen with ts-ignore: people misapprehend what ts-ignore does, thinking it will somehow appear in declaration files. Or that they can use it to ignore TS telling them about a problem that will prevent correct emit. ts-ignore is a "last resort" feature and I would really argue that any line of code for which the ignored error is not immediately apparent by syntactic form alone is a red flag. There's little upside IMO to expanding its use or encouraging it to be more brittle unless there are use cases that simply can't be solved other ways.

@nojvek
Copy link
Contributor

@nojvek nojvek commented Sep 9, 2020

Proposal: Having an opt-in way to limit ts-expect-error radius by specifying exactly which error to ignore.

If I understand correctly, the summary of your viewpoint is that TS team thinks that providing specific error code ignores is not a good future-proof solution. It should not be exposed to the user at all. Since it will regress the upgrade experience and TS team doesn't want to paint itself into a corner.

That's a reasonable argument. Ultimately you are the guardians of TS so it's your call (even if some of us may disagree). I appreciate the explicit clarification.

@vegerot
Copy link

@vegerot vegerot commented Sep 10, 2020

@RyanCavanaugh Then why have @ts-expect-error at all? If we don't want people abusing things then why were per-line @ts-ignores added back in 2.6 instead of having only global no-checks?

I admit the second example is a bit contrived; but for my first question, the documentation for ts-expect-error seems to be in direct conflict with the (good) points you were just making.

I feel conflicted, because I agree with the points you're making, but agree with the points made in the 3.9 documentation, and would be valuable to our team.

@RyanCavanaugh how would you respond to these excerpts:

Unfortunately if our tests are written in TypeScript, TypeScript will give us an error! ... That’s why TypeScript 3.9 brings a new feature: // @ts-expect-error comments. When a line is prefixed with a // @ts-expect-error comment, TypeScript will suppress that error from being reported; but if there’s no error, TypeScript will report that // @ts-expect-error wasn’t necessary.

Given this logic--that it's better to say expect-error than ignore in this type of test--then why isn't it EVEN BETTER to specifically say which error you're expecting?

Pick ts-expect-error if:

  • you’re writing test code where you actually want the type system to error on an operation

By adding a specific error code, instead of just blindly ignoring the line, the error can be PART of the test itself.

In addition, many of the points that you bring up (that I agree with) are similar to the ones given in this article for picking ts-ignore:

Pick ts-ignore if:

  • you have an a larger project and and new errors have appeared in code with no clear owner
  • you are in the middle of an upgrade between two different versions of TypeScript, and a line of code errors in one version but not another.
  • you honestly don’t have the time to decide which of these options is better.

You're right that a danger of adding specific errors is that you will use it to squash random errors in a codebase (point 1). Another danger you point out is how this can be abused and bite people in the future when updating (point 2).

From what I gather, having specific error codes amplifies the benefits listed in "Pick ts-expect-error if:", and exacerbates the problems in "Pick ts-ignore if:".

Just wondering what you think about this. Also, I'm not going to go through the initial expect-error PR threads, but during these debates were you on the side of not adding expect-error at all?

@RyanCavanaugh
Copy link
Member

@RyanCavanaugh RyanCavanaugh commented Sep 10, 2020

Also, I'm not going to go through the initial expect-error PR threads, but during these debates were you on the side of not adding expect-error at all?

Consider the fact that ts-ignore doesn't error on a "missing" error, though we could have easily made it do so. This was a very big discussion at the time we added that feature, with very similar characteristics to this one, and ultimately we went in the direction that would minimize upgrade pain. I think adding ts-expect-error before ts-ignore would have been a huge mistake; doing it in the order we did, with a long gap between, cemented ts-ignore as the "default" way to suppress an error, and then later having ts-expect-error meant it was fairly clear that you were opting in to a very specific behavior with reasonably foreseeable caveats.

I don't see the same analogue here, because it's not at all obvious that error codes are not necessarily stable between versions. C# has to do a ton of backcompat engineering to make sure their warnings are consistent version-to-version and we don't want to do that (though people are already speculating about how we should implement it). If we add this feature, we are guaranteed going to stoke ire from engineers who go all-in on it without realizing this, and there's nothing I see that can prevent it.

Maybe that future ire is worth it for the sake of whatever people are experiencing here, but frankly I have not seen the evidence for what kinds of problems people are encountering that this feature would solve better than other existing solutions. People just keep going in circles asking why it doesn't exist.

I get that it feels bad to ts-ignore some line of code that might acquire some additional error in the future, but it's instructive to read the original ts-ignore thread (which we also pushed back on for a long time) where 95% of people said they "needed" the feature for suppressing some particular error that they could have suppressed with a more tactical as any. We added ts-ignore with the intent that it be used for the remaining 5% that can't be suppressed by any existing type system mechanics; in this light I think the concern about oversuppression is somewhat misguided because there should be very very very few ts-ignores in your codebase, and because so few constructs require a ts-ignore, it should be very unlikely that they acquire additional errors later. I can't reason people out of their worries but we have to weigh "dev X will sleep better at night" vs "dev Y is facing an error cliff during upgrades because dev Z told them that they should always ignore specific error codes instead of using as any"

@doberkofler
Copy link

@doberkofler doberkofler commented Sep 10, 2020

@RyanCavanaugh As a possible use case for this feature. I'm responsible for a pretty large (~100000 loc) project. During our incremental migration from JavaScript to TypeScript we had to use ts-ignore quite a bit and sometimes (by ignoring a complete line) we missed a problem that was accidentally hidden because ts-ignore ignores all errors. The option to restrict ts-ignore to the specific error that "must" be ignore could have prevented quite a few problems during the migration.

@vegerot
Copy link

@vegerot vegerot commented Sep 10, 2020

@RyanCavanaugh Very concisely written, and as someone that is only just thinking about this problem, you illustrated the history of these features in a way that really helped me understand where you're coming from. Thanks!

@vegerot
Copy link

@vegerot vegerot commented Sep 22, 2020

@denissabramovs That's not very helpful. Check out the thread starting here for some rationale

@macdaj
Copy link

@macdaj macdaj commented Dec 11, 2020

@RyanCavanaugh
I get the perspective of backwards compatibility and maintenance of old code being a burden.

In my specific case I'm running into a problem related to immer losing symbol keys which I believe is caused by keyof removing well known symbols. For this case I have to use a ts-ignore statement. I think adding some compiler options would really help because ideally I want to remove these ts-ignore lines once they are no longer needed, or in a perfect world pay attention to different ts exceptions that pop up on the ignored line.

I think for ts-ignore having an explicit opt-in typescript compiler option that would throw an error if there is no typescript error on the line would be useful.

I think the same approach where specific error ts-expect-error-#### statements would fall back to the general ts-ignore would work. You can have a compiler option which specifies whether you want the fallback (by default it is true), effectively making ts-expect-error-#### be an explicit opt-in.

Even if you don't opt into either of these behaviors you have the ability to periodically run a build with these rather than checking each line manually.

@CrimsonCodes0
Copy link

@CrimsonCodes0 CrimsonCodes0 commented Jan 14, 2021

Personally, I find entire TSC errors to be, without explicit rationale behind them, 100% useless.
Consider TS18022, why is that even an error?
Or TS2775.
Or TS18019.
I legitimately had made a class that used a static, private class method, that asserted and determined whether or not something was genuinely from that class.

Oops, I broke at least 3 TS rules at once :)
No static private, no private methods, no assertion object methods.
None of these are poor coding practice, they aren't inherently unsafe, they can be statically analyzed, but they are denied by TS, which prevented me from porting my JS to TS.

There are plenty of errors that are just that, errors, they should make someone stop and think about whether what they had just written was unsafe, and rewrite it, but the "errors" that I would like to disable en masse fall into a whole different category outside of safety and linting, into the area of "why is that an error in the first place?"

@thw0rted
Copy link

@thw0rted thw0rted commented Jan 14, 2021

For both TS18022 and TS18019, your linked issue points out that private methods only reached "Stage 3" quite recently and TS hasn't gotten around to supporting them yet. Likewise for other up-and-coming ES private features (static #foo, etc). The good news is that TS legend and all-around swell guy @dragomirtitian says in #39066 that support is inbound soon.

For TS2775, the linked issue (and a PR that is linked several times from there) make it pretty clear that there's a design limitation for control-flow analysis where you have to basically "re-type" the assert function when it's imported. (I agree with what appears to be the general consensus that the error message could be clearer, but at least searching for that error number brings you to those discussions.) This is irksome but not insurmountable.

@CrimsonCodes0
Copy link

@CrimsonCodes0 CrimsonCodes0 commented Jan 14, 2021

For TS2775, the linked issue (and a PR that is linked several times from there) make it pretty clear that there's a design limitation for control-flow analysis where you have to basically "re-type" the assert function when it's imported. (I agree with what appears to be the general consensus that the error message could be clearer, but at least searching for that error number brings you to those discussions.) This is irksome but not insurmountable.

I think I'm experiencing a different error now that I look at it; I already know that reassigning an assertion function requires an explicit type, but what about direct calls?

let azzert = new Assertion().assert;
azzert(foo);

vs

new Assertion().assert(foo);

my particular case looked more like:

class T {
    static #assertIsT(x: unknown): asserts x is T { ... } // error

    method(x: unknown) {
        T.#assertIsT(x); // error
        ...
    }
}

but the real problem here is the lack of static private support and is the only reason that it can't be typed correctly.

For both TS18022 and TS18019, your linked issue points out that private methods only reached "Stage 3" quite recently

Well... those issues have actually been open for quite some time, 10 and 11 months respectively.
That means in a month or two, private methods and private statics will have been at stage 3 for at least a year.

But, in reality, the numbers are a bit different:

Proposal name Date that it reached stage 3
proposal private methods Sepetember 2017
proposal static class features May 2018
proposal class fields July 2017

An average of three years since they reached stage 3; these are bound to reach stage 4 soon.
And, to top it off, Chrome already ships a full implementation, by default (e.g. no experimental flags).

Also, there are errors like TS2376 and TS17009, which seem... like they fulfill the same role, why does one exist, if the other does?

class T extends H {
     foo = ...;

    constructor() {
        const x = bar();
        super(x, x); // TS#### : "don't do that"
        // do I do this? // super( bar(), bar() );
    }
}
@soullivaneuh
Copy link

@soullivaneuh soullivaneuh commented Jan 31, 2021

I am not a Typescript expert, so I'll not add arguments of the previous debate.

However, I would like to express the need of this granularity with an example.

I use react-navigation that allows us to declare routes with Typescript:

export type MyStackParamList = {
  Index: undefined;
  Show: {
    id?: string;
  };
  Edit: {
    id: string;
  };
};
const Stack = createStackNavigator<RidesStackParamList>();

You may define multiple stacks like that, but I will not going to details here.

Using the navigation to change the current route is supposed to be limited to the current stack, so if I do:

navigation.navigate('Payment');

Typescript will throw error TS2345 because the Payment parameter does not match the related MyStackParamList but an another stack.

However, this code part works like a charm. You can navigate to an another route of an another stack without issue

Is it me wrongly using the library? Maybe, surely. It's classified.

However, supposing it's an issue related to the vendor type definition, I can do anything waiting the fix than ignoring the error.

But as @andy-ms said, I would like to be sure only this error will be silenced and avoid possible other mistakes on the same line.

If specifying errors to ignore is dangerous according to some people here, what are the alternative for this case?

Thanks for reading

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet