Exclude files via configuration rules

[rotu]

Description

Add a new parser named "ignore". This allows excluding files via the existing configuration file using override rules.

This is similar to .prettierignore, but:

1. Can be included directly in .prettierrc, removing a file (config option to ignore paths #4708) and allowing rules to be distributed in plugins as with other configuration
2. Is more monorepo-friendly. Applies based on the location of the file, not the working directory when prettier is invoked (.prettierignore is ignored when inside of a subdirectory #12923)
3. Cascades in the same way as multiple config files, which is more intuitive (Feature: handle .prettierignore location like .gitignore and .npmignore #4081).

This PR does not remove .prettierignore but I recommend it be deprecated in the future.

Checklist

• I’ve added tests to confirm my change works.
• (If changing the API or CLI) I’ve documented the changes I’ve made (in the docs/ directory).
• (If the change is user-facing) I’ve added my changes to changelog_unreleased/*/XXXX.md file following changelog_unreleased/TEMPLATE.md.
• I’ve read the contributing guidelines.

✨Try the playground for this PR✨

[rotu]

I'm not totally happy with having made two not-so-obvious alternate ways to ignore files. I think this makes config easier to understand, but I'd like to eventually see .prettierignore deprecated in favor of this plus vcs integration.

[fisker]

Thanks for your contribution. I don't have a good solution for #4708, but I don't think we can accept this solution either.

[rotu]

@fisker Why don't you think this is acceptable? It doesn't aim to fix #4708 (which should be somewhat more automatic). It does aim to be be a better option over .prettierignore, which has a few footguns (especially e.g. in a monorepo)

[WillsterJohnson]

With all due respect, it's been 5 years since #4708 was requested.

This PR is the only one which has attempted to fulfill even a portion of #4708's request.

If this isn't acceptable, then a top-level option named ignore or exclude of type string[] must be.
If that still isn't, what is?

If there hasn't been any solution for five years, I think this PR qualifies as "good enough". The pain point isn't "we have one additional file in our project", the pain point is "using prettier in a monorepo feels like more hassle than manually formatting files"

Below are the current options for sharing prettier ignores across projects. All of them are worse than what this PR offers.

Multiple .prettierignore files, one per workspace project

This does not require justification as to why it's awful.

Use 3rd party tooling to handle prettier ignores

Declaring this as a recommended method is just declaring prettier an incomplete tool.
Prettier is prettier, not eslint-plugin-opinions, it was built to stand on it's own, and it ought to continue to do so.

Hack other config options

This is objectively worse than what this PR enables. Compare;

{
  "overrides": [
    {
      "files": "dist/**/*",
      "options": { "requirePragma": true },
    },
  ],
  // vs
  "overrides": [
   {
     "files": "dist/**/*",
     "options": { "parser": "ignore" }
    }
  ]
}

This PR makes the config significantly easier to understand, and as an unnecessary bonus it's a few characters shorter.

../../

This is what I'm trying out currently in my monorepo. It's truely awful, and because under packages/ I have some directories for grouping related packages, I constantly end up with an invalid path.

{
  "scripts": {
    "lint": "prettier --ignore-path ../../.prettierignore --check ."
  }
}

---

For monorepos, prettier is more trouble than it's worth.

After this PR, prettier will be the obviously easiest option.

This PR ought to have been merged in 2018.

[31i0t]

Also, run into this issue when working within a monorepo 🙋🏽‍♂️. Was really surprised Prettier didn't have this option yet.

With ESLint, it's as simple as this:

{
    "ignorePatterns": ["temp.js", "**/vendor/*.js"],
    "rules": {
        //...
    }
}

[alexander-akait]

The ideal solution is implement ignorePatterns, but I don't see any problems with "parser": "ignore"

[rotu]

The ideal solution is implement ignorePatterns, but I don't see any problems with "parser": "ignore"

ignorePatterns is necessary for eslint because you can have multiple rulesets and need a way to turn them all off. But a parser precludes other parsers, so you don't need it in the same way.

I considered using "ignore": true instead of "parser": "ignore" but this seems less intrusive and we can always sugar it up later.

[WillsterJohnson]

What are the issues blocking this from being accepted?

[rotu]

@fisker could you please take a look at this and let me know either (1) what I need to to do in order to move this PR forward or (2) that this PR is unwelcome and should be closed? Thank you.

[fisker]

Personally, I really don't like the idea, I'd rather implement ignorePatterns mentioned in #14785 (comment), but if other maintainers think it's okay to add this special parser, I'll be fine.

One important thing worth to mention, current implementation not really "ignore" the file, it will replace EOL(maybe more than this, not sure). And it will read/write files.

[WillsterJohnson]

I agree @fisker, ignorePatterns is the ideal solution. But after 5 years, we don't have it.

