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 files: [] to the configuration object #3860

Open
Tracked by #5142
sindresorhus opened this issue Dec 19, 2018 · 14 comments
Open
Tracked by #5142

Add files: [] to the configuration object #3860

sindresorhus opened this issue Dec 19, 2018 · 14 comments
Labels
status: needs discussion triage needs further discussion

Comments

@sindresorhus
Copy link

What is the problem you're trying to solve?

In my projects, I prefer to always glob for all CSS files **/*.css and rather ignore the few CSS files I want instead of explicitly specify every file. When having to specify the files, it's easy to forget some files, especially when they're added later.

What solution would you like to see?

I would like to see a top-level setting called files that accepts an array of strings. I could then set files: ['**/*.css'], in my shareable config and use the ignoreFiles in my projects to ignore project-specific files.


There are other benefits to specifying the files in a config object:

  • The run script becomes less messy.
    Before: {"lint": "xo && stylelint foo.js bar.js 'fixtures/*.css'"}
    After: {"lint": "xo && stylelint"}
  • People often forget to quote the globs, which can lead to weird inconsistencies, since if you don't quote them, they're interpreted as shell globs instead of being passed in as strings to stylelint and its glob handling. By specifying the files in the stylelint config, that's not a problem.
  • Editor plugins can read the files config and know which files to lint. It cannot reliably parse the run script commands.

I have had files config in both the AVA and XO projects for years with great success.


This is a duplicate of #2580, but I chose to open a new issue as I feel I have a different use-case and many more arguments for why it makes sense.

@alexander-akait
Copy link
Member

Sounds reasonable, strange what eslint doesn't support this (and other feature like #3858 and #3859) 😕

@jeddy3
Copy link
Member

jeddy3 commented Dec 27, 2018

Thanks for listing out the benefits and noting a similar approach in AVA and XO.

This sounds like a good idea. It's related to #3411, which is also about improving the experience of using stylelint.

@jeddy3 jeddy3 changed the title Support specifying input files/globs in the config object Add support for files: [] the configuration object Dec 27, 2018
@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 good first issue is good for newcomers labels Dec 27, 2018
@jeddy3 jeddy3 changed the title Add support for files: [] the configuration object Add support for files: [] to the configuration object Dec 27, 2018
@jeddy3 jeddy3 removed the good first issue is good for newcomers label Jan 19, 2022
@Mouvedia
Copy link
Contributor

Mouvedia commented Feb 8, 2023

  1. should files support negation? (dupe of existing ignorefiles)
    • if it's supported does the cli input get the priority?
      which list overwrite or merge onto the other?
      e.g. "!foo.css" from the cli "foo.css" in files
      should ignorefiles be deprecated?
  2. what's the merger strategy for ignored files/directories?
    • should .stylelintignore + ignorefiles be an union or a concat?
    • take --ignore-path and --ignore-pattern into account before/after the merger
  3. mergeConfigs function doesn't have a case for ignorefiles; should it be a set or a add (i.e. reassign or merge) in cases of extends
  4. does https://stylelint.io/user-guide/usage/node-api/#files become optional if code or files (from config) is provided?

I guess no one has a clue so Ill just use common sense and pick what seems the best if I ever work on this one.

@ybiquitous ybiquitous added status: needs discussion triage needs further discussion and removed status: ready to implement is ready to be worked on by someone type: enhancement a new feature that isn't related to rules labels Mar 1, 2023
@ybiquitous
Copy link
Member

I've reverted the status to needs discussion because I think this feature may conflict with #3411. See also the discussion on #6617.

@ybiquitous
Copy link
Member

Also, we need to consider @Mouvedia's questions on #3860 (comment).

@jeddy3
Copy link
Member

jeddy3 commented Mar 15, 2023

Labelling as ready to implement following the discussion in #6689.

Picking up #6689 (comment)

Let's support strings and an array of strings to be consistent with the other options, including all secondary options for rules. In hindsight, accepting strings was probably a mistake, but we have consistently done it for our options. We can consider moving to arrays only in a major release.

@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 15, 2023
@jeddy3 jeddy3 changed the title Add support for files: [] to the configuration object Add files: [] to the configuration object Mar 15, 2023
@ybiquitous
Copy link
Member

I try responding to some questions in #3860 (comment).

should files support negation? (dupe of existing ignorefiles)

I think it makes sense to allow negation patterns for files because detecting a negation in a glob pattern may be difficult and need extra parsing.

There should be no problems even if people use both negation patterns in files and ignoreFiles at their own risk. Perhaps such use cases are very few.

should ignorefiles be deprecated?

I think we should continue to support ignoreFiles for backward compatibility as long as there are no inconveniences.

@silverwind
Copy link

Would a pattern **/*.css include files like node_modules/foo/index.css? If not, by which mechanism is node_modules excluded?

@ybiquitous
Copy link
Member

No, the current behavior (ignoring node_modules by default) will be kept:

const ALWAYS_IGNORED_GLOBS = ['**/node_modules/**'];

@Mouvedia
Copy link
Contributor

Is this issue ready to be implemented?

@ybiquitous
Copy link
Member

If we introduce the flat config (#7408), this issue no longer seems to be needed; otherwise, at least, it seems to need to reconsider an expected behavior.

@jeddy3 jeddy3 added status: needs discussion triage needs further discussion and removed status: ready to implement is ready to be worked on by someone type: enhancement a new feature that isn't related to rules labels Dec 30, 2023
@jeddy3
Copy link
Member

jeddy3 commented Dec 30, 2023

There's quite a bit to consider here. We want to enable {"lint": "xo && stylelint"} while also being compatible with a future flat config. I believe this is possible. With their larger resources, the ESLint team has put a lot of legwork into designing and testing their flat config. We can save time by mimicking their choices.

We can use files: [] as an opt-in for that behaviour, i.e.:

module.exports = {
  "files": ["**/*.css"],
  "rules": { "color-no-named": true }
}

Would:

The latter makes the Node.js API files: [] optional.

We may also want to add an ignores: [] configuration property that has a different behaviour to ignoreFiles: [], i.e. it supports unignoring files through negation rather than removing the defaults. If we did, files: [] would also disable ignoreFiles: [].

The behaviour that'd be triggered by files: [] feels more straightforward to reason about compared to our current mishmash of ignoreFiles: [] and .stylelintignore/--ignore-pattern.

We'll need to be clear in our documentation that using files: [] in the configuration object opts into this new behaviour.

Do we think this approach is worth exploring? If so, we'll likely want to put together a PoC PR to assess whether it's possible to have the current and new behaviours work alongside each other and the code complexity of doing so.

@Mouvedia
Copy link
Contributor

We may also want to add an ignores: [] configuration property that has a different behaviour to ignoreFiles: []

Can you separate this into its own issue and punt it to v17?

@jeddy3
Copy link
Member

jeddy3 commented Jan 7, 2024

Can you separate this into its own issue

Done in #7449.

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
Development

No branches or pull requests

7 participants