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

Add overrides property to configuration object #3128

Closed
Grawl opened this issue Jan 15, 2018 · 30 comments · Fixed by #5521
Closed

Add overrides property to configuration object #3128

Grawl opened this issue Jan 15, 2018 · 30 comments · Fixed by #5521
Labels
status: ready to implement is ready to be worked on by someone status: wip is being worked on by someone type: enhancement a new feature that isn't related to rules
Milestone

Comments

@Grawl
Copy link
Contributor

Grawl commented Jan 15, 2018

I want to set different rules to lint .css and .sass files. It's different syntaxes.

These rules are not compatible with .sass indented syntax:

So I want to disable them for .sass files, but control them for .css

I created a sandbox repository to test out postcss-sass parser with Stylelint, so you can uncomment any incompatible rules mentioned above to see it's false-positives

https://github.com/Grawl/stylelint-sass-test

"stylelint": "^8.2.0"

My propose for configuration:

{
    "rules": {
        "optionOne": true, 
        "optionTwo": false
    },
    "overrides": {
        "syntax": "sass",
        "rules": {
            "optionOne": false,
            "optionTwo": true
        }
    }
}

Prettier have this option: https://prettier.io/docs/en/configuration.html#configuration-overrides

#2486 #2593

@ai
Copy link
Member

ai commented Jan 15, 2018

I need this feature too. I have different roles for CSS and CSS-in-JS files.

I like overrides from ESLint. We could also take it to define different rules for different file patterns.

@Grawl
Copy link
Contributor Author

Grawl commented Jan 15, 2018

We could also take it to define different roles for different file patterns.

Can we use different config files in different folders?

@ai
Copy link
Member

ai commented Jan 15, 2018

@Grawl not in my case. I have many folders at root. As result, right now I must copy same config to 3-5 places. And it force mistakes — change config in one folder and forget about other folders.

@Grawl
Copy link
Contributor Author

Grawl commented Jan 15, 2018

Appreciate. But we don't have "files" option in Stylelint as I know.

@hudochenkov
Copy link
Member

Overrides is a good idea.

As a temporary solution you can use config extends. Create a second config for .sass files, extend it from the main config and disable unwanted rules:

.stylelintrc.sass.js:

module.exports = {
    extends: './.stylelintrc.js',
    rules: {
        'block-closing-brace-empty-line-before': null,
    },
};

It will require two npm tasks to run it, though:

stylelint "**/*.css"
stylelint "**/*.sass" --config .stylelintrc.sass.js

Or it can be combined into one:

stylelint "**/*.css" && stylelint "**/*.sass" --config .stylelintrc.sass.js

@hudochenkov hudochenkov added the status: needs discussion triage needs further discussion label Jan 15, 2018
@Grawl
Copy link
Contributor Author

Grawl commented Jan 15, 2018

nice workaround!

@jeddy3
Copy link
Member

jeddy3 commented Mar 14, 2018

It sounds like an overrides configuration property would be useful. I'll label up the issue accordingly.

@jeddy3 jeddy3 added status: ready to implement is ready to be worked on by someone type: enhancement a new feature that isn't related to rules and removed status: needs discussion triage needs further discussion labels Mar 14, 2018
@jeddy3 jeddy3 changed the title Syntax dependent rules Add overrides property to configuration object Mar 14, 2018
wKich added a commit to skbkontur/retail-ui that referenced this issue Dec 17, 2018
@gucong3000
Copy link
Member

I like overrides from ESLint. We could also take it to define different rules for different file patterns.

https://eslint.org/docs/user-guide/configuring#configuration-based-on-glob-patterns

@TheJaredWilcurt
Copy link

TheJaredWilcurt commented Feb 11, 2019

This is a great idea, but the proposed implementation needs improved:

{
  "rules": {
    "optionOne": true,
    "optionTwo": false
  },
  "overrides": {
    "syntax": "sass",
    "rules": {
      "optionOne": false,
      "optionTwo": true
    }
  }
}

