From 65dc9eac2e294edad52bfbfe2ca9f92f46badfba Mon Sep 17 00:00:00 2001 From: Kevin Gibbons Date: Sun, 22 Mar 2020 15:52:11 -0700 Subject: [PATCH 1/3] avoid trailing commas on index signatures with only one parameter --- src/language-js/printer-estree.js | 10 +++++- .../__snapshots__/jsfmt.spec.js.snap | 36 +++++++++++++++++++ .../index-signature.ts | 5 +++ 3 files changed, 50 insertions(+), 1 deletion(-) diff --git a/src/language-js/printer-estree.js b/src/language-js/printer-estree.js index 81752b18f837..19c7468db367 100644 --- a/src/language-js/printer-estree.js +++ b/src/language-js/printer-estree.js @@ -3259,6 +3259,14 @@ function printPathNoParens(path, options, print, args) { case "TSIndexSignature": { const parent = path.getParentNode(); + // The typescript parser accepts multiple parameters here. If you're + // using them, it makes sense to have a trailing comma. But if you + // aren't, this is more like a computed property name than an array. + // So we leave off the trailing comma when there's just one parameter. + const trailingComma = n.parameters.length > 1 + ? ifBreak(shouldPrintComma(options) ? "," : "") + : ""; + const parametersGroup = group( concat([ indent( @@ -3267,7 +3275,7 @@ function printPathNoParens(path, options, print, args) { join(concat([", ", softline]), path.map(print, "parameters")), ]) ), - ifBreak(shouldPrintComma(options) ? "," : ""), + trailingComma, softline, ]) ); diff --git a/tests/typescript_error_recovery/__snapshots__/jsfmt.spec.js.snap b/tests/typescript_error_recovery/__snapshots__/jsfmt.spec.js.snap index 69e51fbbd831..c9b4a2b647b6 100644 --- a/tests/typescript_error_recovery/__snapshots__/jsfmt.spec.js.snap +++ b/tests/typescript_error_recovery/__snapshots__/jsfmt.spec.js.snap @@ -173,6 +173,11 @@ type TooLong = { type TooLong81 = { [loooooooooooooooooooooooooong: string, loooooooooooooooooong: string]: string; } type TooLong80 = { [loooooooooooooooooooooooooong: string, looooooooooooooooong: string]: string; } +// note lack of trailing comma in the index signature +type TooLongSingleParam = { + [looooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong: string]: string; +} + =====================================output===================================== type A = { [key: string] }; @@ -199,6 +204,13 @@ type TooLong80 = { [loooooooooooooooooooooooooong: string, looooooooooooooooong: string]: string; }; +// note lack of trailing comma in the index signature +type TooLongSingleParam = { + [ + looooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong: string + ]: string; +}; + ================================================================================ `; @@ -224,6 +236,11 @@ type TooLong = { type TooLong81 = { [loooooooooooooooooooooooooong: string, loooooooooooooooooong: string]: string; } type TooLong80 = { [loooooooooooooooooooooooooong: string, looooooooooooooooong: string]: string; } +// note lack of trailing comma in the index signature +type TooLongSingleParam = { + [looooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong: string]: string; +} + =====================================output===================================== type A = { [key: string] }; @@ -250,6 +267,13 @@ type TooLong80 = { [loooooooooooooooooooooooooong: string, looooooooooooooooong: string]: string; }; +// note lack of trailing comma in the index signature +type TooLongSingleParam = { + [ + looooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong: string + ]: string; +}; + ================================================================================ `; @@ -275,6 +299,11 @@ type TooLong = { type TooLong81 = { [loooooooooooooooooooooooooong: string, loooooooooooooooooong: string]: string; } type TooLong80 = { [loooooooooooooooooooooooooong: string, looooooooooooooooong: string]: string; } +// note lack of trailing comma in the index signature +type TooLongSingleParam = { + [looooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong: string]: string; +} + =====================================output===================================== type A = { [key: string] }; @@ -301,6 +330,13 @@ type TooLong80 = { [loooooooooooooooooooooooooong: string, looooooooooooooooong: string]: string; }; +// note lack of trailing comma in the index signature +type TooLongSingleParam = { + [ + looooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong: string + ]: string; +}; + ================================================================================ `; diff --git a/tests/typescript_error_recovery/index-signature.ts b/tests/typescript_error_recovery/index-signature.ts index 896541357681..55943aec8356 100644 --- a/tests/typescript_error_recovery/index-signature.ts +++ b/tests/typescript_error_recovery/index-signature.ts @@ -12,3 +12,8 @@ type TooLong = { } type TooLong81 = { [loooooooooooooooooooooooooong: string, loooooooooooooooooong: string]: string; } type TooLong80 = { [loooooooooooooooooooooooooong: string, looooooooooooooooong: string]: string; } + +// note lack of trailing comma in the index signature +type TooLongSingleParam = { + [looooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong: string]: string; +} From f7662c87e3746a28c56f89d63b7a721ed54194d3 Mon Sep 17 00:00:00 2001 From: Kevin Gibbons Date: Sun, 22 Mar 2020 16:04:51 -0700 Subject: [PATCH 2/3] add changelog --- changelog_unreleased/typescript/pr-7836.md | 33 ++++++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100644 changelog_unreleased/typescript/pr-7836.md diff --git a/changelog_unreleased/typescript/pr-7836.md b/changelog_unreleased/typescript/pr-7836.md new file mode 100644 index 000000000000..8a7156fb6d57 --- /dev/null +++ b/changelog_unreleased/typescript/pr-7836.md @@ -0,0 +1,33 @@ +#### Avoid trailing commas on index signatures with only one parameter ([#7836](https://github.com/prettier/prettier/pull/7836) by [@bakkot](https://github.com/bakkot)) + +TypeScript index signatures technically allow multiple parameters and trailing commas, but it's an error to have multiple parameters there, and Babel's TypeScript parser does not accept them. So Prettier now avoids putting a trailing comma there when you have only one parameter. + + +```ts +// Input +export type A = { + a?: { + [ + x: string + ]: typeof SomeLongLongLongTypeName[keyof typeof SomeLongLongLongTypeName]; + } | null; +}; + +// Prettier stable +export type A = { + a?: { + [ + x: string, + ]: typeof SomeLongLongLongTypeName[keyof typeof SomeLongLongLongTypeName]; + } | null; +}; + +// Prettier master +export type A = { + a?: { + [ + x: string + ]: typeof SomeLongLongLongTypeName[keyof typeof SomeLongLongLongTypeName]; + } | null; +}; +``` From 681c79b54553a7acf72e821f88c1e9d3240a4898 Mon Sep 17 00:00:00 2001 From: Kevin Gibbons Date: Sun, 22 Mar 2020 16:08:14 -0700 Subject: [PATCH 3/3] lint --- src/language-js/printer-estree.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/language-js/printer-estree.js b/src/language-js/printer-estree.js index 19c7468db367..2c76f6388624 100644 --- a/src/language-js/printer-estree.js +++ b/src/language-js/printer-estree.js @@ -3263,9 +3263,10 @@ function printPathNoParens(path, options, print, args) { // using them, it makes sense to have a trailing comma. But if you // aren't, this is more like a computed property name than an array. // So we leave off the trailing comma when there's just one parameter. - const trailingComma = n.parameters.length > 1 - ? ifBreak(shouldPrintComma(options) ? "," : "") - : ""; + const trailingComma = + n.parameters.length > 1 + ? ifBreak(shouldPrintComma(options) ? "," : "") + : ""; const parametersGroup = group( concat([