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

CamelCase option to allow snake_case in object literals #1919

Closed
joemaller opened this issue Mar 2, 2015 · 9 comments
Closed

CamelCase option to allow snake_case in object literals #1919

joemaller opened this issue Mar 2, 2015 · 9 comments
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion enhancement This change enhances an existing feature of ESLint rule Relates to ESLint's core rules

Comments

@joemaller
Copy link

There are situations where third-party code or services are configured by calling a function with an object literal. Those objects are flagged the camelCase rule:

var snakes = require('somesnakelib');  // not my code
snakes({snake_prop: true});  // ESLint: "Identifier 'snake_prop' is not in camel case."

JSCS dealt with this by adding an ignoreProperties option to their CamelCase rule. I like enforcing names and would rather not disable the CamelCase rule to use code that I don't control without errors.

This was touched on in #656, #672, #674 and #1550, but object literals are still being flagged.

Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

@nzakas nzakas added enhancement This change enhances an existing feature of ESLint rule Relates to ESLint's core rules accepted There is consensus among the team that this change meets the criteria for inclusion labels Mar 2, 2015
@nzakas
Copy link
Member

nzakas commented Mar 2, 2015

I'm not sure if ignoring all properties is what you want. For instance you'd also end up ignoring:

var person = {
    say_name: function(){}
};

This is clearly a case where you would want to flag say_name. So maybe what makes the most sense is to add a flag to that ignores properties unless the object contains a method?

@joemaller
Copy link
Author

True, but if it's just as likely that someone might use a function to fetch data to submit to an external service that used snake_case properties in their configuration object.

var person = fetchPerson({
      first_name: function(user) {
        return user.firstName;
      });

Sure that's a contrived example, but it's not difficult to imagine some logic in there. (though, gah.)

Anyway, thanks for considering this.

@nzakas
Copy link
Member

nzakas commented Mar 3, 2015

Hmmm... Maybe we just need a new option properties that can be "always" (default), " never", "nonmethods".

@doberkofler
Copy link
Contributor

This would be great addition to this rule and allow me to get rid of a ton of disable comments

@fflorent
Copy link

👍 I also would like that flag.

Florent

@nzakas nzakas closed this as completed in 3719b40 Mar 18, 2015
nzakas added a commit that referenced this issue Mar 18, 2015
Update: rule camelcase to allow snake_case in object literals. (fixes #1919)
@Tiagojdferreira
Copy link

Was the nonmethods property dropped ?

@OmgImAlexis
Copy link
Contributor

@nzakas any updates on this?

I have the code below yet I'm still getting an error.

var client = tumblr.createClient({
    'consumer_key': config.get('tumblr:token'),
    'consumer_secret': config.get('tumblr:tokenSecret'),
    'token': tokenSet.token,
    'token_secret': tokenSet.tokenSecret
});

I'm using @sindresorhus's xo linter.

    "xo": {
        "space": 4,
        "rules": {
            "space-before-function-paren": 0,
            "camelcase": [
                2, {
                    "properties": "nonmethods"
                }
            ]
        }
    }

@OmgImAlexis
Copy link
Contributor

@nzakas any reason a commit was merged but without the "nonmethods" option?

@MarkAPhillips
Copy link

Is the nonmethods property going to be included? Just has this issue in our code and this property setting would solve this issue.

@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Feb 7, 2018
@eslint-deprecated eslint-deprecated bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Feb 7, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion enhancement This change enhances an existing feature of ESLint rule Relates to ESLint's core rules
Projects
None yet
Development

No branches or pull requests

7 participants