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

Documenting configuration hierarchy for CLIEngine options and 'useEslintrc: true' #9526

Closed
edmorley opened this issue Oct 27, 2017 · 7 comments · Fixed by Urigo/tortilla#62, mono-js/mono-notifications#5, mono-js/mono-push#5 or terrajs/lib-starter#5 · May be fixed by ali8889/emerald-wallet#4
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 core Relates to ESLint's core APIs and features documentation Relates to ESLint's documentation good first issue Good for people who haven't worked on ESLint before

Comments

@edmorley
Copy link
Contributor

edmorley commented Oct 27, 2017

Hi! Many thanks for a great tool :-)

I was trying to work out from the docs, which options take precedence in the following example:

// Some tool
var CLIEngine = require("eslint").CLIEngine;

var cli = new CLIEngine({
    // ...
    useEslintrc: true,
    rules: {
        semi: "error"
    }
});
// .eslintrc
{
    "rules": {
        "semi": "off"
    }
}

The CLIEngine docs just say:

useEslintrc - Set to false to disable use of .eslintrc files (default: true). Corresponds to --no-eslintrc.

And the docs on configuration hierarchy/precedence don't mention CLIEngine at all:

The complete configuration hierarchy, from highest precedence to lowest precedence, is as follows:

  1. Inline configuration
    1. /*eslint-disable*/ and /*eslint-enable*/
    2. /*global*/
    3. /*eslint*/
    4. /*eslint-env*/
  2. Command line options:
    1. --global
    2. --rule
    3. --env
    4. -c, --config
  3. Project-level configuration:
    1. .eslintrc.* or package.json file in same directory as linted file
    2. Continue searching for .eslintrc and package.json files in ancestor directories (parent has highest precedence, then grandparent, etc.), up to and including the root directory or until a config with "root": true is found.
    3. In the absence of any configuration from (1) thru (3), fall back to a personal default configuration in ~/.eslintrc.

From experimentation, I've found that the options passed to CLIEngine take priority over the ones in .eslintrc, however I think it would be good to document this.

I'm happy to open a PR to do so, but it would be useful to know whether I should do so on the configuration page, the API page, or both?

Many thanks!

@eslint-deprecated eslint-deprecated bot added the triage An ESLint team member will look at this issue soon label Oct 27, 2017
@platinumazure
Copy link
Member

platinumazure commented Oct 27, 2017

Hi @edmorley, thanks for the issue!

The CLIEngine rules object option should be equivalent to the CLI --rule option, so in one sense, we already document that the CLIEngine rules object should have higher precedence than the configuration files. But I agree it could be made more clear. 😄 Maybe we should list the equivalent CLIEngine options in the Configuration Cascading/Hierarchy section? I'm not really sure.

@eslint/eslint-team Anyone else have any thoughts?

@platinumazure platinumazure added accepted There is consensus among the team that this change meets the criteria for inclusion core Relates to ESLint's core APIs and features documentation Relates to ESLint's documentation and removed triage An ESLint team member will look at this issue soon labels Oct 27, 2017
@edmorley
Copy link
Contributor Author

Ah true :-)

Perhaps replacing:

  1. Command line options:

...with:

  1. Command line options (or CLIEngine equivalents):

...might be sufficient?

@platinumazure
Copy link
Member

@edmorley I'm so sorry for letting this slip through the cracks. I agree, we should just replace that header as you suggested in your comment. That would be a good start.

I'll write a PR in a few days if nobody beats me to it.

@platinumazure platinumazure added the good first issue Good for people who haven't worked on ESLint before label Jan 16, 2018
@PiIsFour
Copy link
Contributor

Hi, I'm new to this. But would like to give it a try, if that's ok with you all.

@platinumazure
Copy link
Member

@PilsFour Absolutely, please go ahead! If you haven't already, please check out our Contribution Guidelines on our website (I'll edit a link in here when I get to a PC).

If it's not too much trouble, please just leave a comment here when you're starting work (or planning to work) on this. If something comes up and you can no longer work on this, just let us know. Absolutely no rush and no pressure either way; we just want to communicate with other people on this issue if the status changes.

Thanks again for volunteering, and let us know if we can help with anything. Good luck!

PiIsFour added a commit to PiIsFour/eslint that referenced this issue Jan 17, 2018
@PiIsFour
Copy link
Contributor

@platinumazure I have made the change to the documentation. Under Configuration Cascading and Hierarchy the preview looks fine to me. But being my first change I'm a bit unsure if thats fine. Could you please take a look if the commit is ok?

I did try to run: npm run docs But that failed on my computer, is that necessary for this change?

Thank you for your help.

@platinumazure
Copy link
Member

@PiIsFour The change looks good to me. Don't worry, our Travis Continuous Integration service will check for issues when you create the pull request.

I think npm run docs has been broken for some time. I don't use it at all. We have a separate script which will copy the documentation from ESLint to eslint.github.io as part of our release.

PiIsFour added a commit to PiIsFour/eslint that referenced this issue Jan 17, 2018
Update documentation about configuration hierarchy for CLIEngine options
PiIsFour added a commit to PiIsFour/eslint that referenced this issue Jan 17, 2018
Update documentation about configuration hierarchy for CLIEngine options
PiIsFour added a commit to PiIsFour/eslint that referenced this issue Jan 17, 2018
Update documentation about configuration hierarchy for CLIEngine options
@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Jul 18, 2018
@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 Jul 18, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.