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

no-invalid-this: add switch for function name based heuristic #12271

Closed
Assignees
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

@andidevi
Copy link

andidevi commented Sep 16, 2019

  • ESLint Version: 6.4.0
  • Node Version: 8.16.1
  • npm Version: 6.4.1

What parser (default, Babel-ESLint, etc.) are you using?
babel-eslint

What did you expect to happen?
Detect invalid this in functions with names with leading uppercase

What actually happened? Please include the actual, raw output from ESLint.
The rule no-invalid-this counts functions with leading uppercase in name as constructor.
There is no option to turn this heuristic of.

Reason
When writing React Components as a pure function one likes to keep the leading uppercase in the name. This will lead no-invalid-this to ignore uses of this in the pure function, which is a common error when refactoring call components to pure functions.

This is a request to add a switch to turn off the (documented) heuristic decision, that function is a constructor if

  • the name of the function starts with uppercase, or
  • the function is assigned to a variable which starts with an uppercase letter.
@eslint-deprecated eslint-deprecated bot added the triage An ESLint team member will look at this issue soon label Sep 16, 2019
@andidevi andidevi changed the title no-invalid-this: add switch for heristics no-invalid-this: add switch for function name based heuristic Sep 16, 2019
@mdjermanovic
Copy link
Member

Seems reasonable to me, a project can have a different convention for capitalized functions and not have 'ES5' constructor functions at all.

Also, User-Defined Components Must Be Capitalized

@mdjermanovic mdjermanovic added enhancement This change enhances an existing feature of ESLint evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion rule Relates to ESLint's core rules and removed triage An ESLint team member will look at this issue soon labels Sep 16, 2019
@mdjermanovic
Copy link
Member

I'm willing to champion this, but it still needs one more 👍 from the team.

The name of the option could be capIsConstructor (default true)?

@mdjermanovic mdjermanovic self-assigned this Sep 17, 2019
@mdjermanovic
Copy link
Member

I removed my vote to avoid confusion, as the issue needs 3 👍 from other members to be accepted.

To illustrate the idea, the following would be an error in strict mode (e.g., in a module):

/* eslint no-invalid-this: ["error", { "capIsConstructor": false }] */

function Hello() {
  const { foo } = this.state; // Error: Unexpected 'this' 
}

Setting the option to false says that the project does not use the uppercase convention for constructors and that they would like this rule to treat functions that start with an uppercase as 'regular' functions, i.e. warn if the function that uses this isn't assigned to a property etc. - same logic as for all other functions.

@mysticatea
Copy link
Member

I removed my vote to avoid confusion, as the issue needs 3 👍 from other members to be accepted.

It's okay even if you don't remove your vote.

Because 👍 on GitHub issues includes votes from the community, it's hard to count votes from the team on issues. Instead, we can check champion and supporters from the team in https://github.com/mysticatea/eslint-evaluating-issues#readme.

@g-plane g-plane added accepted There is consensus among the team that this change meets the criteria for inclusion and removed evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion labels Sep 24, 2019
@mdjermanovic
Copy link
Member

I'm working on this.

@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators May 1, 2020
@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 May 1, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.