trailingComma validation error about prettier-atom HOT 39 CLOSED

Just for anyone coming across this, my issue was a old version of prettier installed as a devDependency of the project I was working on, which expected trailingComma to be a boolean and was being passed the string all.
Should prettier-atom check that the version of prettier it will use is compatible with the options being passed?

from prettier-atom.

I tried 0.21.3. Unfortunately... same issue :-(

from prettier-atom.

@dgcoffman I checked that repo using prettier-eslint's CLI directly and the bug still happens, so this is a prettier-eslint error. Can you make an issue over at https://github.com/prettier/prettier-eslint?

In the meantime, I noticed that, for some reason, adding this to the end of your .eslintrc stops the bug from happening even though it's redundant to the eslint:recommended preset.

  comma-dangle:
    - error
    - never

from prettier-atom.

I just updated prettier-atom and get the same error as Aaron. When I enabled "ESLint integration" it immediately fails with the error listed above. My config.cson doesn't have anything listed except
useEslint: true

from prettier-atom.

@aaronshaf It looks like you've got old options set in there, I would wipe those and use the settings menu to repopulate your config.

from prettier-atom.

Where can I find the old options?

from prettier-atom.

In your config.cson, it still is listing the old options. I would delete those manually. Then I would go to Preferences -> Packages -> prettier-atom and set the new settings there. Let me know if that works.

from prettier-atom.

I did that and it did not work. I ended up deleting the entire .atom directory, still no cigar after reinstall of the module.

from prettier-atom.

Can you screenshot your settings page for prettier-atom and show me which option you have selected for Trailing Commas?

from prettier-atom.

https://cl.ly/2m0r0l1T211y

https://cl.ly/2Z3O0D3o0F3o

It's set to "none".

from prettier-atom.

@aaronshaf Hmm, this is a very strange bug, it seems like your old config settings are still there no matter what you do on your end. I'm not really sure what's going on, but I am going to try something weird in the plugin and see if it fixes your issue.

from prettier-atom.

@aaronshaf Please let me know if the latest version fixes this issue for you.

from prettier-atom.

@aaronshaf Ugh, sorry man. Okay, so if you manually delete those options from your config.cson, then restart atom, they come back again? Or do they stay gone from the config?

Is anyone else having issues like this? Maybe we can figure out a common thread between environments and that will point towards what the cause is.

from prettier-atom.

That worked! Thank you so much.

from prettier-atom.

Users will have to delete the relevant section of config.cson and restart.

from prettier-atom.

Drat, sorry. False positive. Once I re-enabled eslint integration, same issue.

from prettier-atom.

When you re-enable the eslint integration, the old values come back and put themselves into your config.cson again?

from prettier-atom.

New config.cson:

"*":
  core:
    telemetryConsent: "no"
  editor:
    fontSize: 16
  "prettier-atom":
    useEslint: true

from prettier-atom.

So, you delete the old options from your config.cson, then it works. Then you uncheck and check the ESlint integration option, and although the old options don't reappear in your config, you get an error about the trailing comma being a string instead of a boolean again? Except now there's nothing to delete?

from prettier-atom.

It looks like it never worked when eslint integration was enabled for the plugin.

from prettier-atom.

Shoot, I can't think of what the problem may be here.

from prettier-atom.

I'm having a similar problem where commas are inserted always regardless of what the setting is. In addition, ESLint is not running fix.

I have deleted the relevant section in my config.cson and re-created it from the settings, no change.

config.cson now has

  "prettier-atom":
    prettierOptions:
      singleQuote: true
    useEslint: true

Yet commas are inserted and eslint is not run. Double-quoted strings are changed to single-quoted strings, so prettier is definitely running and picking up that setting.

Here is a repo showing this behavior: https://github.com/dgcoffman/DailyProgrammer

from prettier-atom.

@aaronshaf Are you still encountering this? I believe it may actually be an error with atom and how it deals with changes to package settings, as I noticed similar problems when other packages changed their own settings API as well.

from prettier-atom.

What version were you on prior to updating?

from prettier-atom.

Similar issue here. prettier-atom will not respect my settings (commas, line-length, etc.) while eslint integration is turned on. I think it's falling back to the defaults or something.

Using the latest version (0.26.0)

from prettier-atom.

prettier-atom will not respect my settings (commas, line-length, etc.) while eslint integration is turned on. I think it's falling back to the defaults or something.

@spudly This is the intended behavior, if you want it to use the prettier settings you chose and not the eslint settings, you need to turn the eslint integration off

from prettier-atom.

@robwise So in my setup, I've disabled all style configs for eslint because I want prettier to handle style (I only want eslint to fix errors / bad practices). I don't have any configured line-length or comma rules in my eslint config because I want prettier to handle that.

Let's take line-length for example. I would expect it to look at the maxlen eslint rule first. If it's configured, use the configured length. If not, look at the settings for prettier-atom. If there's a line length configured there, use that. If not, fall back to the default prettier settings (80 chars I believe).

If I understand you correctly, it seems like the second step is not happening, and it first looks like it looks in eslint first and if there's no maxlen rule it just uses the prettier default. Is that right? Would you be willing to accept a PR that makes it fallback to the prettier-atom settings before falling back to the prettier defaults?

from prettier-atom.

I don't think this is going to be possible to do from this plugin, because the detection of the ESlint rules and whether they are defined or need to fall back is all done by prettier-eslint, not this atom plugin?

from prettier-atom.

@robwise,

I spent some time digging through the code, and it looks to me like we should be sending the configured options to prettier-eslint. It will then behave as expected.

prettier-eslint takes a prettierOptions option. See https://github.com/prettier/prettier-eslint/blob/master/src/index.js#L47

If prettierOptions is specified, it uses the prettierOptions configured by the user. Otherwise, it infers the prettier options from the package's ESLint config. See: https://github.com/prettier/prettier-eslint/blob/master/src/utils.js#L88

It's not passing prettierOptions here:

Perhaps the best way forward is to add a config in the plugin called "Infer prettier options from eslint config." If false, it will pass prettierOptions to prettierEslint.

from prettier-atom.

@spudly Can you elaborate on the use case for this? Why would you want to add trailing commas with prettier, for example, if your eslint config is set to not add trailing commas? The original intent was that if you are going to use the ESlint integration, then prettier-eslint "just works" by picking up your configuration from the ESlint config. This way, as you switch to different projects with different ESlint configurations, you don't need to adjust anything.

from prettier-atom.

@robwise

My use case is this: I want eslint to handle warnings and errors (possible bugs), while prettier makes the code look pretty.

To that end, I:

• have no style rules enabled in my eslint config
• created #110 (merged) to run prettier after eslint (not before)

Unfortunately, the prettier options I specify in the plugin settings are not respected because they are not passed on to prettier-eslint.

from prettier-atom.

Oh okay, now I see. Yeah so maybe what we should do is add a boolean flag like you say to the prettier-eslint options section of the settings that you made in #110 and we default this to off. If we detect it's true, then we pass the prettier options on to ESlint?

from prettier-atom.

Sounds good to me.

from prettier-atom.

I'm still seeing this issue with the latest version of prettier-atom, definitely not fixed.

from prettier-atom.

@tommoor A fix was never attempted. I'm pretty sure this is an Atom bug related to config schema changes.

from prettier-atom.

from prettier-atom.

Still getting this error:

atom version 1.17.2 prettier-atom version 0.31.1

$ prettier -v
1.3.1

$ prettier
[...]
--trailing-comma <none|es5|all>
[...]

Last time, the fix was to upgrade the prettier binary. Seems 1.3.1 is the latest. Is the atom package somehow referencing an old version?

from prettier-atom.

FWIW in my case it was definitely the the dependency in my project and prettier-atom were mismatched. It turns out that I didn't need to use prettier-atom at all anyway as I was already running prettier through eslint, the atom integration is pointless in that scenario.

from prettier-atom.

apologies, it seems my yarn.lock had two versions and it was resolving the old version 😭

from prettier-atom.