What if you have more than just one syntax you want to specify unique rules for? For example if you are linting documentation with markdown examples in multiple meta-languages, or converting a codebase over from Less to Sass.

A better implementation could handle this situation by specifying the file extension for the overrides to apply to. This makes the stuff outside of the overrides section the global settings, and then each filetype has it's own settings that inherit the globals, but can also override them. This also keeps the file backwards compatible.

{
  "rules": {
    "optionOne": true,
    "optionTwo": false
  },
  "overrides": {
    ".sass": {
      "rules": {
        "optionOne": false,
        "optionTwo": true
      }
    },
    ".less": {},
    ".styl": {},
    ".scss": {},
    ".sss": {},
    ".css": {}
  }
}

I understand that stylelint may be linting a syntax in a markdown file or something else, and not a literal .sass file. These override codes (.sass, .scss, .styl etc) are just to reference which syntax the rules will apply to, not the files specifically. This will help to avoid confusion around "Sass" being a language which refers to two syntaxes, or Stylus being referenced in markdown codeblocks as styl. It becomes more intuitive and predictable to use the designated files extensions.

Alternatively, there could be logic to handle multiple strings referencing the same syntax, if you wanted to make the API more loose so people could name them any version of the syntax/meta-language name.

if (stylelintrc.styl || stylelintrc['.styl'] || stylelintrc.stylus || stylelintrc.Stylus) {
  // apply stylus specific overrides
}

@jeddy3
Copy link
Member

jeddy3 commented Feb 12, 2019

@TheJaredWilcurt Thanks for chiming in!

but the proposed implementation needs improved

Yep, I think we should adopt (as mentioned in #3128 (comment)) the ESLint/Prettier approach of an array of overrides targetting globs of files e.g.

{
  "rules": {
    "declaration-block-trailing-semicolon": true,
    "at-rule-no-unknown": true,
  },
  "overrides": [
    {
      "files": ["*.sss", "3rd-party/*.sass"],
      "rules": {
        "declaration-block-trailing-semicolon": false
      }
    }, {
      "files": ["*.less", "*.sass", "*.scss", "*.my-custom-extension"],
      "rules": {
         "at-rule-no-unknown": false,
      }
    }
  ]
}

I understand that stylelint may be linting a syntax in a markdown file or something else, and not a literal .sass file. These override codes (.sass, .scss, .styl etc) are just to reference which syntax the rules will apply to, not the files specifically.

I think this might be tricky and could muddy the waters. I think we should start with file globs for now as I believe that will address the most common use cases. codeFilename is also already part of the stylelint's Node.js API, which could make implementing this functionality in editors easier.

Once we have the files glob functionality in place, then we can have a think about what we can do, if anything, about the syntax use case e.g. <style lang="scss"> and markdown fences.

@TheJaredWilcurt
Copy link

I'm down with that!

@soullivaneuh
Copy link

@jeddy3 proposition with file globing is the best for flexibility.

@soullivaneuh
Copy link

Well, maybe not. Syntax definition may be more suitable for special file like svelte template:

see: https://github.com/sveltejs/template/blob/4e3a4089b4f0275964eb10a432dc1c15526a0b4d/src/App.svelte

@Grawl
Copy link
Contributor Author

Grawl commented Oct 7, 2019

The same for Vue SFC

nex3 added a commit that referenced this issue Feb 4, 2021
This will also make it easier to control disables on a file-by-file
basis once #3128 is implemented. It will also make it possible to add
finer-grained configuration to these rules in the future.
nex3 added a commit that referenced this issue Feb 4, 2021
This will also make it easier to control disables on a file-by-file
basis once #3128 is implemented. It will also make it possible to add
finer-grained configuration to these rules in the future.
nex3 added a commit that referenced this issue Feb 4, 2021
This will also make it easier to control disables on a file-by-file
basis once #3128 is implemented. It will also make it possible to add
finer-grained configuration to these rules in the future.
jeddy3 pushed a commit that referenced this issue Feb 6, 2021
* Remove unused disable-reporting code

This was outmoded by #4973

* Allow disable reporting to be controlled from the config file

This will also make it easier to control disables on a file-by-file
basis once #3128 is implemented. It will also make it possible to add
finer-grained configuration to these rules in the future.

* Fix a typo

Co-authored-by: Jennifer Thakar <jathak@google.com>

* Code review changes

Co-authored-by: Jennifer Thakar <jathak@google.com>
This was referenced May 13, 2021
@jeddy3
Copy link
Member

jeddy3 commented May 18, 2021

It'll help users migrate to 14.0.0 if we add an overrides property that supports customSyntax to the configuration object. It needn't support anything else to start. Users reliant on postcss-syntax's syntax inferring will then have something to migrate to.

For example, someone linting SCSS and CSS:

npx stylelint "**/*.{css,scss}"
{
  "overrides": [
    {
      "files": ["**/*.scss"],
      "customSyntax": "postcss-scss"
    }
  ]
}

@hudochenkov
Copy link
Member

I think it's a good idea to have it in 14.0.0. Should we put it in a milestone?

@jeddy3
Copy link
Member

jeddy3 commented May 18, 2021

Should we put it in a milestone?

Oops, I added to the release issue checklist but not the milestone.

@jeddy3 jeddy3 added this to the future-major milestone May 18, 2021
@Airkro
Copy link
Contributor

Airkro commented Jun 3, 2021

How is that work in vue?

<!-- example.vue -->
<style lang="scss"></style>
<style lang="less"></style>

@jeddy3
Copy link
Member

jeddy3 commented Jun 6, 2021

How is that work in vue?

In terms of custom syntax? Someone from the Vue community will need to write a postcss-vue PostCSS syntax, like vue-eslint-parser, that will handle the syntax switching, and bundle postcss-scss, postcss-less etc.

In terms of the overrides property? Not sure yet, but it's an edge case that only impacts users who use two different styling languages within their .vue files. The proposal is to start by adding the overrides property with the files and customSyntax properties within that. This will address most use cases and once that's in place we can investigate adding support for targeting nested syntaxes.

Once the overrides property with the files and customSyntax properties are in place, and the postcss-vue created, you'll use the following if you're writing both .css and single-language .vue files:

{
  "overrides": [
    {
      "files": ["**/*.vue"],
      "customSyntax": "postcss-vue"
    }
  ]
}
npx stylelint "**/*.{css,vue}"

And simply the following if you're only writing .vue files:

{
   "customSyntax": "postcss-vue"
}
npx stylelint "**/*.vue"

In both cases, you'd configure your stylelint rules for whatever is the nested syntax.

@itsdouges
Copy link

Fantastic! This will also help large mono repo configurations be able to incrementally roll out changes to packages instead of a big bang.

@AndreasNasman
Copy link

I'm currently implementing Stylelint in a huge monorepo and having rule overrides per files/paths would help us segregate rules to be warnings in existing files and errors in new files.

The incidence number of certain non-fixable rules we'd chosen is so high that it's practically impossible to fix them all manually. A good workaround for us would be to mark the violations in the old files as warnings, but have the rules act as errors in files created in the future.

@jeddy3
Copy link
Member

jeddy3 commented Sep 4, 2021

@AndreasNasman Thanks for chiming in with your use case. It sounds like combining overrides with defaultSeverity would be ideal for you.

Please consider contributing if you have time, as the overrides feature is one of two new features in the way of us releasing 14.0.0 (the other tasks are mostly refactoring and documentation).

@jeddy3 jeddy3 mentioned this issue Sep 7, 2021
23 tasks
@hudochenkov hudochenkov added the status: wip is being worked on by someone label Sep 11, 2021
hudochenkov added a commit that referenced this issue Sep 12, 2021
@ybiquitous ybiquitous linked a pull request Sep 13, 2021 that will close this issue
@jeddy3
Copy link
Member

jeddy3 commented Sep 14, 2021

Done in #5522

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: ready to implement is ready to be worked on by someone status: wip is being worked on by someone type: enhancement a new feature that isn't related to rules
Development

Successfully merging a pull request may close this issue.