Add allowEmptyInput, cache, fix options to configuration object

[mattxwang]

Which issue, if any, is this issue related to?

Part of (but does not close) #5142. This PR handles all the boolean flags (which should be relatively simple).

Is there anything in the PR that needs further explanation?

Sorry about the CodeQL alerts - it was an approach that I tried (and chose to abandon) that I forgot to remove 😅

As well as my note below on cache, two quick questions:

1. For the configuration docs, what orders should the options go in? I may have missed a pattern - I was confused since it's not in the same order as the flags.
2. Any tests I should add?

---

Note that I didn't augmentConfig for allowEmptyInput and cache, since we never access them through the config object outside of standalone.js; including them drops code coverage. That being said, maybe there's a cleaner way to use these options "downstream", and be consistent in augmenting the config?

[changeset-bot]

🦋 Changeset detected

Latest commit: 52b2ad5

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
stylelint	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[mattxwang]

In #6402, it sounds like cache may be more complicated than it first seems. I feel like in this case, I must be missing a test case that addresses the complication in the issue (about per-file config resolution). My understanding is that the test cases are when the "global" cache disagrees with a per-file cache; in that case, the per-file cache should win. Is that correct?

(that's not what is implemented in the code right now)

[ybiquitous]

1. For the configuration docs, what orders should the options go in? I may have missed a pattern - I was confused since it's not in the same order as the flags.

I believe there is no clear order. We can improve the doc later, so this PR's change looks good.

[ybiquitous]

@mattxwang Thanks for the pull request. In this good opportunity, I'm reconsidering this feature:

Should CLI flags win config files?

For example, assume we have the following config file named .stylelintrc.json:

{ "cache": true }

Then, should we expect the following? Note that --no- prefix is supported by meow.

# Cache is enabled via config file
stylelint *.css

# Cache is enabled via CLI flag
stylelint *.css --cache

# Cache is disabled via CLI flag
stylelint *.css --no-cache

My answer to the question above is yes. Because people would like to turn on/off caching without editing the config file.

Should we support per-file configs for properties like cache?

For example, assume we have the following config files:

.stylelintrc.json:

{ "cache": true }

special/folder/.stylelintrc.json:

{ "cache": false }

I don't believe such a per-file overriding for global settings makes sense. Lint performance and code maintainability would degrade. Even usability might decrease due to config files everywhere.

So, I recommend supporting only properties in the root config file. We can just ignore properties in sub-folder configs. Of course, we should clearly state the restriction in the document.

What do you think?

Similar discussions around config file

• Add files: [] to the configuration object #3860
• Add cache to the configuration object #6402
• Fix config resolution performance #6383

[mattxwang]

Thanks for the quick response @ybiquitous, I think you bring up some really great points!

Should CLI flags win config files?

For example, assume we have the following config file named .stylelintrc.json:

{ "cache": true }

Then, should we expect the following? Note that --no- prefix is supported by meow.

# Cache is enabled via config file
stylelint *.css

# Cache is enabled via CLI flag
stylelint *.css --cache

# Cache is disabled via CLI flag
stylelint *.css --no-cache

My answer to the question above is yes. Because people would like to turn on/off caching without editing the config file.

I agree with you here (in other words, the CLI flag takes precedence over the config file)! Just implemented the change + have added tests for each of those three cases.

Should we support per-file configs for properties like cache?

I don't believe such a per-file overriding for global settings makes sense. Lint performance and code maintainability would degrade. Even usability might decrease due to config files everywhere.

So, I recommend supporting only properties in the root config file. We can just ignore properties in sub-folder configs. Of course, we should clearly state the restriction in the document.

This makes sense to me as well. Do you think there are other config options that fall into this category? The rest of them feel like they could be enabled/disabled per-file.

If the user has set cache in a subdirectory - should we error, print a message saying that we're ignoring, proceed quietly, or do something else? If this is an exception rather than a rule, I think informing users may be a good idea.

[ybiquitous]

Do you think there are other config options that fall into this category? The rest of them feel like they could be enabled/disabled per-file.

Basically, we should prevent per-file config props from increasing in the future (we have now overrides). I'm not sure which config props support per-file configs, though. Anyway, not touching the existing props might be safe not to break users' linting.

If the user has set cache in a subdirectory - should we error, print a message saying that we're ignoring, proceed quietly, or do something else? If this is an exception rather than a rule, I think informing users may be a good idea.

I believe erroring is best, but if hard to implement it, just documenting the limitation might be enough.

[mattxwang]

Apologies for the delay, got held up with some other work. I think I've addressed most of the code review comments!

To clarify, when you say

Anyway, not touching the existing props might be safe not to break users' linting.

Do you mean the minimal-invasive change (ex, right now - we follow the "default" behaviour", and I've added a little note in the docs)? Or, do you mean that I should explicitly add a subdirectory check?

[ybiquitous]

I think we should add some actions for the new config props in this PR, but I don't think they are needed for the existing props.

[ybiquitous]

Thanks, LGTM 👍🏼

[mattxwang]

Shall I hold off on merging this since #6795 is a patch?

(also, if you think it's appropriate, I could submit a follow-up issue to explore better ways to deal with the subdirectory issue you mentioned)

[ybiquitous]

Let's merge this and include it in the next release. 👍🏼

I've commented on the reason: #6795 (comment)