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

Support providing API with options to use that extend the config it finds #5722

Closed
adalinesimonian opened this issue Nov 18, 2021 · 2 comments

Comments

@adalinesimonian
Copy link
Member

Context: stylelint/vscode-stylelint#328

We're running into an issue downstream in vscode-stylelint in which formatting doesn't work for files that need a customSyntax option to be set. Effectively, the problem is that we need to convert the formatting options (e.g. indent size, spaces or tabs) in VS Code to their respective Stylelint rules (e.g. indentation), then have Stylelint give us fixes for the document using those rules.

However, when we provide the Stylelint API with a config, it stops searching for a configuration file. So when we pass the rules to the API using the config option, Stylelint doesn't load the user's config that has the customSyntax configuration set and throws the When linting something other than CSS, you should install an appropriate syntax, e.g. "postcss-scss", and use the "customSyntax" option error.

I don't think it would make sense to have to duplicate the logic for finding and parsing through config files in the extension. That'd be a significant additional maintenance burden. Instead, perhaps we could find a way to tell Stylelint, using the API, that we want the config we provide it with to override the config it locates instead of replacing it entirely.

There are different ways we could tackle this, so I think we should discuss the right way to handle this. Off the top of my head without any real thought:

const config = {
  rules: {
    indentation: [4],
    'no-missing-end-of-source-newline': true,
    'no-eol-whitespace': true,
  },
};
const code = '…';
const codeFilename = 'test.scss';

/*
Assuming that the resolved config for test.scss is:

{
  extends: ['stylelint-config-standard-scss'],
  customSyntax: 'postcss-scss',
  rules: {
    'max-nesting-depth': null,
    'selector-max-id': null,
  },
}

We'd want the effective config to be:

{
  extends: ['stylelint-config-standard-scss'],
  customSyntax: 'postcss-scss',
  rules: {
    'max-nesting-depth': null,
    'selector-max-id': null,
    indentation: [4],
    'no-missing-end-of-source-newline': true,
    'no-eol-whitespace': true,
  },
}
*/

// Maybe a flag to tell Stylelint the config is meant to override?
stylelint.lint({ code, codeFilename, config, overrideConfig: true })

// Maybe a different property with the config with options to override?
stylelint.lint({ code, codeFilename, override })
@adalinesimonian adalinesimonian added the status: needs discussion triage needs further discussion label Nov 18, 2021
@jeddy3
Copy link
Member

jeddy3 commented Nov 21, 2021

Is is related to the configOverrides option remove in 14.0.0?

I hadn't realised the extension was using this option to merge in VSCode settings.

Is it better not to merge, and keep the Stylelint config as the source of truth?

@adalinesimonian
Copy link
Member Author

It was actually mistakenly using the config option from day 1, so it never worked properly. I forgot about the whole configOverrides thing; if #5723 gets implemented, then we can do what we need to downstream: get the effective config, set the formatting rules using the LSP parameters, then pass it back into Stylelint to format the file as requested. I think we can close this now in favour of that issue.

@adalinesimonian adalinesimonian removed the status: needs discussion triage needs further discussion label Nov 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants