Add @ts-ignore-start and @ts-ignore-end directives

[mohsen1]

History and rationale

This is a request from the community for about 3 years now. The most concrete answer that came from the TypeScript is @weswigham's comment in #19573 (comment):

I cannot, for the life of me, come up with a compelling, principled example where specific ranges of code should be allowed to be unchecked (unlike files, which had a clear migration-strategy-backed reasoning), other than "writing multiple consecutive suppressions makes my eyes bleed"...

I want us to be more sympathetic with the needs of the community. Casting to any might be a great solution but if users don't want to do it they might have a reason for that.

• For instance, in my example imagine foo being typed with only {prop1: string; prop2: string}. I can technically cast my argument's type to any to add more props but that will remove type checking for prop1 and prop2.
• There could be teams that have strong feelings around casting to any but take it easier with @ts-ignore's. Maybe they have processes in place to address comment directives later...
• In some cases users mentioned they want to @ts-ignore a few imports. Casting to any won't work there.

I'm sure there are other cases that @ts-nocheck or casting to any won't fit the bill. Let's give the community what it is asking for!

A full example

Imagine a library that is wrongly type. foo accepts either a string or an object.

interface ThirdPartyLib {
   /** @param arg can also be an object but not typed */ 
   callFn(foo: string): void;
}

Also consider that scope of my my work doesn't allow me to change the function signiture. But in my changes I need to pass a large object to foo:

thirdPartyLib.callFn({
  prop1: 'abc',
  prop2: 'efg',
  // ...
  propN: 'xyx'
});

To get past type errors I have to put in n @ts-ignore or @ts-expect-error directives :

// @ts-ignore
thirdPartyLib.callFn({
  // @ts-ignore
  prop1: 'abc',
  // @ts-ignore
  prop2: 'efg',
  // ...
  // @ts-ignore
  propN: 'xyx'
});

This change allows a more gentle way of going around this issue:

// @ts-ignore-start
thirdPartyLib.callFn({
  prop1: 'abc',
  prop2: 'efg',
  // ...
  propN: 'xyx'
});
// @ts-ignore-end

Implementation Notes

A @ts-ignore-start without a matching @ts-ignore-end will ignore type-errors up to end of the file

This made sense to me. Initially I had @ts-ignore-start without a matching @ts-ignore-end being an error like an unnecessary @ts-expect-error. It was a poor UX because the moment you start using these directives you get errors telling you the first directive you put in is an error

A @ts-ignore-end without a matching @ts-ignore-start is an error

For obvious reasons. This is probably due to a mistake

Multiple @ts-ignore-start is fine

@ts-ignore-start
// some code
@ts-ignore-start
@ts-ignore-end

Let me know if this should be an error. I feel this is more forgiving and user friendly.

To do

This is still work in progress but I'm open for feedback. While I'm working on these items, please let me know what I should fix!

• Add more conformance tests
• Make sure tests pass in CI
• Add autocomplete suggestion

Fixes #19573

[typescript-bot]

The TypeScript team hasn't accepted the linked issue #19573. If you can get it accepted, this PR will have a better chance of being reviewed.

[weswigham]

Suppressions are a hack for hacking your way out of issues a cast won't fix (like weird inheritance issues). But a cast will fix almost any issue (even those issues if you're creative with where you cast and to what) - every example you have here can be fixed with a single any cast (on the receiver, library itself, or argument), which is much better handled in the language. In other cases, where you have a massive chunk of unchecked code, the per-file directive is fine. If a third party lib has the wrong type, you cast to the correct type or any, up to you, a suppression is completely incorrect and the wrong solution to the problem.

