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

comma-dangle "off" with object option is not working properly. #105

Closed
zyy7259 opened this issue Jul 6, 2017 · 3 comments
Closed

comma-dangle "off" with object option is not working properly. #105

zyy7259 opened this issue Jul 6, 2017 · 3 comments

Comments

@zyy7259
Copy link

zyy7259 commented Jul 6, 2017

Versions:

  • prettier-eslint version: 4.1.1
  • node version: v8.1.2
  • npm (or yarn) version: npm 5.0.4

Have you followed the debugging tips?

Yes

Relevant code or config

{
  "extends": ["airbnb-base", "prettier"]
}

What I did:

What happened:

Reproduction repository:

git@github.com:zyy7259/prettier-foo.git

Problem description:

comma-dangle "off" with object option is not working properly.

Run prettier-eslint index.js --log-level trace in the root dir.

The input is

const arr = [
  "asdfasdfasdfasdf",
  "asdfasdfasdfasdf",
  "asdfasdfasdfasdf",
  "asdfasdfasdfasdf",
];

arr.sort();

The output is just the same, with the redundant comma.

Then I search "comma-dangle" (with quotation mark) in the terminal, and found some log.

Trace: prettier-eslint [TRACE]: adding to relevant rules: {"comma-dangle":["off",{"arrays":"always-multiline","objects":"always-multiline","imports":"always-multiline","exports":"always-multiline","functions":"always-multiline"}]}

"comma-dangle": Array [
      "off",
      Object {
        "arrays": "always-multiline",
        "exports": "always-multiline",
        "functions": "always-multiline",
        "imports": "always-multiline",
        "objects": "always-multiline",
      },
    ]

I guess maybe the config is wrong, there is a redundant object option.

Then I change the .eslintrc.json to (I create a branch fix to demo this step)

{
  "extends": ["airbnb-base", "prettier"],
  "rules": {
    "comma-dangle": ["off", {}]
  }
}

and run prettier-eslint index.js --log-level trace again, this time the output is right. Search "comma-dangle" again, got

Trace: prettier-eslint [TRACE]: adding to relevant rules: {"comma-dangle":["off",{}]}

"comma-dangle": Array [
      "off",
      Object {},
    ]

Suggested solution:

Not sure if this is a issue with prettier-eslint

@kentcdodds
Copy link
Member

Thanks for the detailed issue! It's unlikely I'll have time to dedicate to hunting down the core issue. If you could do a little more digging and propose a solution that would be helpful. Thanks!

@cspanring
Copy link

The short answer is, that this issue can be resolved by changing the line https://github.com/prettier/prettier-eslint/blob/master/src/utils.js#L211 with

const [, {arrays = '', objects = '', functions = ''} = {}] = objectConfig

...giving the destructured object a default value. (made that change but the jest --coverage kept failing and I'm not familiar with the underlying test suites and how to quickly fix the coverage test, but happy to dig through that if you have a few pointers).

The larger question is about error handling. I got here from this issue: prettier/prettier-atom#231 (comment) , an atom-plugin. prettier-eslint was throwing an error that bubbled through the plugin and was a little bit hard to decipher. Part of the problem being a missing stack trace in the recent versions of Atom and/or the plugin.

What do you think about an error handling system that produces slightly more verbose or even "human readable" messages (e.g. especially useful for Atom users)?

@kentcdodds
Copy link
Member

Happy to accept a pull request 😄

@zimme zimme closed this as completed in 3316f12 Dec 12, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants