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

TypeScript: avoid trailing commas on index signatures with only one parameter #7836

Merged
merged 3 commits into from Mar 23, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
33 changes: 33 additions & 0 deletions 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.

<!-- prettier-ignore -->
```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;
};
```
11 changes: 10 additions & 1 deletion src/language-js/printer-estree.js
Expand Up @@ -3259,6 +3259,15 @@ 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(
Expand All @@ -3267,7 +3276,7 @@ function printPathNoParens(path, options, print, args) {
join(concat([", ", softline]), path.map(print, "parameters")),
])
),
ifBreak(shouldPrintComma(options) ? "," : ""),
trailingComma,
softline,
])
);
Expand Down
36 changes: 36 additions & 0 deletions tests/typescript_error_recovery/__snapshots__/jsfmt.spec.js.snap
Expand Up @@ -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] };

Expand All @@ -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;
};

================================================================================
`;

Expand All @@ -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] };

Expand All @@ -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;
};

================================================================================
`;

Expand All @@ -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] };

Expand All @@ -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;
};

================================================================================
`;

Expand Down
5 changes: 5 additions & 0 deletions tests/typescript_error_recovery/index-signature.ts
Expand Up @@ -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;
}