Honestly, we thought suppressions were a bad idea to begin with and I had to argue they had uses when migrating JS projects to TS (essentially as automatable TODOs since per issue suppressions are easy to place automatically). But some people turn to them rather than the much simpler cast, and seeing the arguments here just reinforces that - suppressions aren't "better" than a cast - they're much much worse, and they leave the compiler in a silent error state where things may not behave as expected. They are an absolute last resort, and honestly the presence of a suppression should itself be an error, to indicate that the project has unchecked code that may have unintended cascading effects on downstream code (like quietly leaking an any that isn't caught by noImplicitAny).

If anyone is left thinking a suppression like this is their only possible solution, then we've failed in our documentation somewhere - even in JS we support jsdoc casts now, so you can cast out of almost any issues in JS, too. (Even missing imports can be fixed with declare module declarations)

[knownasilya]

I just tried to use this syntax without know it was a thing, and then I found this PR. So I'd say the syntax is intuitive to say the least.

[HolgerJeromin]

I just tried to use this syntax without know it was a thing, and then I found this PR. So I'd say the syntax is intuitive to say the least.

Not everything which is intuitive is a good thing. I think that is was weswigham is trying to say.
I moved (after reading his comment) most of my unit test codebase from @ts-ignore to as unknown as string (for tests with wrong function parameters) and found some unwanted wrong function parameters.

Typescript does not want to give you a bigger gun to shoot yourself in your foot.

[knownasilya]

My use-case is a new experimental syntax for EmberJS which uses new syntax that uses in scope variables, but is not traditional javascript since it's transformed during build time (kinda like jsx). So things like imports or variables used in this space are marked as unused, hence grouping all of those and using a group comment to disable "errors".

https://github.com/chriskrycho/ember-rfcs/blob/fcct/text/0779-first-class-component-templates.md#class-body

[LynnKirby]

There's a conflict from there being two kinds of errors that typically get suppressed: code quality and type checking.

Code quality errors are produced by settings like noUnusedLocals. These errors don't always mean that your code is wrong so suppressing them can be justified since they don't affect the correctness of your program. Adding ranged suppressions can be useful in case there are many errors, as in @knownasilya's use case.

Type errors are different. Ignoring these errors isn't a magic "make my code work" button. Ignoring them can have unintended consequences on how the rest of your code is checked. They should never be ignored unless there's absolutely no other way of getting around the problem (and I've never seen an example of this). There's no valid use case for range suppressions of type checking errors; we should encourage people to fix their types and not ignore errors.

This dilemma should be resolved in favour of not adding ranged suppressions. The TSC diagnostics system is basically unconfigurable and it's impossible to ignore specific kinds of errors (which is fine for type checking but less for linting). If you need more control over code quality diagnostics than what TSC provides you can use something like ESLint.

[n10000k]

Suppressions are a hack for hacking your way out of issues a cast won't fix (like weird inheritance issues). But a cast will fix almost any issue (even those issues if you're creative with where you cast and to what) - every example you have here can be fixed with a single any cast (on the receiver, library itself, or argument), which is much better handled in the language. In other cases, where you have a massive chunk of unchecked code, the per-file directive is fine. If a third party lib has the wrong type, you cast to the correct type or any, up to you, a suppression is completely incorrect and the wrong solution to the problem.

Honestly, we thought suppressions were a bad idea to begin with and I had to argue they had uses when migrating JS projects to TS (essentially as automatable TODOs since per issue suppressions are easy to place automatically). But some people turn to them rather than the much simpler cast, and seeing the arguments here just reinforces that - suppressions aren't "better" than a cast - they're much much worse, and they leave the compiler in a silent error state where things may not behave as expected. They are an absolute last resort, and honestly the presence of a suppression should itself be an error, to indicate that the project has unchecked code that may have unintended cascading effects on downstream code (like quietly leaking an any that isn't caught by noImplicitAny).

If anyone is left thinking a suppression like this is their only possible solution, then we've failed in our documentation somewhere - even in JS we support jsdoc casts now, so you can cast out of almost any issues in JS, too. (Even missing imports can be fixed with declare module declarations)

Then why do we have @ts-ignore? The same case goes, there's already a few use cases over in the issue linked in the OP. I understand the point that we want everything to be strict and casted correctly, however if we have options such as ts-ignore already, aren't we are already doing that suppression option anyway? what harm does this really have at the end of the day? If people are adding @ts-ignore already, surely we want to enhance the developers experience?

I do agree suppressions are bad and we shouldn't supress before casts. However.. there's a use case for developers during debugging, migration etc.

[mohsen1]

In this example casting to any will lose some of type safety.

interface ThirdPartyLib {
   /** @param arg can also be an object but not typed */ 
   callFn(foo: { prop1: string; prop1: string; }): void;
}

// casting to any
thirdPartyLib.callFn({
  prop1: 123,  // this is not checked anymore
  prop2: 'efg',
  // ...
  propN: 'xyx'
} as any);

// ignore block
thirdPartyLib.callFn({
  prop1: 'abc', // these lines are checked now.
  prop2: 'efg',
  // @ts-ignore-start
  // ...
  propN: 'xyx'
  // @ts-ignore-end
});

I can do some inference gymnastic to do better (callFn({ ...({prop1: 'a', prop2: 'b'} as ArgsType), ...rest)) but this sort of situation is very common in big migrations.

---

This was fun to hack on and I learned something working on it. It's up to the core team to decide at the end of the day.

[sandersn]

I don't like this either.

I don't mind having an open issue for discussion pro and con, but I'm going to close the PR since the compiler code changes all the time and PRs get stale pretty quickly.

[n10000k]

I don't like this either.

I don't mind having an open issue for discussion pro and con, but I'm going to close the PR since the compiler code changes all the time and PRs get stale pretty quickly.

The discussion has been going on for years, I think someone at Microsoft needs to go over the discussion to offer a solution or feedback to the request (there's been enough feedback). It can't be another 4+ years of an open issue.

[weswigham]

I mean, we don't outright decline most feature requests that don't violate the project's guiding principles, so discussions of features we don't really like can stay open, even if we don't currently want to accept the feature. After all, there's always the possibility that some super compelling justification for the feature appears, or that the discussion informs some other feature we create.

[andyslack]

It's showing as closed but I still cannot use

// @ts-ignore-start and // @ts-ignore-end

Is there an alternative?