Skip to content

TSX @ts-ignore comments no longer suppressing errors in TypeScript 3.9 #37738

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

Closed
brieb opened this issue Apr 1, 2020 · 11 comments · Fixed by #38228
Closed

TSX @ts-ignore comments no longer suppressing errors in TypeScript 3.9 #37738

brieb opened this issue Apr 1, 2020 · 11 comments · Fixed by #38228
Labels
Bug A bug in TypeScript Domain: JSX/TSX Relates to the JSX parser and emitter Fixed A PR has been merged for this issue

Comments

@brieb
Copy link

brieb commented Apr 1, 2020

TypeScript Version: 3.9.0-beta and 3.9.0-dev.20200330

Search Terms:

ts-ignore 3.9 tsx @ts-ignore

Code

      {/*
      // @ts-ignore */}
      <MyOtherComponent prop={123} />

Expected behavior:

Error suppressed when running tsc (TS 3.8 behavior)

Actual behavior:

Error emitted when running tsc

Repro Repo Link:

https://github.com/brieb/ts-39-issue-ts-ignore-tsx

@brieb
Copy link
Author

brieb commented Apr 1, 2020

I was trying to help beta test TypeScript 3.9 and we have some @ts-ignores in our TSX code that are no longer being respected, so we aren't able to get a passing build to assess performance improvements. Let us know if there is a workaround for this or a new build we should try out. So excited for all the new TypeScript 3.9 features! 😄
cc @DanielRosenwasser

@mohsen1
Copy link
Contributor

mohsen1 commented Apr 1, 2020

line comments still work:

        <Comp
            // @ts-ignore
            badProp={1}
        />
         <Comp
            // @ts-expect-error
            badProp={1}
        />

cc @JoshuaKGoldberg

Playground

@brieb
Copy link
Author

brieb commented Apr 1, 2020

line comments still work

Definitely true, though is it always possible to express @ts-ignore as a line comment in TSX?
I'm trying to think of a counter example.

I suppose this is also an option if the error is on the component

    <
    // @ts-ignore
    Comp />

Though that breaks syntax highlighting in vscode and prettier goes out of whack.

My guess is that this was an unintentional breakage.

@JoshuaKGoldberg
Copy link
Contributor

Indeed, this was an unintentional breakage. Specifically the breakage was here: https://github.com/microsoft/TypeScript/pull/36014/files#diff-08a3cc4f1f9a51dbb468c2810f5229d3L1782
Sorry for bungling up your TS 3.9 usage @brieb! 😅

TypeScript used to check if a line looked like // @ts-ignore on its own, regardless of whether the line was part of a multiline comment. I suspect that itself was an unintentional side effect of how it was checking for comments, and just happened to work well in the .jsx/.tsx use case?

TypeScript now understands where single-line // @ts-(expect-error|ignore) directives are, and only ignores comments after any of them. I suppose a good fix would be to also understand the last line of multiline comments when that line includes a comment directive. E.g. /* \n // @ts-ignore */.

..but I'll defer to the Typescript team. Happy to send a PR if that or another strategy is approved!

@brieb
Copy link
Author

brieb commented Apr 1, 2020

No worries, at all! Glad we could help catch this. We're so thankful for all the great work going into the TS 3.9 release!! 😃

Ya, the format is not ideal. But, that's the best that we came up with here #27552

Looks like there has been some follow-up requests about a cleaner syntax for this #31147

{/* @ts-ignore */} was my preference in the original issue as well.

Though I'm not sure how much scope it would add to accept that format vs fixing up the good 'ol

{/* 
// @ts-ignore */}

@DanielRosenwasser
Copy link
Member

My intuition tells me that if @ts-ignore appears in the last line of any comment, the following line's errors should be suppressed.

@JoshuaKGoldberg
Copy link
Contributor

Swell. Just to be clear @DanielRosenwasser, is your intuition strong enough for TypeScript to accept a PR making that behavior change?

@JoshuaKGoldberg
Copy link
Contributor

Ping, sorry to re-tag you @DanielRosenwasser - since TS 3.9 is close to release, is there a way this could get triaged? I would very much like to fix this regression before the major release. 🤗

@DanielRosenwasser DanielRosenwasser added Bug A bug in TypeScript Domain: JSX/TSX Relates to the JSX parser and emitter labels Apr 28, 2020
@DanielRosenwasser DanielRosenwasser added this to the TypeScript 3.9.2 milestone Apr 28, 2020
@DanielRosenwasser
Copy link
Member

DanielRosenwasser commented Apr 28, 2020

I'm so sorry this didn't get triaged and fell off my radar. If you can get a PR out, we can work on through it. Ideally this would go in before the RC, but I don't think we'll have time for that.

@JoshuaKGoldberg
Copy link
Contributor

Swell, thanks! 🚀 #38228 minimally adds directive parsing for multiline comments.

@DanielRosenwasser
Copy link
Member

Thanks to everyone involved on this thread for your patience, and thanks @JoshuaKGoldberg for the quick fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Domain: JSX/TSX Relates to the JSX parser and emitter Fixed A PR has been merged for this issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants