Skip to content

Commit 71cd5d5

Browse files
Blaszandrewbranch
andauthored
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 <[email protected]>
1 parent 9ed608b commit 71cd5d5

File tree

3 files changed

+30
-18
lines changed

3 files changed

+30
-18
lines changed

src/services/codefixes/convertToTypeOnlyExport.ts

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ namespace ts.codefix {
1515
const fixedExportDeclarations = new Map<string, true>();
1616
return codeFixAll(context, errorCodes, (changes, diag) => {
1717
const exportSpecifier = getExportSpecifierForDiagnosticSpan(diag, context.sourceFile);
18-
if (exportSpecifier && !addToSeen(fixedExportDeclarations, getNodeId(exportSpecifier.parent.parent))) {
18+
if (exportSpecifier && addToSeen(fixedExportDeclarations, getNodeId(exportSpecifier.parent.parent))) {
1919
fixSingleExportDeclaration(changes, exportSpecifier, context);
2020
}
2121
});
@@ -35,16 +35,7 @@ namespace ts.codefix {
3535
const exportDeclaration = exportClause.parent;
3636
const typeExportSpecifiers = getTypeExportSpecifiers(exportSpecifier, context);
3737
if (typeExportSpecifiers.length === exportClause.elements.length) {
38-
changes.replaceNode(
39-
context.sourceFile,
40-
exportDeclaration,
41-
factory.updateExportDeclaration(
42-
exportDeclaration,
43-
exportDeclaration.decorators,
44-
exportDeclaration.modifiers,
45-
/*isTypeOnly*/ true,
46-
exportClause,
47-
exportDeclaration.moduleSpecifier));
38+
changes.insertModifierBefore(context.sourceFile, SyntaxKind.TypeKeyword, exportClause);
4839
}
4940
else {
5041
const valueExportDeclaration = factory.updateExportDeclaration(
@@ -61,7 +52,10 @@ namespace ts.codefix {
6152
factory.createNamedExports(typeExportSpecifiers),
6253
exportDeclaration.moduleSpecifier);
6354

64-
changes.replaceNode(context.sourceFile, exportDeclaration, valueExportDeclaration);
55+
changes.replaceNode(context.sourceFile, exportDeclaration, valueExportDeclaration, {
56+
leadingTriviaOption: textChanges.LeadingTriviaOption.IncludeAll,
57+
trailingTriviaOption: textChanges.TrailingTriviaOption.Exclude
58+
});
6559
changes.insertNodeAfter(context.sourceFile, exportDeclaration, typeExportDeclaration);
6660
}
6761
}

tests/cases/fourslash/codeFixConvertToTypeOnlyExport1.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,15 @@
77
////export type B = {};
88

99
// @Filename: /b.ts
10+
/////* Comment */
1011
////export { A, B } from './a';
1112

1213
goTo.file("/b.ts");
1314
verify.codeFix({
1415
index: 0,
1516
description: ts.Diagnostics.Convert_to_type_only_export.message,
1617
errorCode: ts.Diagnostics.Re_exporting_a_type_when_the_isolatedModules_flag_is_provided_requires_using_export_type.code,
17-
newFileContent: "export type { A, B } from './a';"
18+
newFileContent:
19+
`/* Comment */
20+
export type { A, B } from './a';`
1821
});

tests/cases/fourslash/codeFixConvertToTypeOnlyExport3.ts

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,19 +11,34 @@
1111
////export type T3 = {};
1212
////export const V2 = {};
1313
////export type T4 = {};
14+
////export type T5 = {};
1415

1516
// @Filename: /c.ts
17+
////export type T6 = {};
18+
////export type T7 = {};
19+
20+
// @Filename: /d.ts
21+
/////* Test comment */
1622
////export { T1, V1, T2 } from './a';
17-
////export { T3, V2, T4 } from './b';
23+
////export { T3, V2, T4, T5 } from './b';
24+
////// TODO: fix messy formatting
25+
////export {
26+
//// T6 // need to export this
27+
//// , T7, /* and this */ } from "./c";
28+
1829

19-
goTo.file("/c.ts");
30+
goTo.file("/d.ts");
2031
verify.codeFixAll({
2132
fixAllDescription: ts.Diagnostics.Convert_all_re_exported_types_to_type_only_exports.message,
2233
fixId: "convertToTypeOnlyExport",
2334
newFileContent:
24-
`export { V1 } from './a';
35+
`/* Test comment */
36+
export { V1 } from './a';
2537
export type { T1, T2 } from './a';
2638
export { V2 } from './b';
27-
export type { T3, T4 } from './b';
28-
`
39+
export type { T3, T4, T5 } from './b';
40+
// TODO: fix messy formatting
41+
export type {
42+
T6 // need to export this
43+
, T7, /* and this */ } from "./c";`
2944
});

0 commit comments

Comments
 (0)