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

ESLint wants properties that are constructors to use object literal shorthand. #4487

Closed
qubyte opened this Issue Nov 20, 2015 · 12 comments

Comments

Projects
None yet
6 participants
@qubyte
Copy link

qubyte commented Nov 20, 2015

With the object-shorthand rule active, ESLint complains at the following snippet:

const test = {
  MyConstructor: function () {}
};

It wants it to look like:

const test = {
  MyConstructor () {}
};

The issue is that object literal shorthands cannot be used as constructors, so this transformation is invalid.

Seen with eslint@1.9.0.

@eslintbot

This comment has been minimized.

Copy link

eslintbot commented Nov 20, 2015

Thanks for the issue! If you're reporting a bug, please be sure to include:

  1. The version of ESLint you are using (run eslint -v)
  2. What you did (the source code and ESLint configuration)
  3. The actual ESLint output complete with numbers
  4. What you expected to happen instead

Requesting a new rule? Please see Proposing a New Rule for instructions.

@eslintbot eslintbot added the triage label Nov 20, 2015

@michaelficarra

This comment has been minimized.

Copy link
Member

michaelficarra commented Nov 20, 2015

If you want to use it as a constructor, use class, not function.

@qubyte

This comment has been minimized.

Copy link
Author

qubyte commented Nov 20, 2015

Perhaps, however that doesn't change that the shown ESLint behaviour is in error.

@qubyte

This comment has been minimized.

Copy link
Author

qubyte commented Nov 20, 2015

(assuming the convention that constructors use an initial upper cased character)

@michaelficarra

This comment has been minimized.

Copy link
Member

michaelficarra commented Nov 20, 2015

This rule shouldn't care about casing. If you plan to call it as a function, use a method. If you want to construct it, use a class.

@nzakas

This comment has been minimized.

Copy link
Member

nzakas commented Nov 20, 2015

We can add an option for this.

@michaelficarra

This comment has been minimized.

Copy link
Member

michaelficarra commented Nov 20, 2015

@nzakas Can you elaborate what you mean by "this"?

@nzakas

This comment has been minimized.

Copy link
Member

nzakas commented Nov 20, 2015

Yes, we can add an option that omits uppercase methods from this check.

kaicataldo added a commit to kaicataldo/eslint that referenced this issue Nov 30, 2015

kaicataldo added a commit to kaicataldo/eslint that referenced this issue Nov 30, 2015

kaicataldo added a commit to kaicataldo/eslint that referenced this issue Nov 30, 2015

kaicataldo added a commit to kaicataldo/eslint that referenced this issue Nov 30, 2015

kaicataldo added a commit to kaicataldo/eslint that referenced this issue Nov 30, 2015

kaicataldo added a commit to kaicataldo/eslint that referenced this issue Nov 30, 2015

@nzakas nzakas closed this in 44d2276 Dec 8, 2015

nzakas added a commit that referenced this issue Dec 8, 2015

Merge pull request #4574 from kaicataldo/objectshorthandignoreconstru…
…ctors

Update: Add option to ignore constructor functions in object-shorthand rule (fixes #4487)
@qubyte

This comment has been minimized.

Copy link
Author

qubyte commented Dec 8, 2015

Thanks folks. Greatly appreciated. :)

@nrotta

This comment has been minimized.

Copy link

nrotta commented Feb 17, 2016

I'm currently using the object-shorthand rule to flag the following code as valid (as previously discussed in this thread):

const User = BaseModel.extend({
  constructor: function (attrs) {
    objectAssign(this, attrs);
  }
});

I'm using eslint v1.10.3 and the following is the content of my .eslintrc.js file:

module.exports = {
  extends: "airbnb",
  rules: {
    "comma-dangle": [2, "never"],
    strict: 0,
    "no-console": 0,    
    "object-shorthand": [2, "always", { "ignoreConstructors": true }]
  }
};

However, when running eslint the following error is raised:

Configuration for rule "object-shorthand" is invalid:
    Value "2,always,[object Object]" has more items than allowed.
    at validateRuleOptions (node_modules/eslint/lib/config/config-validator.js:102:15)
    at eslint/lib/config/config-validator.js:148:13
    at Array.forEach (native)
    at Object.validate (node_modules/eslint/lib/config/config-validator.js:147:35)
    at Object.load (eslint/lib/config/config-file.js:390:19)
    at loadConfig (eslint/lib/config.js:74:33)
    at getLocalConfig (eslint/lib/config.js:177:23)
    at Config.getConfig (eslint/lib/config.js:272:22)
    at processText (eslint/lib/cli-engine.js:203:27)
    at processFile (eslint/lib/cli-engine.js:270:18)

Am I missing anything? I would appreciate your help.

@ilyavolodin

This comment has been minimized.

Copy link
Member

ilyavolodin commented Feb 17, 2016

@nrotta ignoreConstructors option was added to v2.0.0 so it's not available for 1.10.3 You would need to upgrade to latest to use this option.

@nrotta

This comment has been minimized.

Copy link

nrotta commented Feb 17, 2016

Works!! @ilyavolodin thanks for the quick response.

@eslint eslint bot locked and limited conversation to collaborators Feb 7, 2018

@eslint eslint bot added the archived due to age label Feb 7, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.