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

Allow configOverrides to be a function #4374

Closed
shawnbot opened this issue Oct 23, 2019 · 10 comments
Closed

Allow configOverrides to be a function #4374

shawnbot opened this issue Oct 23, 2019 · 10 comments
Labels
status: needs discussion triage needs further discussion type: enhancement a new feature that isn't related to rules

Comments

@shawnbot
Copy link
Contributor

This is a feature request for the Node API.

In #4331 I described the need to filter the loaded configuration by one or more rules from the CLI to ease refactoring efforts. One way to do this is to make the configOverrides option more powerful by allowing it to be a function that modifies the loaded configuration. Filtering the rules would be as simple as:

const rules = ['rule/one', 'rule/two']

stylelint.lint({
  files: 'src/**/*.css',
  configOverrides: config => {
    for (const ruleName of Object.keys(config.rules)) {
      if (!rules.includes(ruleName)) {
        delete config.rules[ruleName]
      }
    }
  }
})

Since there's no CLI option for configOverrides, it seems like that might actually be a good way to implement a fix for #4331, right about here:

if (cli.flags.quiet) {
  optionsBase.configOverrides.quiet = cli.flags.quiet;
}

if (cli.flags.rules) {
  // `stylelint --rules foo,bar,baz/qux` → {rules: ['foo', 'bar', 'baz/qux']}
  const ruleNames = cli.flags.rules.split(',')
  const {configOverrides} = optionsBase

  optionsBase.configOverrides = config => {
    for (const ruleName of Object.keys(config.rules)) {
      if (!rules.includes(ruleName)) {
        delete config.rules[ruleName]
      }
    }
    // re-apply {quiet: cli.flags.quiet}
    return Object.assign(config, configOverrides)
  }
}

It looks like the function would need to be called after the config is augmented, and any changes to the config would then need to be resolved ("extended" and "absolutized") again in the same way.

I've done some digging around in the source and I think that I could tackle this if there's any interest. Of course, if there's another way to modify the config after it's been augmented and before it's used in a rule, I would love to hear about that! 🙏

@hudochenkov
Copy link
Member

Thank you for proposal. We have issue for this feature #3128 We can continue discussion there. We would be happy if you would like to contribute this functionality :)

@hudochenkov
Copy link
Member

Sorry, I didn't read your proposal carefully enough.

Having configOverrides as a function is an interesting idea. I have a mixed feelings about having configOverrides as not pure function.

Currently, configOverrides merged into provided config. So if it would be a function it would follow the same logic — mutate config.

On the other side, it would be harder to write and test for user. Pure functions are more safe to use.

I would make configOverrides to accept function, which receives config and returns new config.

// Example of usage
{
    configOverrides: function (config) {
        const newConfig = {
			...config,
			rules: {
				...config.rules,
				indentation: null,
			},
		};

		return newConfig;
	}
}

Actually it could be unpure function, but it should return new config.

@hudochenkov hudochenkov reopened this Oct 27, 2019
@hudochenkov hudochenkov added status: needs discussion triage needs further discussion type: enhancement a new feature that isn't related to rules labels Oct 27, 2019
@hudochenkov
Copy link
Member

We need more opinions from @stylelint/contributors before proceeding to implementation.

@nosilleg
Copy link
Contributor

Is this level of configuration not already possible by calling the cli with --config rewrite.config.js and in the rewrite.config.js file calling the cli again using the default config with --print-config and then manipulating the resulting config data?

@hudochenkov
Copy link
Member

There is no official API to do this, only workarounds.

@nosilleg
Copy link
Contributor

Another way of phrasing that is "We have provided low level tools to allow people to do uncommon things without creating a maintenance burden on the project."

If configOverrides is a function then shouldn't config be a function? Maybe rules should be a function as well? In those situations we have said that users should use the .config.js config files instead. The above enhancement request is achievable the same way.

@hudochenkov
Copy link
Member

@nosilleg good thinking. We might need to provide --print-config as Node.js API then.

@nosilleg
Copy link
Contributor

That gets a 👍 vote from me.

@shawnbot
Copy link
Contributor Author

Nice, thanks for the input @nosilleg! I'd be happy to rework #4377 to expose printConfig() via the Node API if y'all agree that's worth pursuing. It would certainly serve my use case!

@hudochenkov
Copy link
Member

I've create new issue to discuss new direction #4392. I'm closing this issue because it was decided not to go the route proposed initially.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: needs discussion triage needs further discussion type: enhancement a new feature that isn't related to rules
Development

No branches or pull requests

3 participants