-
Notifications
You must be signed in to change notification settings - Fork 26.5k
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
Reconsider capIsNew: false for new-cap #2200
Comments
To match the behavior of eslint-config-airbnb-base@8.0.0 and before, where functions starting with a capital letter must be called with the new operator. The motivation for setting `capIsNew: false` is not clear to me and the value of catching incorrect constructor calls is non-zero. See: airbnb/javascript#2200 Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
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 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. |
Nope. I recently started invoking
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 How common capitalized functions which should not be invoked with Kevin |
With
eslint-config-airbnb-base@8.0.0
the codewould result in the error:
As a result of #1090, which set
capIsNew: false
fornew-cap
, this is no longer the case.#1089 (comment) which motivated the change only mentions adding
"capIsNewExceptions": ["Immutable.Map", "Immutable.Set", "Immutable.List"]
. Would it make sense to revertcapIsNew: false
(and perhaps adopt"capIsNewExceptionPattern": "^Immutable.\\w"
as suggested in #1106) or was there another rationale for settingcapIsNew: false
?Thanks,
Kevin
The text was updated successfully, but these errors were encountered: