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

Let ts-ignore ignore suggestions #27979

Closed
wants to merge 1 commit into from
Closed

Conversation

sandersn
Copy link
Member

@sandersn sandersn commented Oct 18, 2018

Having checkJs on a JS file can potentially create many suggestions. One of the suggested codefixes is "add // @ts-ignore", but this does nothing. Making ts-ignore work with suggestions is one fix.

Two others are

  1. Do not suggest ts-ignore for suggestions.
  2. Allow (VS Code?) users to selectively turn off specific errors.

Of course, there is

  1. Do nothing. The small suggestion … isn't that intrusive and (currently) is explicitly requested by checkJs users, who will not be bothered by the bogus ts-ignore suggestion.

@sandersn
Copy link
Member Author

Not suggesting ts-ignore looks pretty hard; I followed the trail back to CodeFixRequestArgs in protocol.ts; it only provides a list of errorCodes, but not whether they originated from a suggestion or an error.

@sandersn
Copy link
Member Author

Yep, after consulting with @sheetalkamat, it looks like changing CodeFixRequestArgs requires a change in VS Code as well. I'll bring this up next time we meet.

@ghost
Copy link

ghost commented Oct 19, 2018

This should probably wait on #19139. We don't want people adding // @ts-ignore to disable a suggestion and missing out on actual errors.

@sandersn
Copy link
Member Author

I agree; in any case I think not suggesting ts-ignore is the better option since preliminary evidence [1] suggests that the noImplicitAny suggestions are not that obtrusive.

[1] that is, talking to @uniqueiniquity, who has been using the new suggestions.

@sandersn sandersn closed this Oct 22, 2018
@jakebailey jakebailey deleted the tsignore-suggestions branch November 7, 2022 17:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant