Add support to report errors in js files #14496
Conversation
|
This means that it'll be possible to opt-in to have types that can be inferred from the environment ( Sounds pretty great for progressively migrating a codebase to typescript 🎉 I wonder how this interacts with |
correct. this is the intention.
I do not know what is the best solution here. We were discussing this topic today, and we are doing some additional inferences/modifications to types in a .js file that we do not do in a .ts file. some of these involve |
tests/cases/compiler/checkJsFiles.ts
Outdated
| @@ -0,0 +1,6 @@ | |||
| // @allowJs: true | |||
| // @checkJsFiles: true | |||
There was a problem hiding this comment.
You might want to add @noEmit: true or @outFile to suppress all the Cannot write file... because it would overwrite errors in the baselines.
src/compiler/commandLineParser.ts
Outdated
| @@ -481,6 +481,12 @@ namespace ts { | |||
| name: "plugin", | |||
| type: "object" | |||
| } | |||
| }, | |||
| { | |||
| name: "checkJsFiles", | |||
There was a problem hiding this comment.
Since we have allowJs, shouldn't this be checkJs? Having 'Files' on just one of them seems inconsistent.
src/compiler/program.ts
Outdated
| // Instead, we'll report errors for using TypeScript-only constructs from within a | ||
| // JavaScript file when we get syntactic diagnostics for the file. | ||
| const checkDiagnostics = isSourceFileJavaScript(sourceFile) ? [] : typeChecker.getDiagnostics(sourceFile, cancellationToken); | ||
| // For JavaScript files, we don't want to report semantic errors unless ecplicitlly requested. |
src/compiler/parser.ts
Outdated
| @@ -5878,13 +5879,24 @@ namespace ts { | |||
| amdDependencies.push(amdDependency); | |||
| } | |||
| } | |||
|
|
|||
| const checkJsDirectiveRegEx = /^\/\/\/?\s*@check(\s+(true|false))?/gim; | |||
There was a problem hiding this comment.
Or maybe an @nocheck directive?
…' into checkJSFiles_QuickFixes
Allow skipping diagnostics in .js file using comments and quick fixes to add them
| function shouldReportDiagnostic(diagnostic: Diagnostic) { | ||
| const { file, start } = diagnostic; | ||
| const lineStarts = getLineStarts(file); | ||
| let { line } = computeLineAndCharacterOfPosition(lineStarts, start); |
There was a problem hiding this comment.
- can line have more than one error where only one should be suppresed
- seems that
ts-suppressshould be located immediately before the line to be suppressed so if user will put a comment betweents-suppressand target line it will stop working. Is it intended?
| .map(d => allDiagnostcs[d].code); | ||
| } | ||
|
|
||
| function shouldCheckJsFile(sourceFile: SourceFile, compilerOptions: CompilerOptions) { |
There was a problem hiding this comment.
this is the same snipped with this one. Can we share this?
| @@ -5883,13 +5884,24 @@ namespace ts { | |||
| amdDependencies.push(amdDependency); | |||
| } | |||
| } | |||
|
|
|||
| const checkJsDirectiveRegEx = /^\/\/\/?\s*(@ts-check|@ts-nocheck)\s*$/gim; | |||
There was a problem hiding this comment.
I would recommend you pull this out of the function and remove the g option. g is only really useful if you want to preserve lastIndex when using a regular expression in a loop. Also I would remove the m option as it should be unnecessary.
Consider this instead:
const checkJsDirectiveRegEx = /^\/\/\/?\s*@ts-(no)?check\s*$/i;
...
const checkJsDirectiveMatchResult = checkJsDirectiveRegEx.exec(comment);
if (checkJsDirectiveMatchResult) {
checkJsDirective = {
enabled: !!checkJsDirectiveMatchResult[1],
end: range.end,
pos: range.pos
};
}Capturing only the no allows you to avoid the additional case-insensitive string comparison.
|
Does this close #7661 ? |
|
Nope. |
|
Very nice option. But it will be more powerful if allows to suppression several errors |
--checkJsFiles--checkJsflag to enable error reporting in all .js files in a project (used in conjunction with--allowJs)./// @checkto enable the error reporting per file.