javascript icon indicating copy to clipboard operation
javascript copied to clipboard

Reconsider capIsNew: false for new-cap

Open kevinoid opened this issue 4 years ago • 2 comments

With [email protected] the code

class Example {}
module.exports = Example();

would result in the error:

example.js:2:18: A function with a name starting with an uppercase letter should only be used as a constructor. [Error/new-cap]

As a result of #1090, which set capIsNew: false for new-cap, this is no longer the case.

https://github.com/airbnb/javascript/issues/1089#issuecomment-249409624 which motivated the change only mentions adding "capIsNewExceptions": ["Immutable.Map", "Immutable.Set", "Immutable.List"]. Would it make sense to revert capIsNew: false (and perhaps adopt "capIsNewExceptionPattern": "^Immutable.\\w" as suggested in #1106) or was there another rationale for setting capIsNew: false?

Thanks, Kevin

kevinoid avatar Mar 22 '20 14:03 kevinoid

This change was made 4 years ago, did you just upgrade now?

You're right that it wasn't discussed in the PR, and the default for this value is true.

I'd be concerned about the large number of exceptions enabling it would force people to add. Do you think this case happens often enough, where it's not tested whatsoever, that it'd be worth it?

I'd be content to switch to the exception pattern to resolve #1106, but I don't think that addresses your question.

ljharb avatar Mar 22 '20 15:03 ljharb

This change was made 4 years ago, did you just upgrade now?

Nope. I recently started invoking eslint with --report-unused-disable-directives and noticed the issue.

I'd be concerned about the large number of exceptions enabling it would force people to add. Do you think this case happens often enough, where it's not tested whatsoever, that it'd be worth it?

I think the rule is useful for my own projects, which is why I enabled the option in my own config. The only exceptions I've added are in tests which invoke constructors without new to test invalid invocations.

How common capitalized functions which should not be invoked with new are in Airbnb code and user code, and whether they comply with Airbnb style guidelines, I can't say. If you think the harm outweighs the gain, feel free to close.

Kevin

kevinoid avatar Mar 22 '20 18:03 kevinoid