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

Respect //@ts-nocheck in TS files #33383

Merged
merged 1 commit into from
Sep 27, 2019

Conversation

weswigham
Copy link
Member

Fixes parts of #19573 kinda.

Seeing as we already support //@ts-ignore, //@ts-nocheck is really just a file-scoped ignore, so there's nothing new here, really. It's also not really a big change.

}


//// [DtsFileErrors]
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just decided to have declaration emit on to capture the true "garbage in, garbage out" aspect of error suppression. Again, this was already doable with multiple individual //@ts-ignores so....

@weswigham weswigham force-pushed the allow-ts-nocheck-in-ts branch from 30ed2fb to a6f3eb7 Compare September 11, 2019 22:49
@orta
Copy link
Contributor

orta commented Sep 12, 2019

Interesting, I guess this is another angle for of a migration path to TS 👍

@orta orta added the Update Docs on Next Release Indicates that this PR affects docs label Sep 12, 2019
@@ -1691,9 +1691,10 @@ namespace ts {
Debug.assert(!!sourceFile.bindDiagnostics);

const isCheckJs = isCheckJsEnabledForFile(sourceFile, options);
const isTsNoCheck = !!sourceFile.checkJsDirective && sourceFile.checkJsDirective.enabled === false;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question, currently unchecked files have also some fuzzy completions (currently only in js files since those are the only ones) but would we want that? I think no but I am just pointing out one of the checkJs enabled and disabled difference.

Similarly there is disableJsDiagnostics code fix, do we need similar one now for ts?

Handle report reportImplicitAny for early exit?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think a //@ts-nocheck in a TS file should do anything other than suppress errors - everything else should be as-is.

@orta
Copy link
Contributor

orta commented Sep 20, 2019

@typescript-bot pack this

@typescript-bot
Copy link
Collaborator

typescript-bot commented Sep 20, 2019

Heya @orta, I've started to run the tarball bundle task on this PR at 1c3ee9c. You can monitor the build here. It should now contribute to this PR's status checks.

@typescript-bot
Copy link
Collaborator

Hey @orta, I've packed this into an installable tgz. You can install it for testing by referencing it in your package.json like so:

{
    "devDependencies": {
        "typescript": "https://typescript.visualstudio.com/cf7ac146-d525-443c-b23c-0d58337efebc/_apis/build/builds/44912/artifacts?artifactName=tgz&fileId=59CC485028CC8ADB6232385F18A7C913EC14E28817413D17FDC3DE01EED61A6502&fileName=/typescript-3.7.0-insiders.20190920.tgz"
    }
}

and then running npm install.

@weswigham
Copy link
Member Author

Approved in today's design meeting ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Update Docs on Next Release Indicates that this PR affects docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants