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

Documentation bug in max-depth #11991

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 documentation Relates to ESLint's documentation good first issue Good for people who haven't worked on ESLint before Hacktoberfest Recommended issue for those participating in Hacktoberfest https://hacktoberfest.digitalocean.com/ help wanted The team would welcome a contribution from the community for this issue rule Relates to ESLint's core rules

Comments

@imai0917
Copy link

imai0917 commented Jul 15, 2019

The version of ESLint you are using.
v5.16.0

The problem you want to solve.
The below example is from https://eslint.org/docs/rules/max-depth, ESLint v5.16.0 doesn't make issues for the example.

/*eslint max-depth: ["error", 4]*/
/*eslint-env es6*/

function foo() {
    for (;;) { // Nested 1 deep
        let val = () => (param) => { // Nested 2 deep
            if (true) { // Nested 3 deep
                if (true) { // Nested 4 deep
                    if (true) { // Nested 5 deep
                    }
                }
            }
        };
    }
}

Nesting function expressions would not be counted as a nesting depth, so the arrow function expression would not increase the depth count but reset the count.

Your take on the correct solution to problem.
The below example or other equivalents would work.

/*eslint max-depth: ["error", 3]*/

function foo() {
    for (;;) { // Nested 1 deep
        if (true) { // Nested 2 deep
            if (true) { // Nested 3 deep
                if (true) { // Nested 4 deep
                }
            }
        }
    }
}

Are you willing to submit a pull request to implement this change?

@imai0917 imai0917 added core Relates to ESLint's core APIs and features enhancement This change enhances an existing feature of ESLint triage An ESLint team member will look at this issue soon labels Jul 15, 2019
@platinumazure platinumazure added accepted There is consensus among the team that this change meets the criteria for inclusion documentation Relates to ESLint's documentation good first issue Good for people who haven't worked on ESLint before rule Relates to ESLint's core rules and removed core Relates to ESLint's core APIs and features enhancement This change enhances an existing feature of ESLint triage An ESLint team member will look at this issue soon labels Jul 15, 2019
@platinumazure
Copy link
Member

Hi @imai0917, thanks for the issue.

You are right: that is not a good example in the docs and it needs to be changed. Would you like to submit a pull request?

@g-plane
Copy link
Member

g-plane commented Jul 15, 2019

However, the default option of max-depth is 4, so I prefer updating the example with that default option, instead of specifying another value at the example.

@imai0917
Copy link
Author

Yeah, I would submit a pull request.

The below example would work with the default option 4.

/*eslint max-depth: ["error", 4]*/

function foo() {
    for (;;) { // Nested 1 deep
       while (true) { // Nested 2 deep
            if (true) { // Nested 3 deep
                if (true) { // Nested 4 deep
                    if (true) { // Nested 5 deep
                    }
                }
            }
        }
    }
}

imai0917 pushed a commit to imai0917/eslint that referenced this issue Jul 18, 2019
@kaicataldo kaicataldo added Hacktoberfest Recommended issue for those participating in Hacktoberfest https://hacktoberfest.digitalocean.com/ help wanted The team would welcome a contribution from the community for this issue labels Oct 1, 2019
gabrieldrs added a commit to gabrieldrs/eslint that referenced this issue Oct 1, 2019
Signed-off-by: Gabriel R. Sezefredo <g@briel.dev>
Mahesh419 added a commit to Mahesh419/eslint that referenced this issue Oct 2, 2019
Mahesh419 added a commit to Mahesh419/eslint that referenced this issue Oct 2, 2019
gabrieldrs added a commit to gabrieldrs/eslint that referenced this issue Oct 2, 2019
Signed-off-by: Gabriel R. Sezefredo <g@briel.dev>
@jorgeruvalcaba
Copy link

Could this be assigned to @imai0917? I was looking for something to take for Hacktober Fest but this is already fixed.

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 documentation Relates to ESLint's documentation good first issue Good for people who haven't worked on ESLint before Hacktoberfest Recommended issue for those participating in Hacktoberfest https://hacktoberfest.digitalocean.com/ help wanted The team would welcome a contribution from the community for this issue rule Relates to ESLint's core rules
Projects
None yet
5 participants