Repository: okonet/lint-staged
Pull request #385 (2018-01-19 22:53:31)

feat(config): add `ignore` config option
This allows users to prevent some files from being processed by lint-staged ever. This is useful if your project contains generated files that you commit to the repository, but don't want to process.

[I know it's not great to commit generated files](https://blog.kentcdodds.com/why-i-don-t-commit-generated-files-to-master-a4d76382564), but in this case it's really the best solution for me.

I didn't open an issue for this because I really need this anyway so if it's not merged then I'll just use my own fork.

codecov[bot]: # [Codecov](https://codecov.io/gh/okonet/lint-staged/pull/385?src=pr&el=h1) Report
> Merging [#385](https://codecov.io/gh/okonet/lint-staged/pull/385?src=pr&el=desc) into [master](https://codecov.io/gh/okonet/lint-staged/commit/e4f0f4438317d2033b0bd6cf2460ee7cdf29252d?src=pr&el=desc) will **increase** coverage by `0.03%`.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/okonet/lint-staged/pull/385/graphs/tree.svg?token=LSoEgsjk4O&src=pr&height=150&width=650)](https://codecov.io/gh/okonet/lint-staged/pull/385?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master     #385      +/-   ##
==========================================
+ Coverage    98.1%   98.13%   +0.03%     
==========================================
  Files          10       10              
  Lines         211      215       +4     
  Branches       23       26       +3     
==========================================
+ Hits          207      211       +4     
  Misses          4        4
```


| [Impacted Files](https://codecov.io/gh/okonet/lint-staged/pull/385?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [src/getConfig.js](https://codecov.io/gh/okonet/lint-staged/pull/385/diff?src=pr&el=tree#diff-c3JjL2dldENvbmZpZy5qcw==) | `100% <ø> (ø)` | :arrow_up: |
| [src/generateTasks.js](https://codecov.io/gh/okonet/lint-staged/pull/385/diff?src=pr&el=tree#diff-c3JjL2dlbmVyYXRlVGFza3MuanM=) | `100% <100%> (ø)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/okonet/lint-staged/pull/385?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/okonet/lint-staged/pull/385?src=pr&el=footer). Last update [e4f0f44...009d5e1](https://codecov.io/gh/okonet/lint-staged/pull/385?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

sudo-suhas: Hey @kentcdodds, thanks for the PR 😄. I think overall, the feature addition makes sense. I have a minor suggestion. Instead of the multiple filter steps, how about we switch over to [`micromatch`](https://github.com/micromatch/micromatch#micromatch)? This can filter files with multiple glob patterns and should be faster too.
kentcdodds: Sounds fine. What would the API look like to add an ignore? The same? Should we maybe do it in a different pull request in that case?
sudo-suhas: I was thinking the API would be the same. We can just concat the ignore patterns(with `!` at the beginning) to the selection criteria and filter in a single step. However, if you have any suggestion for how it could be different, we could consider that. 
kentcdodds: I don't mind making the change. Is this the only place minimatch is used in the codebase?
sudo-suhas: Yes, it is only used in `generateTasks.js`. It's also mentioned in a couple of places in the readme.

However, looks like there are very minor differences in the options for `minimatch` and `micromatch`. The following options are not present in `micromatch`:
- https://github.com/isaacs/minimatch#nocomment
- https://github.com/isaacs/minimatch#flipnegate

So we might need to do this change in a separate PR and make a major release to denote the breaking change. I personally do not think these options would be used by anybody so it should be okay to make this change in a minor version. @okonet Could you take a call on this?
kentcdodds: Yes, I'd prefer to do that in a separate PR. Seems unrelated to the core feature I want to add here :)
okonet: @kentcdodds thanks for your PR. We have a staple d(my fault!) PR that should be able to make the whole setup more flexible here https://github.com/okonet/lint-staged/pull/273

As @sudo-suhas mentioned we want to transition to micromatch and introduce array syntax to `linters` key so you'll be able to add multiple globs and ignore patterns in an obvious way. Do you want to jump in and help with this perhaps?
sudo-suhas: @okonet While having the option to include multiple file filter patterns would satisfy the requirement, I think the feature addition of global ignore is still justified. I think one of the good things is that it doesn't require any changes for existing configuration format. Also, do you think the `minimatch ➜ micromatch` switch requires a major version change?
okonet: Not sure about global ignore tbh if we want to have arrays anyways. Wouldn’t it just add more frustration to have both?

Switching glob engine shouldn’t be a major release unless it’s a breaking change. 
sudo-suhas: > Wouldn’t it just add more frustration to have both?

Wasn't the use-case of being able to ignore files our strongest motivation for introducing the multiple glob patterns option? Also, cascading rules are quite common(ex: `eslint`) so it shouldn't cause any issue in my opinion.

> Switching glob engine shouldn’t be a major release unless it’s a breaking change.

Please see https://github.com/okonet/lint-staged/pull/385#issuecomment-359140964
okonet: @sudo-suhas regarding mircomatch: we should a major release then. No need to take any risks here.

Regarding the `ignore` option: if the only use case is to ignore files globally, when we should not over complicate things and introduce this global ignore patterns option. My concern is that next request will be "that's great but how do I ignore files only for this task" :)

But again, I'm leaving towards solving one problem at the time and it seems like @kentcdodds solution is good enough for now so let's go ahead with it.
okonet: Closes #310 (I guess?)
okonet: The only thing that just popped in my head is that this PR will prevent us from doing https://github.com/okonet/lint-staged/issues/277 which I'd really love to do since I never was a fan of having multiple config formats. So, the question is: should the `ignore` go into linter's items config, perhaps?
sudo-suhas: #277 might make this obsolete in which case, we can drop it from the global options. Since it will be a breaking change, we can just ask users to move the ignore to each linter's options.
okonet: Not sure if this is a more desired approach in this case. I'd prefer to pull off the #273 instead, TBH. I'm wondering what @kentcdodds has to say if proposed changes would solve his use case.

Looks like I'll get some time to work on #62 and #273 soon!
kentcdodds: Looks like #273 was closed?
okonet: I will need to create a new PR but the discussion is still relevant. 
kentcdodds: That looks like it would work fine for me. I just need a way to specify a glob of files I don't want processed.
kentcdodds: What's the status on this? Could we merge it as-is and then iterate to the improvements?
okonet: I still didn’t find to work on it yet. Sorry. 
sudo-suhas: > Could we merge it as-is and then iterate to the improvements?

I second this. Moreover, I have come to have reservations about 'Get rid of advanced config options'. We would be writing ourselves into a corner where _every_ option would have to be at the linters level. This would restrict us in adding options which might make sense to have at the global level. To clarify, I support removing config options like `subTaskConcurrency` but I'd still like to leave the window open for any future requirements.
okonet: Okay let’s go forward with this PR then. @sudo-suhas please go ahead and merge it. 

Patch:
diff --git a/README.md b/README.md
index ec4120b..8e25de0 100644
--- a/README.md
+++ b/README.md
@@ -124,11 +124,13 @@ To set options and keep lint-staged extensible, advanced format can be used. Thi
 
 ## Options
 
-* `linters` — `Object` — keys (`String`) are glob patterns, values (`Array<String> | String`) are commands to execute.
 * `concurrent` — *true* — runs linters for each glob pattern simultaneously. If you don’t want this, you can set `concurrent: false`
 * `chunkSize` — Max allowed chunk size based on number of files for glob pattern. This is important on windows based systems to avoid command length limitations. See [#147](https://github.com/okonet/lint-staged/issues/147)
+* `globOptions` — `{ matchBase: true, dot: true }` — [minimatch options](https://github.com/isaacs/minimatch#options) to 
+customize how glob patterns match files.
+* `ignore` - `['**/docs/**/*.js']` - array of glob patterns to entirely ignore from any task.
+* `linters` — `Object` — keys (`String`) are glob patterns, values (`Array<String> | String`) are commands to execute.
 * `subTaskConcurrency` — `1` — Controls concurrency for processing chunks generated for each linter. Execution is **not** concurrent by default(see [#225](https://github.com/okonet/lint-staged/issues/225))
-* `globOptions` — `{ matchBase: true, dot: true }` — [minimatch options](https://github.com/isaacs/minimatch#options) to customize how glob patterns match files.
 
 ## Filtering files
 
diff --git a/src/generateTasks.js b/src/generateTasks.js
index 5d3fe73..0446922 100644
--- a/src/generateTasks.js
+++ b/src/generateTasks.js
@@ -14,6 +14,12 @@ module.exports = function generateTasks(config, relFiles) {
   const normalizedConfig = getConfig(config) // Ensure we have a normalized config
   const linters = normalizedConfig.linters
   const globOptions = normalizedConfig.globOptions
+  const ignoreFilters = normalizedConfig.ignore.map(pattern => minimatch.filter(pattern))
+  // if there are filters, then return false if the input matches any
+  // if there are not, then return true for all input
+  const ignoreFilter = ignoreFilters.length
+    ? input => !ignoreFilters.some(filter => filter(input))
+    : () => true
 
   const gitDir = resolveGitDir()
   const cwd = process.cwd()
@@ -30,6 +36,7 @@ module.exports = function generateTasks(config, relFiles) {
       .map(file => path.relative(cwd, file))
       // We want to filter before resolving paths
       .filter(filter)
+      .filter(ignoreFilter)
       // Return absolute path after the filter is run
       .map(file => path.resolve(cwd, file))
 
diff --git a/src/getConfig.js b/src/getConfig.js
index f9536ec..a46626b 100644
--- a/src/getConfig.js
+++ b/src/getConfig.js
@@ -27,6 +27,7 @@ const defaultConfig = {
     dot: true
   },
   linters: {},
+  ignore: [],
   subTaskConcurrency: 1,
   renderer: 'update'
 }


Test patch:
diff --git a/test/__snapshots__/getConfig.spec.js.snap b/test/__snapshots__/getConfig.spec.js.snap
index 511c5c6..9e6d960 100644
--- a/test/__snapshots__/getConfig.spec.js.snap
+++ b/test/__snapshots__/getConfig.spec.js.snap
@@ -8,6 +8,7 @@ Object {
     "dot": true,
     "matchBase": true,
   },
+  "ignore": Array [],
   "linters": Object {},
   "renderer": "update",
   "subTaskConcurrency": 1,
@@ -22,6 +23,7 @@ Object {
     "dot": true,
     "matchBase": true,
   },
+  "ignore": Array [],
   "linters": Object {},
   "renderer": "update",
   "subTaskConcurrency": 1,
@@ -36,6 +38,7 @@ Object {
     "dot": true,
     "matchBase": true,
   },
+  "ignore": Array [],
   "linters": Object {
     "*.js": Array [
       "eslint --fix",
diff --git a/test/__snapshots__/index.spec.js.snap b/test/__snapshots__/index.spec.js.snap
index 39f2d21..8f1cdd4 100644
--- a/test/__snapshots__/index.spec.js.snap
+++ b/test/__snapshots__/index.spec.js.snap
@@ -13,6 +13,7 @@ LOG {
     matchBase: true,
     dot: true
   },
+  ignore: [],
   subTaskConcurrency: 1,
   renderer: 'verbose'
 }"
@@ -33,6 +34,7 @@ LOG {
     matchBase: true,
     dot: true
   },
+  ignore: [],
   subTaskConcurrency: 1,
   renderer: 'verbose'
 }"
diff --git a/test/generateTasks.spec.js b/test/generateTasks.spec.js
index b3313bd..013c514 100644
--- a/test/generateTasks.spec.js
+++ b/test/generateTasks.spec.js
@@ -205,4 +205,18 @@ describe('generateTasks', () => {
       )
     })
   })
+
+  it('should ignore patterns in the ignore array of configuration', () => {
+    const pattern = '**/*.js'
+    const commands = 'lint'
+    const result = generateTasks(
+      { ignore: ['**/ignore/**', '**/ignore.*'], linters: { [pattern]: commands } },
+      ['ignore/me.js', 'ignore.me.js', 'cool/js.js']
+    )
+    expect(result[0]).toEqual({
+      pattern,
+      commands,
+      fileList: [`${workDir}/cool/js.js`].map(path.normalize)
+    })
+  })
 })
diff --git a/test/getConfig.spec.js b/test/getConfig.spec.js
index 8f14de8..3ffd0eb 100644
--- a/test/getConfig.spec.js
+++ b/test/getConfig.spec.js
@@ -139,6 +139,7 @@ describe('getConfig', () => {
       linters: {
         '*.js': 'eslint'
       },
+      ignore: ['docs/**/*.js'],
       subTaskConcurrency: 10,
       renderer: 'custom'
     }