Epic: Trailing commas in TS/Flow

[duailibe]

This is an old problem and I thought I had an issue open to discuss this topic, but we should make a decision on this matter:

How do we handle trailing commas in different versions of Flow/TS?
Should we add flowXXX and tsX.X options?
Should we always support new trailing commas in "all" and treat "es5" as "none"?

cc @prettier/core

[duailibe]

Issues raised on this topic: #6197 #3722 #3662 #3313

[alexander-akait]

Should we add flowXXX and tsX.X options?

I think yes, we planned same for php plugin, sometimes languages change syntax

Should we always support new trailing commas in "all" and treat "es5" as "none"?

To avoid breaking code we should considered:

• all and tsX.X/flowXXX together (all + ts3.3 !== all +ts3.5 )
• with none no problems
• es5 was bad idea, i don't know here solution, maybe apply trailing comma for lower level (i.e. apply commas only for lower ts/flow version like we use es5 and lower ts/flow version)

[ikatyang]

• Explicit trailingComma: "es5" value in config causes "Invalid trailingComma ..." error plugin-php#964 (comment)

• Personally, I think we should replace --trailing-comma <none|es5|all> with --[no-]trailing-comma/--js-trailing-comma es5, but unfortunately there's no backward-compatible way to do it in the CLI since string flags would suddenly become part of filenames.
• PHP plugin: any option that wants to extend the existing one should have their own name, --php-trailing-comma for example. (Since we do support sub-language formatting (embed), it's not possible to use the same name with different option.)

• Explicit trailingComma: "es5" value in config causes "Invalid trailingComma ..." error plugin-php#964 (comment)

I meant the php-specific value (php5, php7.2) should have their own name (--php-trailing-comma) but the general value (all, none) can still be used as before (--trailing-comma). The --php-trailing-comma should take precedence over --trailing-comma in this plugin, something like:

function getPhpTrailingCommaValue(options): "all" | "php5" | "php7.2" | "none" {
  if (options.phpTrailingComma) return options.phpTrailingComma;
  if (options.trailingComma === "es5") return "none";
  return options.trailingComma;
}

Same thing can be applied to TS/Flow as well.

[duailibe]

@ikatyang your solution sounds reasonable. I'll investigate that for 2.0

[kachkaev]

We can probably close this issue in favor of #11465

[nickmccurdy]

I just noticed #3722 is still open. Should that be included with Prettier 3?

[fisker]

Fixed by #14085