I considered using "ignore": true instead of "parser": "ignore" but this seems less intrusive and we can always sugar it up later.

- Rotu, #14785 (comment)

Let's merge this PR so we can have an implementation for this feature request, and work on making it perfect later. It's taken over 5 years (#4708 - June 17 2018) to get to this stage, who knows how long it will be before ignorePatterns is implemented?

From what you've said, the only blocker is;

• ignore parser should fully ignore files (currently may or may not do other things - needs further investigation)

@rotu given that you implemented and tested this PR I think it's best to come to you for this - what are your thoughts/findings on the above inquiry?

[alexander-akait]

One important thing worth to mention, current implementation not really "ignore" the file, it will replace EOL(maybe more than this, not sure). And it will read/write files.

We need to fix this before the merge, ignore should ignores all

[rotu]

The decision to not do ignorePatterns was on purpose, as it's confusing. If I specify both ignorePatterns and a matching pattern with options for some files, it's not clear which should win out (or how multiple matching prettierrc files should interact). Additionally, this makes it equally easy to ignore certain files or ignore all but certain files.

I changed the approach and it now leaves whitespace alone. BUT I think the test fixture is mangling the data before it gets fed into the test case (i.e. the input part of the jsfmt.spec.js.snap file is not what I expect). So I may yet need to adjust the tests.

[WillsterJohnson]

The decision to not do ignorePatterns was on purpose, as it's confusing [...]

This was something I had considered, but I didn't know how to articulate that concern. I suppose it might work similar to the priority .prettierignore has, but providing it as a config option means there may be multiple instances in different contexts, making it difficult to clearly see what is and is not ignored.

Having an ignorant parser is much clearer, and while I stand by agreeing with Fisker that ignorePatterns is the ideal solution, figuring out how exactly that ought to work will take time, and deciding to wait for that could mean we wait a total of a decade for prettier to be viable in monorepos.

ignorePatterns would be great; we seriously cannot afford to wait until it's well defined to have this feature, let's take this PR (assuming the issue found last week is resolved - which it appears to be) and we can deprecate it in a few years time once ignorePatterns has a sensible and clear definition and behavior.

[WillsterJohnson]

Any decision on this? Is it going to be merged or are we going to continue with poor monorepo support?

[WillsterJohnson]

@fisker since you last commented the concern you rose has been addressed by a new commit. Assuming that commit correctly addresses that concern (which it appears to do), can we have this merged and released in the next minor bump?

[fisker]

What do you think we add support for ignore: boolean to overrides? #15322

[rotu]

What do you think we add support for ignore: boolean to overrides? #15322

I don’t quite understand the why. It seems like “ignore” is mutually exclusive with a choice of parser (so it’s clear that when a file is ignored, no parser will be used).

Making it a separate flag makes it orthogonal to choice of parser. Is there a reason to ignore a file AND to specify a parser?

[fredericrous]

Following use case for ignorePatterns not mentioned in this discussion

We have a global prettier config for the whole company. the same way we do for eslint.

our prettier config looks like this:

module.exports = require('@teckro/prettier-config/prettier.config');

Today, I cannot have a global config to ignore build/ , public/ and coverage/ folders.
I am required to create a .prettierignore file per repo. If I want to do a global change to the list of ignored folders, I need to edit each .prettierignore file

This issue is very similar to the mono repo issue.. except it's even more of a burden because everything is split into different repo

[WillsterJohnson]

What do you think we add support for ignore: boolean to overrides? #15322

I agree with @rotu on this.

Additionally, this PR resolves the issue effectively, and given that this issue has been open for over 5 years I don't see any value in continuing the discussion over fixing a problem that's been around - I stress again - for over 5 years.

This PR has been open for a long time, during which precisely one minor issue has been found and swiftly resolved.

In response then to your original question, "What do you think we add support for ignore: boolean to overrides? #15322" I think absolutely not. We have a better solution already, and it comes in the form of merging this PR.

[WillsterJohnson]

An issue with this was raised in #15322 which I actually agree with (#15322 (comment))

To be clear;

I still very much think ignore: true is an anti-pattern we should avoid

However I can't ignore that "parser": "ignore" could actually be a name conflict, or at the very least be misleading to a degree. I think "parser": false resolves this quite well, and would make this PR an absolute no-brainer to merge if it somehow wash't already the better of the two.

[rotu]

I’m rather fond of parser: ignore because 1) it’s more discoverable/searchable 2) it’s less magical (even if we were to admit a false literal in the config file, I’d probably want to desugar it internally since each parser has a unique string identifier.

I say ship it. I just care that this functionality exists, not how you spell the config.

[rotu]

@fisker What needs to happen to get this merged in?

[rotu]

Merged in master, freshened up the description.

@fisker, @sosukesuzuki Could we please get this merged in?