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
ghost opened this issue Oct 12, 2017 · 128 comments
Open

Support @ts-ignore for specific errors #19139

ghost opened this issue Oct 12, 2017 · 128 comments
Labels
Revisit An issue worth coming back to Suggestion An idea for TypeScript

Comments

@ghost
Copy link

ghost 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 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 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 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

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

@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

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

@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

@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

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

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

@ghost
Copy link

ghost commented Sep 13, 2018

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

👍

@nicholasbailey
Copy link

I'm not trying to be overly pedantic here, but Typescript isn't a "linter", it's a type system. I asked for an example because, when you ts-ignore an error, 9 times out of 10 what you're doing under the hood is basically an any-cast -- you tried to do something that the type system doesn't allow, and by ignoring the error and doing it anyway you're telling TS not to enforce types on that line.

That's exactly correct. But Typescript is also an incremental type system. It is explicitly designed to allow gradual migration from Javascript. If you've ever tried to migrate a 500k line plus codebase to Typescript, you've probably noticed that you can't do it all in one go. You have to work incrementally, which means you have to allow any into your codebase. It makes a great deal of sense to allow developers to make specific type violations in narrow contexts. ts-ignore-error and friends are intended for use in hybrid codebases as an aid in migration. Allowing them to target specific errors would better facilitate that purpose.

@fatcerberus
Copy link

I only want to ignore strict null checks in a tiny number of cases where they pose a problem. If the type is number | number[] | undefined, I still want it to throw an error when someone passes a string.

I don't think this feature would help, then, as errors aren't that granular. undefined is not assignable to number is the same error as string is not assignable to number. If you're specifically trying to bypass a null check, just write foo!.

@magwas
Copy link

magwas commented Mar 31, 2023

I'm not trying to be overly pedantic here, but Typescript isn't a "linter", it's a type system. I asked for an example because, when you ts-ignore an error, 9 times out of 10 what you're doing under the hood is basically an any-cast -- you tried to do something that the type system doesn't allow, and by ignoring the error and doing it anyway you're telling TS not to enforce types on that line.

No difference between the two on high level: both enforces a particular set of rules against the source code. The type system is just specifically enforces that the input and output value constraints should be explicitly spelled out.

@magwas
Copy link

magwas commented Mar 31, 2023

Although I agree with those saying that TS should support migration from legacy code, here is a use case with code which never have been JS, and written with all type enforcements turned on:

It is a library to support building things using the fluent pattern. Therefore the type for this contains quite a lot of optional fields which are meant to filled in gradually. And there is one field which will be reset to undefined in one special case. With exactOptionalPropertyTypes turned on, that special case is flagged as an error.
I could add |undefined to the field, but I want to ensure that this particular operation is done in exactly one place in the whole codebase.

And yes, I could still add some type magic to that particular function to allow it there specifically.

But from the ease of review standpoint it is much easier to just automatically mark all comments as something needing discussion than put the type magic there, and detect/prevent cut&pastes/reuses of that.

@arthur-clifford
Copy link

arthur-clifford commented Mar 31, 2023 via email

@SiddHyper
Copy link

when will they add this feature

@typeholes
Copy link

I'll be adding this check to my extension, but the release timeline is somewhere between a long time from now and never so you can use ts-morph and something like this code if you really want to check that casts are handling specific error codes

function checkIgnoreTSC(project: Project) {
   project.getSourceFiles().forEach((file) => {
      console.log(`checking ignore TSC for file ${file.getFilePath()}`);

      file.getDescendantsOfKind(SyntaxKind.AsExpression).forEach((expr) => {
         const castText = expr.getTypeNode()?.getText() ?? 'no type node';
         const holdText = expr.getFullText();
         const exprText = expr.getExpression().getFullText();
         console.log(castText);
         const exprLineNumber = expr.getEndLineNumber();
         if (castText.match(/^ignore_TSC[0-9]+$/)) {
            const code = castText.slice(10);
            const replacedWith = expr.replaceWithText(
               `${exprText} /*${castText}*/`
            );

            const err = file
               .getPreEmitDiagnostics()
               .find(
                  (diagnostic) => diagnostic.getLineNumber() === exprLineNumber
               );
            if (err === undefined) {
               console.log(
                  `No error found at line: ${exprLineNumber} ${castText})`
               );
            } else {
               const errCode = err.getCode().toString();
               if (errCode !== code) {
                  console.log(
                     `Incorrect error ${errCode} vs ${code} at line: ${exprLineNumber} ${castText})`
                  );
               }
            }
            replacedWith.replaceWithText(holdText);
         }
      });
   });
}
type ignore_TSC1234 = any;
type ignore_TSC2322 = any;

const no_error: number = 1 as ignore_TSC1234;

const should_mismatch: string = 1 as ignore_TSC1234;

const should_match: string = 1 as ignore_TSC2322;
 
ignore_TSC1234
No error found at line: 4 ignore_TSC1234)
ignore_TSC1234
Incorrect error 2322 vs 1234 at line: 6 ignore_TSC1234)
ignore_TSC2322

@xsjcTony
Copy link

Any plan for this to be implemented?

@tx0c
Copy link

tx0c commented Sep 14, 2023

error TS2445 ....

