Lint-staged: Clarify `gitDir` usage?

[FezVrasta]

I spent quite some time trying to figure out why my lint-staged config didn't work:

  "lint-staged": {
    "gitDir": "../",
    "linters": {
      "src/**/*.{js,jsx,json}": [
        "prettier-eslint --write --single-quote",
        "git add"
      ]
    }
  },

Then I tried to define packages/myapp/src/**/*{js,jsx,json} and it worked.

May you add somewhere in the README this information? (that any pattern must use as root the ones defined as gitDir)

[sudo-suhas]

Hey @FezVrasta, Could you please share your directory structure?

[okonet]

Hey @FezVrasta! Thanks for reporting it. Is that something specific to how minimatch handles that? Could it be resolved by using different set of options? https://github.com/isaacs/minimatch#options

[FezVrasta]

So, the project looks like a classic lerna repository:

- .git
- packages/
  - project1/
  - project2/

The only thing that is confusing is that any glob pattern I'm going to use inside packages/project1/package.json.lint-staged.linters will work with / as root path instead of packages/project1/ how I would expect in a "non gitDir" situation.

I think the behavior is fine, I'd just like to have this information clearly available in the README to avoid others to spend time trying to figure out why it doesn't work for them

[FezVrasta]

RE:

@FezVrasta I'm not sure about that... Why do you think so?

Let's keep this discussion inside your original issue.

Because otherwise people will have to (anyway) define in their glob patterns the project path relative to the git root.

[okonet]

I think the behavior is fine

Well, I think we can be smarter about it when we have gitDir set. We could make globs relative to the gitDir package.json. Do you think that would be an improvement?

[FezVrasta]

globs are already relative to the gitDir:

- .git
- packages/
  - project1/
    - package.json
    - src
      - index.js
  - project2/

src/**/*.js will not match anything if gitDir is set to ../../, you will have to use packages/project1/src/**/*.js to make it match something.

If gitDir is not set (default to .), you can use src/**/*.js

[okonet]

Oh sorry I meant relative to package.json of course...

[sudo-suhas]

I just tested this out and I think the current behavior needs to be changed. When I am defining the lint-staged config inside a package, I shouldn't have to think about relative paths from a parent folder. And I like the idea of automatically detecting the git directory. That should kill a ton of birds in one shot 😛

[Arhane]

What if after cosmicconfig() there will be a check for gitDir?

I have the idea of using git rev-parse --show-toplevel. But it will return the absolute path of git folder. Will it be useful in this case?

[okonet]

I'm closing this one in favor of #271. Please add your comments there.

[sudo-suhas]

@okonet Other than the gitDir option being ignored, there are implications for glob patterns as well.
For example consider a project with the following file structure:

`-- packages
    |-- prj
    |   |-- package.json
    |   |-- src
    |   |   `-- index.js
    |   `-- yarn.lock
    `-- prj-2
        `-- file

With [email protected], the config would need to be something like this:

gitDir: ../..
linters:
  packages/prj/src/*.js:
    - eslint --fix
    - git add

lint-staged output

λ yarn run lint-staged
yarn run v1.3.2
warning ..\..\..\package.json: No license field
$ lint-staged
 > Running tasks for packages/prj/src/*.js
   × eslint --fix
     git add
× eslint --fix found some errors. Please fix them and try committing again.

E:\Projects\experiments\test-lint-staged\packages\prj\src\index.js
  4:5  error  Unexpected console statement  no-console

✖ 1 problem (1 error, 0 warnings)

With lint-staged@5, this simplifies to:

@@ -1,5 +1,4 @@
-gitDir: ../..
 linters:
-  packages/prj/src/*.js:
+  src/*.js:
     - eslint --fix
     - git add

lint-staged output

λ yarn run lint-staged
yarn run v1.3.2
warning ..\..\..\package.json: No license field
$ lint-staged
 > Running tasks for src/*.js
   × eslint --fix
     git add
× eslint --fix found some errors. Please fix them and try committing again.

E:\Projects\experiments\test-lint-staged\packages\prj\src\index.js
4:5  error  Unexpected console statement  no-console

✖ 1 problem (1 error, 0 warnings)

Can we update the release notes to explain this?

[okonet]

@sudo-suhas sure. Could you create a PR for that? Is that ever needed to be clarified since the new way is much more logical?

_{Sent with GitHawk}

[sudo-suhas]

Apologies, I somehow missed replying to this. Since release notes serve as our migration guide, it would make sense to show all the changes required for upgrading to lint-staged@5. I have updated the release notes, please take a look.

If you want to include this information in the readme as well, let me know. I can make a PR.

[okonet]

Thanks @sudo-suhas! You're awesome!

[sudo-suhas]

Thanks @okonet, I appreciate it 😄