-
Notifications
You must be signed in to change notification settings - Fork 12.2k
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
Comments
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 - |
I would also love this, more specifically for the noUnusedLocals rule, we get errors on this:
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. |
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 Specific error suppression would allow us to gradually migrate everything to strict compilation. |
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: We were then left with three options
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)? |
@Ristaaf I know it's been a while, but I think your use case ( |
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 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, // inside worker function
setInterval(() => postMessage('from worker!'), 2000);
// ^^^ this causes TS error! 😿 Oh no! TypeScript's implementation of // 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". |
@wosevision disclaimer: I never worked with Webworker till now. |
@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:
...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. |
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. |
@jlengrand you could use a generic |
Ha nice, I didn't understand that! I saw the ts-ignore issue closed, thinking it had been dismissed. Thanks for the tip! |
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. |
👍 |
I don't think this feature would help, then, as errors aren't that granular. |
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. |
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 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. |
I’m not sure I perfectly followed what you were saying here. But generally isn’t the idea to export a type a named type like:export typ e myType = number | Array<number> | undefinedOr, whatever you need it to be. then use that type as the type for your optional variable type wherever that combo of types is needed so that you aren’t copying and pasting the long type definition. I don’t think that is type magic as much as it is clearly describing the range of options for your optional variable. And if you name it like OptionalNonString then that would be more meaningful for review’s sake wouldn’t it?I apologize if I misunderstood what you were saying. Im not sure what you are gaining from an override in this case. Sent from my iPhoneOn Mar 30, 2023, at 10:44 PM, Árpád Magosányi ***@***.***> wrote:
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.
—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: ***@***.***>
|
when will they add this feature |
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 |
Any plan for this to be implemented? |
when tsc reports a single error, can ask ignore a single error only? e.g.
|
Multiple errors. Maybe case insensitive too.
Right now, this is a pipe dream. It's all or nothing, folks. I like using |
Or even better we can have // @ts-expect-error TS2445 ts1942 Ts552 |
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 |
@minecrawler It's, IMO, often preferable over using Having the 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 |
Exactly as described above, using |
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. 🙏 |
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. |
@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 |
Is there any work being done on this? |
@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. |
@doberkofler there's so much to read through, could you please show which comments do you mean? I'd like to argue against them. |
#22011 here I found "no conclusion yet", so I suspect there was sth further? |
@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. |
TypeScript Version: 2.6.0-dev.20171011
Code
Expected behavior:
Ability to make
ts-ignore
apply to the--noFallthroughCasesInSwitch
error but not to other errors.Actual behavior:
case "1":
would also compile.The text was updated successfully, but these errors were encountered: