Plugin-php: Explicit `trailingComma: "es5"` value in config causes "Invalid trailingComma ..." error

[jasonschock]

Prettier 1.16.4

PHP Plugin 0.10.2

I have an existing project to which I'm wanting to process PHP files using this plugin. After installing the plugin, when I try to process non-PHP files, Prettier tells me Invalid trailingComma value. Expected "all", "none", "php5" or "php7.2", but received "es5".

I already have the following .prettierrc:

{
  "printWidth": 80,
  "tabWidth": 2,
  "useTabs": false,
  "semi": true,
  "singleQuote": false,
  "trailingComma": "es5",
  "bracketSpacing": true,
  "jsxBracketSameLine": false,
  "arrowParens": "avoid",
  "requirePragma": false,
  "insertPragma": false,
  "proseWrap": "preserve"
}

1. Works as expected, before installing the plugin:

Command:

prettier 'src/app.js'  --write --loglevel debug --config .prettierrc

Output:

[debug] normalized argv: {"_":["src/app.js"],"color":true,"editorconfig":true,"write":true,"loglevel":"debug","config":".prettierrc","plugin-search-dir":[],"plugin":[],"ignore-path":".prettierignore","debug-repeat":0,"config-precedence":"cli-override"}
[debug] load config file from '.prettierrc'
[debug] loaded options `{"printWidth":80,"tabWidth":2,"useTabs":false,"semi":true,"singleQuote":false,"trailingComma":"es5","bracketSpacing":true,"jsxBracketSameLine":false,"arrowParens":"avoid","requirePragma":false,"insertPragma":false,"proseWrap":"preserve"}`
[debug] applied config-precedence (cli-override): {"arrowParens":"avoid","bracketSpacing":true,"insertPragma":false,"jsxBracketSameLine":false,"printWidth":80,"proseWrap":"preserve","requirePragma":false,"semi":true,"singleQuote":false,"tabWidth":2,"trailingComma":"es5","useTabs":false}
src/app.js 39ms

2. Install plugin

yarn global add @prettier/plugin-php

yarn global v1.13.0
[1/4] 🔍  Resolving packages...
[2/4] 🚚  Fetching packages...
...
[3/4] 🔗  Linking dependencies...
[4/4] 🔨  Building fresh packages...
warning "@prettier/[email protected]" has no binaries
✨  Done in 5.33s.

3. Breaks, after installing plugin:

prettier 'src/app.js'  --write --loglevel debug --config .prettierrc

Output

[debug] normalized argv: {"_":["src/app.js"],"color":true,"editorconfig":true,"write":true,"loglevel":"debug","config":".prettierrc","plugin-search-dir":[],"plugin":[],"ignore-path":".prettierignore","debug-repeat":0,"config-precedence":"cli-override"}
[debug] load config file from '.prettierrc'
[debug] loaded options `{"printWidth":80,"tabWidth":2,"useTabs":false,"semi":true,"singleQuote":false,"trailingComma":"es5","bracketSpacing":true,"jsxBracketSameLine":false,"arrowParens":"avoid","requirePragma":false,"insertPragma":false,"proseWrap":"preserve"}`
[error] Unable to expand glob patterns: src/app.js !**/node_modules/** !./node_modules/** !**/.{git,svn,hg}/** !./.{git,svn,hg}/**

[error] Invalid trailingComma value. Expected "all", "none", "php5" or "php7.2", but received "es5".

Against PHP files, it works as expected after adding this to .prettierrc:

  "overrides": [
    {
      "files": "*.php",
      "options": {
        "tabWidth": 4,
        "trailingComma": "php7.2",
      }
    }
  ]

Expected behavior:

• My existing config should just work after installing the plugin, but it appears to be trying to process .js files as PHP.
• The plugin should know that .js files aren't .php files and ignore them.

Sorry if I'm not understanding something with setup, but the docs don't suggest I need to do much more than install and go.

• Am I supposed to set up two separate scripts to process PHP separately from other files?

Thanks.

[jasonschock]

I noticed the Ruby plugin recently changed the option name to addTrailingCommas.

Perhaps needed for this plugin as well?

[alexander-akait]

@jasonschock oh, looks you should use overrides right now https://github.com/prettier/prettier/blob/master/docs/configuration.md#configuration-overrides,

It is bug in prettier, not in plugin, prettier should respect same options for plugins, i think.

/cc @ikatyang What do you think?

[ikatyang]

It's indeed a bug in Prettier but it's also a bug in this plugin:

• Prettier: Built-in options should not be overwritten.
  
  
  
  • 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.)

[alexander-akait]

@ikatyang what about change logic: if option exists in plugin(s) take possible values from plugin(s) option. * --js-trailing-comma and --php-trailing-comma sound good, but i am afraid for repos who have many languages it is require a lot of CLI flags (for each language) or increase config what is also very bad (we love zero conflagration).

[ikatyang]

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;
}

[alexander-akait]

@ikatyang thanks for the explanation, i am afraid only what many languages require a lot of cli flags and/or big config, let's ask @vjeux what he thinks about this.

[vjeux]

Arg, this is annoying. I wasn’t happy about having es5 as an option in the first place and it’s coming to bit us again :(

@ikatyang solution sounds reasonable.

[alexander-akait]

Ok, let's fix it

[Kocal]

I'm posting on this issue because I faced the same problem. However I tried this workaround:

{
  "overrides": [
    {
      "files": "*.js",
      "options": {
        "trailingComma": "es5"
      }
    },
    {
      "files": "*.php",
      "options": {
        "trailingComma": "php7.2"
      }
    }
  ]
}

But it's not working:

Also, I tried with two config files:

# .prettierrc
{
  "trailingComma": "es5"
}

# .prettierrc-php
{
  "trailingComma": "php7.2"
}

It works fine when prettifying PHP files:

But not when I'm running ESLint (configured to works with Prettier), because the PHP plugin is automatically loaded:

---

Does it mean that we can't use Prettier for both prettify both PHP and JavaScript? :confused:

[Kocal]

Okay, so when updating my .prettierrc for this:

{
  "trailingComma": "es5",
  "pluginSearchDirs": ["./web/app"]
}

I "counter" the plugins autoloading behavior and I got ESLint + Prettier working:

[alexander-akait]

@Kocal yep, it is bug in design, and we fix it for next version

[Kocal]

Understand, thanks! :)

[flexchar]

@evilebottnawi is there an eta for next version release? This would be super helpful to have:)

[alexander-akait]

No ETA, PR welcome

[flexchar]

@evilebottnawi Good idea, I have renamed option to trailingCommaPHP.
It seems like all problems went away. Would this be helpful?

[alexander-akait]

Yes, we need rename option

[flexchar]

Would a simple PR with trailingCommaPHP work?

[alexander-akait]

Yes, we need rename option and respect default trailingComma values like none and all

[alexander-akait]

Fixed https://github.com/prettier/plugin-php/pull/1059