when tsc reports a single error, can ask ignore a single error only? e.g.

// @ts-ignore TS2445

@ADTC
Copy link

ADTC commented Sep 15, 2023

Multiple errors. Maybe case insensitive too.

// @ts-ignore TS2445 ts1942 Ts552

Right now, this is a pipe dream. It's all or nothing, folks.

I like using @ts-expect-error though. At least if the line no longer has an error, this throws an error and we have to remove this comment and clean up the code. :D

@xsjcTony
Copy link

Or even better we can have

// @ts-expect-error TS2445 ts1942 Ts552

@minecrawler
Copy link

how would expect-error work? There needs to be an error else it doesn't build? That sounds dangerous, do you have a use-case? I'm not sure if that wouldn't be out of scope of the original request

@liamjones
Copy link

@minecrawler @ts-expect-error is already something that exists, it just can't be targeted to specific codes currently.

It's, IMO, often preferable over using @ts-ignore when you encounter something that you have to work around for now but should be fixed in future TS/dependency versions as both directives have some level of danger (being a global error ignore for the subsequent line).

Having the @ts-expect-error error once the problem is fixed (by seeing Unused '@ts-expect-error' directive.(2578) later) means you get rid of the dangerous error silencing behaviour in your codebase sooner.

Contrived but simple example:

// v1 of some module I depend on - their function is typed wrong, it says it takes string not number.
// Assume these two calls are in different parts of a big application:
someFunctionFromAModule(4) // Argument of type 'number' is not assignable to parameter of type 'string'.(2345)
someFunctionFromAModule(4) // Argument of type 'number' is not assignable to parameter of type 'string'.(2345)

// So, we add ts-ignore/ts-expect-error and report the issue to the project owners
// @ts-expect-error types wrong
someFunctionFromAModule(4) // No error
// @ts-ignore types wrong
someFunctionFromAModule(4) // No error

// Upgrade to v2 and they fixed the typing issue, we get prompted about the ts-expect-error no longer applying but not the ts-ignore
// @ts-expect-error types wrong
someFunctionFromAModule(4) // Unused '@ts-expect-error' directive.(2578)
// @ts-ignore types wrong
someFunctionFromAModule(4) // No error

// So we remove the ts-expect-error but forgot about the ts-ignore elsewhere in the codebase
someFunctionFromAModule(4) // No error
// @ts-ignore types wrong
someFunctionFromAModule(4) // No error

// Later we upgrade to v3 of the module, but didn't notice they'd renamed this function
// Since we were prompted to remove the ts-expect-error when our original issue was cleared we have now been prompted that this function doesn't exist anymore
someFunctionFromAModule(4) // Cannot find name 'someFunctionFromAModule'.(2304)
// But, because this one still has a ts-ignore on it we forgot about our code just dies when it reaches this point
// @ts-ignore types wrong
someFunctionFromAModule(4) // No error

Yes, unit tests, e2e testing etc could also pick up the second call's problem but the choice of @ts-expect-error over @ts-ignore allows us to pick up the issue earlier in the process.

@xsjcTony
Copy link

Exactly as described above, using @ts-ignore generally considered harmful to the codebase, where you have no idea whether the suppressed line errored because of some unnoticed updates

@gautelo
Copy link

gautelo commented Sep 24, 2023

Would love to see this. I was actually a bit surprised that there wasn't already support for this.

There is prior art to use as inspiration in eslint. Things like this:

// eslint-disable-next-line @typescript-eslint/no-unused-vars
const unusedVar = 'hello';

Would love to see this implemented for @ts-expect-error and @ts-ignore. 🙏

@ehoops-cz
Copy link

We did (most of) a typescript migration and are now going through a major refactor. We would really like to enable strictNullChecks and noImplicitAny to ensure the quality of our refactored code going forward. However, our codebase has ~1500 errors for each rule and we aren't going to be able to fix all of them immediately. It's worrisome to completely ignore 3000 lines of code and we would much prefer to ignore only these specific errors.

Please consider adding this feature.

@tylerlaprade
Copy link

@ehoops-cz, I would also like to see this feature added, but I don't expect it any time soon. In the meantime, you should use a ratchet

@wscourge
Copy link

Is there any work being done on this?

@doberkofler
Copy link

@wscourge I have been using TypeScript for years and it is the single most annoying problem that bites us again and again. Unfortunately I don't think this will ever happen given all the comments by members of the TypeScript team.

@wscourge
Copy link

wscourge commented Nov 30, 2023

@doberkofler there's so much to read through, could you please show which comments do you mean? I'd like to argue against them.

@wscourge
Copy link

#22011 here I found "no conclusion yet", so I suspect there was sth further?

@sarimarton
Copy link

sarimarton commented Nov 30, 2023

@wscourge I clearly remember that they argued with something like that the TS error codes are assigned to their message text, not the actual technical error. I.e., if they change the text of one error, they are gonna assign a new TS-number to that, leaving the previous number unused. However silly this sounds, this, according to them, ensures some kind of googling consistency, and hence, the numbers aren't meaningful representation of the errors, hence, they don't want to encourage us to rely on it.

@doberkofler
Copy link

@wscourge Have a look at the PR #21602 and SR #19139

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Revisit An issue worth coming back to Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests