Fix two issues with ConvertToTypeOnlyExport codefix (#40490)

* Modify test case to reproduce error

* Fix TypeOnlyExport codefix to work with 3 or more type exports in the same declaration

The check to ensure that a fixed export declaration wasn't fixed again
was reversed. This only surfaced when 3 or more type exports existed in
the same declaration.

* Add failing test cases for comments being duplicated

* Fix convertToTypeOnlyExport codefix from duplicating leading comments

* Simplify convertToTypeOnlyExport when change is just inserting `type` keyword

Co-authored-by: Andrew Branch <andrew@wheream.io>
This commit is contained in:
Michael
2020-10-28 09:08:20 +11:00
committed by GitHub
parent 9ed608b439
commit 71cd5d522d
3 changed files with 31 additions and 19 deletions

View File

@@ -15,7 +15,7 @@ namespace ts.codefix {
const fixedExportDeclarations = new Map<string, true>();
return codeFixAll(context, errorCodes, (changes, diag) => {
const exportSpecifier = getExportSpecifierForDiagnosticSpan(diag, context.sourceFile);
if (exportSpecifier && !addToSeen(fixedExportDeclarations, getNodeId(exportSpecifier.parent.parent))) {
if (exportSpecifier && addToSeen(fixedExportDeclarations, getNodeId(exportSpecifier.parent.parent))) {
fixSingleExportDeclaration(changes, exportSpecifier, context);
}
});
@@ -35,16 +35,7 @@ namespace ts.codefix {
const exportDeclaration = exportClause.parent;
const typeExportSpecifiers = getTypeExportSpecifiers(exportSpecifier, context);
if (typeExportSpecifiers.length === exportClause.elements.length) {
changes.replaceNode(
context.sourceFile,
exportDeclaration,
factory.updateExportDeclaration(
exportDeclaration,
exportDeclaration.decorators,
exportDeclaration.modifiers,
/*isTypeOnly*/ true,
exportClause,
exportDeclaration.moduleSpecifier));
changes.insertModifierBefore(context.sourceFile, SyntaxKind.TypeKeyword, exportClause);
}
else {
const valueExportDeclaration = factory.updateExportDeclaration(
@@ -61,7 +52,10 @@ namespace ts.codefix {
factory.createNamedExports(typeExportSpecifiers),
exportDeclaration.moduleSpecifier);
changes.replaceNode(context.sourceFile, exportDeclaration, valueExportDeclaration);
changes.replaceNode(context.sourceFile, exportDeclaration, valueExportDeclaration, {
leadingTriviaOption: textChanges.LeadingTriviaOption.IncludeAll,
trailingTriviaOption: textChanges.TrailingTriviaOption.Exclude
});
changes.insertNodeAfter(context.sourceFile, exportDeclaration, typeExportDeclaration);
}
}

View File

@@ -7,6 +7,7 @@
////export type B = {};
// @Filename: /b.ts
/////* Comment */
////export { A, B } from './a';
goTo.file("/b.ts");
@@ -14,5 +15,7 @@ verify.codeFix({
index: 0,
description: ts.Diagnostics.Convert_to_type_only_export.message,
errorCode: ts.Diagnostics.Re_exporting_a_type_when_the_isolatedModules_flag_is_provided_requires_using_export_type.code,
newFileContent: "export type { A, B } from './a';"
newFileContent:
`/* Comment */
export type { A, B } from './a';`
});

View File

@@ -11,19 +11,34 @@
////export type T3 = {};
////export const V2 = {};
////export type T4 = {};
////export type T5 = {};
// @Filename: /c.ts
////export { T1, V1, T2 } from './a';
////export { T3, V2, T4 } from './b';
////export type T6 = {};
////export type T7 = {};
goTo.file("/c.ts");
// @Filename: /d.ts
/////* Test comment */
////export { T1, V1, T2 } from './a';
////export { T3, V2, T4, T5 } from './b';
////// TODO: fix messy formatting
////export {
//// T6 // need to export this
//// , T7, /* and this */ } from "./c";
goTo.file("/d.ts");
verify.codeFixAll({
fixAllDescription: ts.Diagnostics.Convert_all_re_exported_types_to_type_only_exports.message,
fixId: "convertToTypeOnlyExport",
newFileContent:
`export { V1 } from './a';
`/* Test comment */
export { V1 } from './a';
export type { T1, T2 } from './a';
export { V2 } from './b';
export type { T3, T4 } from './b';
`
export type { T3, T4, T5 } from './b';
// TODO: fix messy formatting
export type {
T6 // need to export this
, T7, /* and this */ } from "./c";`
});