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

Can't specify object keys that aren't camel case, even when quoted #225

Closed
Qix- opened this issue Jun 6, 2017 · 16 comments
Closed

Can't specify object keys that aren't camel case, even when quoted #225

Qix- opened this issue Jun 6, 2017 · 16 comments

Comments

@Qix-
Copy link

Qix- commented Jun 6, 2017

It appears I'm going to have to create pragmas to ignore some rules.

With the following object:

const obj = {some_key: 1234}; // 'some_key' is required by the consuming library

XO dies with

Identifier some_key is not in camel case.  camelcase

However, when quoting it (thinking maybe it would ease the error), it errored out with:

Unnecessarily quoted property some_key found.  quote-props

It appears there's no proper way in XO to do this without ignoring some lines (which is ugly).

What XO should do is detect when errors arise in quote-props would cause other errors if unquoted and ignore such cases, though I would imagine this would require some hefty plugin code.

@vadimdemedes
Copy link

Very valid point, I ran into this couple of times before.

@sindresorhus
Copy link
Member

The problem is that ESLint rules have no knowledge of each other and the ESLint team doesn't seem interested in solving such problem. We could maybe work around it in XO by checking the results and handle conflicts.

@coreyfarrell
Copy link
Contributor

I've run into this issue once on a module that deals with mysql tables, in the past I just disabled camelcase. I just submitted a PR to eslint for camelcase to have an "exceptions" option with a list of identifiers. I feel like this is within the norm for an eslint rule and honestly it will serve my own needs.

As for quote-props, maybe it could be extended to support underscore: true to require quotes around property names that have underscores? The quote-props rule already has similar options numbers and keywords.

@sindresorhus
Copy link
Member

I just submitted a PR to eslint for camelcase to have an "exceptions" option with a list of identifiers.

For reference: eslint/eslint#9684

As for quote-props, maybe it could be extended to support underscore: true to require quotes around property names that have underscores? The quote-props rule already has similar options numbers and keywords.

That seems too specific to the underscore style. Maybe it could be an ignore option that accepted a regex for identifiers to ignore?

@j-f1
Copy link

j-f1 commented Dec 4, 2017

camelcase has an option to ignore object keys. i.e.

/* eslint camelcase: ["error", {properties: "never"}] */

// invalid
const foo_bar = 'baz'

// valid:
const bar = {
  foo_bar: fooBar
}

@sindresorhus
Copy link
Member

@j-f1 I don't want to ignore all object keys, just the ones with underscore.

@j-f1
Copy link

j-f1 commented Dec 4, 2017

What’s the difference? camelcase only checks if the key has an _ and isn’t UPPER_SNAKE_CASE.

@sindresorhus
Copy link
Member

I assumed the camelcase rule would also disallow PascalCase.

@sindresorhus
Copy link
Member

I'm not really concerned how it's solved though. I just want a solution to the rule conflict with camelcase and quote-props.

@j-f1
Copy link

j-f1 commented Dec 4, 2017

camelcase doesn’t appear to forbid PascalCase.

I just want a solution to the rule conflict with camelcase and quote-props.

I don’t see one if you set properties: 'never' — the quotes in { 'some_key': 123 } are unnecessary, and { some_key: 123 } is valid. Also, the fact that { 'some_key': 123 } isn’t reported is a bug in the camelcase rule IMO.

@ideastouch
Copy link

Issue fixed.
Add to package.json

  "xo": {
    "extends": ".eslintrc"
  },

and in .eslintrc add camelcase's rule

{
    "rules": {
        "camelcase": [
            "error",
            { "properties": "never" }
        ]
    }
}

The file .eslintrc probably could be any other name.
Read ESLint Rules for more properties available to .eslintrc .
I understood what to do reading ESLint Doc Shareable Config -> Sharing Multiple Configs, which was mentioned at npmjs XO -> Config -> extends.

@Qix-
Copy link
Author

Qix- commented Dec 29, 2017

@ideastouch yes of course, that's an option - overriding the default configuration is always an option. But this issue is addressing two conflicting rules in the default XO configuration that don't make sense.

@pvdlg
Copy link
Contributor

pvdlg commented Jan 7, 2018

The quote-props already has exception cases for numbers and JS keywords.
Maybe it's reasonable to open an issue/PR to add a regex options that would force to quote / not quote properties that match a regex.
In XO we could configure the quote-props to always quote non camel case props.

@j-f1
Copy link

j-f1 commented Jan 7, 2018

According to the ESLint team, the fact that { 'foo_bar': baz } is allowed by camelcase is a bug.

@pvdlg
Copy link
Contributor

pvdlg commented Jan 7, 2018

Yep. So not sure that quoting the properties not in camel case, as suggested in the present issue, is the right approach...

The best option might to use eslint-disable comment when you have to use a snake case property because an API requires it

@fregante
Copy link
Member

fregante commented Aug 6, 2022

The bugs in the camelcase rule have been fixed and there’s nothing XO can do about allowing some camelcase keys. You can disable the rule globally or inline or, if possible, add some exclusions to the rule’s config.

@fregante fregante closed this as not planned Won't fix, can't repro, duplicate, stale Aug 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants