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

configOverrides.extends breaks Node APIs #2280

Closed
veej opened this issue Jan 23, 2017 · 5 comments · Fixed by singapore/lint-condo#230
Closed

configOverrides.extends breaks Node APIs #2280

veej opened this issue Jan 23, 2017 · 5 comments · Fixed by singapore/lint-condo#230
Labels
status: wip is being worked on by someone type: bug a problem with a feature or rule

Comments

@veej
Copy link

veej commented Jan 23, 2017

Describe the issue. Is it a bug or a feature request (new rule, new option, etc.)?

Trying to pass configOverrides: { extends: 'stylelint-config-standard' } to Node API doesn't seem to work (and it seems to break the linting process).

What CSS is needed to reproduce this issue?

.a {}

What stylelint configuration is needed to reproduce this issue?

 {
  code: 'a { }',
  configBasedir: __dirname,
  config: {
    rules: {}
  },
  configOverrides: { extends: 'stylelint-config-standard' },
  syntax: 'scss',
  formatter: 'string'
}

Which version of stylelint are you using?

7.5.0

How are you running stylelint: CLI, PostCSS plugin, Node API?

Running the following script on Node:

import { lint } from 'stylelint';

lint({
  code: 'a { }',
  configBasedir: __dirname,
  config: {
    rules: {}
  },
  configOverrides: { extends: 'stylelint-config-standard' },
  syntax: 'scss',
  formatter: 'string'
})
  .then(({ output }) => console.log(output))
  .catch(err => console.log(err));

Does your issue relate to non-standard syntax (e.g. SCSS, nesting, etc.)?

No

What did you expect to happen?

Stylelint should give something like the following:

<input css 1>
1:3 ✖ Unexpected empty block block-no-empty

What actually happened (e.g. what warnings or errors you are getting)?

literally nothing is printed in console (not even errors)

@veej veej changed the title configOverrides.extends break Node APIs configOverrides.extends breaks Node APIs Jan 23, 2017
@jeddy3
Copy link
Member

jeddy3 commented Jan 23, 2017

@veej Thanks for using the issue template.

I think you can have either a config property or a configOverrides property. It doesn't make sense to have both together.

The difference between the configOverrides and config options is this: If any config object is passed, stylelint does not bother looking for a .stylelintrc file and instead just uses whatever config object you've passed; but if you want to both load a .stylelintrc file and override specific parts of it, configOverrides does just that.

If you can let us know exactly what outcome you're trying to achieve we can best advise you what options you should use?

If you want to stylelint to search for a .stylelintrc file and then override it with a config, use:

{
  code: 'a { }',
  configOverrides: {
    rules: { 
      "block-no-empty": true
    }
  },
  syntax: 'scss',
  formatter: 'string'
}

If you don't want stylelint to search for a .stylelintrc file, then just use config:

{
  code: 'a { }',
  config: {
    rules: { 
      "block-no-empty": true
    }
  },
  syntax: 'scss',
  formatter: 'string'
}

Also, just add extends: "stylelint-config-standard" to your configuration object if you wish to extend it e.g.

{
  code: 'a { }',
  config: {
    extends: "stylelint-config-standard",
    rules: { 
      "block-no-empty": true
    }
  },
  syntax: 'scss',
  formatter: 'string'
}

literally nothing is printed in console (not even errors)

I suspect that's because config is taking precedence over configOverrides and you've no rules defined for config (and so the input string a { } passes the lint). Use the verbose reporter to see what sources were checked e.g.

{
  code: 'a { }',
  config: {
    rules: {}
  },
  syntax: 'scss',
  formatter: 'verbose'
}

@jeddy3 jeddy3 added the status: needs investigation triage needs further investigation label Jan 23, 2017
@jeddy3
Copy link
Member

jeddy3 commented Jan 23, 2017

Umm, having said all that, you might have uncovered a bug.

{
  code: 'a { }',
  configOverrides: { 
    "extends": ["stylelint-config-standard"],
    "rules": {
      "block-no-empty": true,
    }
  },
  syntax: 'scss',
  formatter: 'verbose'
}

Displays nothing.

@veej Is this what you're trying to achieve i.e. override a .stylelintrc file with a partial config that extends stylelint-config-standard?

@davidtheclark Should you be able to extend from configOverrides config partial?

@veej
Copy link
Author

veej commented Jan 23, 2017

Hey @jeddy3, thank you for your reply.
As far as I can see from tests, configOverrides should override config.
Moreover (as you uncovered in your previous post) configOverrides seems to break things even using a .stylelintrc file instead of config property.

What I'm trying to achieve is a sort of library aimed at sharing our basic configs and scripts (like linting) between all our projects. This library will be included in all our projects and should provide, among other things, a script to lint our source files using a basic config, extensible through .stylelintrc files located in the root paths of our projects.
So, in my intentions, the script in the library would make use of configOverrides to provide the main config (through extends), allowing each project to override single rules via their .stylelintrc files.

@davidtheclark
Copy link
Contributor

Should you be able to extend from configOverrides config partial?

I do not think we've written any code to allow extend to work from configOverrides. This should be documented.

I don't think you need it, really, because when you are writing a Node module you should be able to write whatever code you need to put together the config object that you want. That idea makes me think we should deprecate that option: why have a configOverrides option when you could just use JavaScript to put things together according to your whims?

That said, if the only problem we have with configOverrides is extends, then I'd be open to a PR fixing that.

@veej
Copy link
Author

veej commented Jan 24, 2017

@davidtheclark the issue also arise in stylelint-webpack-plugin that, as far I can see, uses the Node APIs.
In webpack config implementing the config merging logic could be a little bit more overwhelming.

@davidtheclark davidtheclark added status: wip is being worked on by someone type: bug a problem with a feature or rule and removed status: needs investigation triage needs further investigation labels Jan 28, 2017
davidtheclark pushed a commit that referenced this issue Jan 28, 2017
davidtheclark added a commit that referenced this issue Jan 29, 2017
sergesemashko pushed a commit to sergesemashko/stylelint that referenced this issue Mar 3, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: wip is being worked on by someone type: bug a problem with a feature or rule
Development

Successfully merging a pull request may close this issue.

3 participants