Lint-staged: Glob issues with 4.2.2 and Prettier

[ltegman]

Starting with lint-staged version 4.2.2 I have glob issues when using with husky and prettier. Here's my lint-staged config:

"lint-staged": {
  "{app,test,config}/**/*.js": [
    "prettier --single-quote --tab-width 4 --print-width 100 --no-bracket-spacing --write",
    "eslint --fix --max-warnings 0",
    "git add"
  ]
}

And here's the error from prettier:

husky > npm run -s precommit (node v6.11.3)

 ❯ Running tasks for {app,test,config}/**/*.js
   ✖ prettier --single-quote --tab-width 4 --print-width 100 --no-bracket-spacing --write
     → TypeError: patterns must be a string or an array of strings
     eslint --fix --max-warnings 0
     git add
✖ prettier --single-quote --tab-width 4 --print-width 100 --no-bracket-spacing --write found some errors. Please fix them and try committing again.

Unable to expand glob patterns: {app,test,config}/**/*.js 4 100 /home/user/dev/my-app/app/entity/myEntity.js !**/node_modules/** !./node_modules/**
TypeError: patterns must be a string or an array of strings

Anyways, thanks for making this awesome tool and if I have time this weekend I'd be glad to look into this problem myself, just wanted to get this out for others to look at ASAP.

[okonet]

Looks like it’s related to #295.

[sudo-suhas]

@ltegman Could you please share your package.json? I am guessing you have an prettier entry in your package.json scripts. Also, could you add console.log(res.bin, args) before the following return statement and share the output ?- https://github.com/okonet/lint-staged/blob/1dc3bd60f301f52cb2e96f441338837ae363131e/src/runScript.js#L38-L45

@okonet Maybe we should have an issue template?
Edit: Perhaps we can add logs for all the information we need in verbose mode. Then we can just request people to submit error reports with verbose mode output.

[sudo-suhas]

I was able to reproduce the issue with the following package.json:

{
  "scripts": {
    "prettier": "prettier",
    "lint-staged": "lint-staged"
  },
  // ...
  "lint-staged": {
    "{app,test,config}/**/*.js": [
      "prettier --single-quote --tab-width 4 --print-width 100 --no-bracket-spacing --write",
      "eslint --fix --max-warnings 0",
      "git add"
    ]
  }
}

[sudo-suhas]

@okonet I am not sure about the solution for this. Right now, console.log(res.bin, args) prints this:

npm [ 'run',
  '--silent',
  'prettier',
  '--single-quote',
  '--tab-width',
  '4',
  '--print-width',
  '100',
  '--no-bracket-spacing',
  '--write',
  '--',
  'E:\\Projects\\experiments\\test-lint-staged-script\\test\\index.js',
  'E:\\Projects\\experiments\\test-lint-staged-script\\config\\index.js',
  'E:\\Projects\\experiments\\test-lint-staged-script\\app\\index.js' ]

The issue is fixed for this specific case if -- is added before --single-quote. However this can cause issues in other cases:

λ .\node_modules\.bin\jest --onlyChanged
 PASS  test\findBin.spec.js
 PASS  test\index.spec.js
 PASS  test\runAll.spec.js
 PASS  test\runScript.spec.js
 PASS  test\runScript-mock-pMap.spec.js
 PASS  test\runScript-mock-findBin.spec.js

λ .\node_modules\.bin\jest -- --onlyChanged
No tests found
In E:\Projects\repos\lint-staged
  38 files checked.
  testMatch: **/__tests__/**/*.js?(x),**/?(*.)(spec|test).js?(x) - 11 matches
  testPathIgnorePatterns: \\node_modules\\ - 38 matches
Pattern: --onlyChanged - 0 matches

I think the right solution would be to not have a prettier script.

Edit:

λ npm run jest -- --onlyChanged

> [email protected] jest E:\Projects\repos\lint-staged
> jest "--onlyChanged"

 PASS  test\index.spec.js
 PASS  test\findBin.spec.js
 PASS  test\runAll.spec.js
 PASS  test\runScript.spec.js
 PASS  test\runScript-mock-findBin.spec.js
 PASS  test\runScript-mock-pMap.spec.js

Test Suites: 6 passed, 6 total
Tests:       29 passed, 29 total
Snapshots:   5 passed, 5 total
Time:        1.127s, estimated 3s
Ran all test suites related to changed files.

So adding -- should be safe from what I understand.

[okonet]

Thanks for investigating! Yes, the right solution imo would be just removing the scripts section for the prettier.

I’m wondering if we should just drop support for implicit npm run behavior whatsoever. We looking for the locally installed packages either way. I saw some confusion with this kind of issue before.

[sudo-suhas]

@okonet I think you are right about this being a source of confusion. If I have a bin _and_ a script, the user probably does not expect the npm script to take precedence. Should we just ask the user to write npm run script-name instead(in lint-staged commands)? This would be a breaking change. We should probably do all the breaking changes in 5.0 branch.

[okonet]

Yes I have been think about doing this for some time now and it’s seems to be a better thing to do. I’ll create the next branch next week when I’m back from conference.

[aleburato]

Thanks guys you just saved me a lot of headaches. I had the same issue 👍

[ltegman]

Thanks for looking into this so quickly -- I didn't even realize lint-staged supported referencing npm scripts without using npm run until I was reading through this thread. Renaming our prettier script definitely seems like the easiest way around this for now.

[sudo-suhas]

@aleburato @ltegman, Glad to help 😄. And sorry for the bug 😅

[sudo-suhas]

Hey @aleburato, @ltegman, We just released lint-staged v4.2.3. Do you mind checking if it fixes the issue? Hopefully it did not introduce any new ones.

[cloud-walker]

Hi all, I'm not sure is the same problem, but I've upgraded from 4.1 to 4.2.3 and my configuration now lint ALL the files (staged && not staged).

[sudo-suhas]

@cloud-walker, Could you share your package.json? I suspect that you have a npm script entry to lint all files for the command you try to run with lint-staged.

[cloud-walker]

sure!

package.json

{
  "scripts": {
    "precommit": "lint-staged",
    "test": "jest",
    "test-watch": "jest --watch",
    "coverage": "jest --coverage --no-cache",
    "stylelint": "stylelint \"src/**/*.css\"",
    "eslint": "eslint src/** tools/**",
    "prettier": "prettier --write \"**/*.js\"",
    "lint": "npm run stylelint && npm run eslint",
    "clean": "rimraf build",
    "build-client": "webpack --config tools/webpack/client.config.babel.js",
    "build-server": "webpack --config tools/webpack/server.config.babel.js",
    "build": "npm run build-client && npm run build-server",
    "rebuild": "npm run clean && npm run build",
    "build-langs": "npm run clean && NODE_ENV=production npm run build-client && babel-node --presets env ./tools/translate.js",
    "ensure-env": "node ./tools/ensureEnv.js",
    "dev": "babel-node tools/devServer.js",
    "prestart": "npm run ensure-env && cross-env NODE_ENV=production NEW_RELIC_ENABLED=false npm run build",
    "start": "npm run start-server",
    "start-server": "node -r dotenv/config build/server/main",
    "app": "npm run ensure-env && npm run rebuild && node -r dotenv/config build/server/main",
    "webpack-analyze": "npm run clean && NODE_ENV=production npm run build-client -- --env.analyze",
    "storybook": "start-storybook -p 9000 -c tools/storybook"
  },
  "private": true,
  "engines": {
    "node": ">=6.3.1",
    "npm": ">=3.10"
  },
  "lint-staged": {
    "*.css": [
      "prettier --write",
      "stylelint",
      "git add"
    ],
    "*.js": [
      "prettier --write",
      "eslint",
      "git add"
    ]
  },
  "jest": {
    "modulePaths": [
      "src",
      "build"
    ],
    "snapshotSerializers": [
      "enzyme-to-json/serializer"
    ],
    "setupFiles": [
      "./tools/test/setup.js"
    ],
    "moduleNameMapper": {
      "\\.(ico|jpg|jpeg|png|gif|eot|otf|webp|svg|xml|ttf|woff|woff2|mp4|webm|wav|mp3|m4a|aac|oga)$": "<rootDir>/tools/test/fileMock.js",
      "\\.(css)$": "<rootDir>/tools/test/styleMock.js",
      "assets.json": "<rootDir>/tools/test/assetsMock.json",
      "uuid": "<rootDir>/tools/test/uuidMock.js"
    },
    "collectCoverageFrom": [
      "**/src/shared/**/*.js",
      "!**/src/shared/components/Styleguide/**/*.js",
      "!**/src/shared/mocks/**/*.js"
    ]
  },
  "dependencies": {
    "@storybook/addon-knobs": "^3.2.8",
    "@storybook/addon-notes": "^3.2.7",
    "@storybook/react": "^3.2.8",
    "analytics.js": "segmentio/analytics.js#^2.11.0",
    "app-root-dir": "^1.0.2",
    "assets-webpack-plugin": "^3.5.1",
    "autoprefixer": "^7.0.1",
    "axios": "^0.16.0",
    "babel-cli": "^6.22.2",
    "babel-core": "^6.22.1",
    "babel-eslint": "^7.1.1",
    "babel-loader": "^7.0.0",
    "babel-plugin-dynamic-import-node": "^1.0.2",
    "babel-plugin-react-intl": "^2.3.1",
    "babel-plugin-transform-class-properties": "^6.24.1",
    "babel-plugin-transform-object-rest-spread": "^6.26.0",
    "babel-plugin-transform-react-remove-prop-types": "^0.4.1",
    "babel-preset-env": "^1.2.2",
    "babel-preset-react": "^6.22.0",
    "babel-preset-stage-2": "^6.22.0",
    "classnames": "^2.2.5",
    "common-tags": "^1.4.0",
    "compression": "^1.6.2",
    "cookie-dough": "^0.1.0",
    "cookie-parser": "^1.4.3",
    "cross-env": "^5.0.1",
    "css-loader": "^0.28.0",
    "dotenv": "^4.0.0",
    "enzyme": "^2.7.1",
    "enzyme-to-json": "1.5.1",
    "eslint": "^4.2.0",
    "eslint-plugin-react": "^7.0.0",
    "express": "^4.14.1",
    "express-request-language": "^1.1.9",
    "extend": "^3.0.0",
    "extract-text-webpack-plugin": "^3.0.0",
    "file-loader": "^0.11.1",
    "fontfaceobserver": "^2.0.8",
    "formik": "^0.8.9",
    "fs-extra": "^4.0.0",
    "history": "^4.6.1",
    "husky": "^0.14.3",
    "intl": "^1.2.5",
    "intl-locales-supported": "^1.0.0",
    "jest": "^21.0.2",
    "json-loader": "^0.5.4",
    "lint-staged": "4.1.*",
    "material-design-icons-iconfont": "^3.0.2",
    "moment": "^2.15.2",
    "moment-duration-format": "^1.3.0",
    "morgan": "^1.7.0",
    "new-relic-react": "^1.0.0",
    "newrelic": "^2.0.0",
    "node-notifier": "^5.0.2",
    "nprogress": "^0.2.0",
    "payment-icons": "^0.0.13",
    "postcss-calc": "^6.0.0",
    "postcss-color-function": "^4.0.0",
    "postcss-color-hex-alpha": "^3.0.0",
    "postcss-conditionals": "^2.1.0",
    "postcss-loader": "^2.0.5",
    "postcss-mixins": "^6.0.0",
    "postcss-nesting": "^3.0.0",
    "postcss-simple-vars": "^4.0.0",
    "postcss-smart-import": "^0.7.0",
    "prettier": "^1.6.1",
    "query-string": "^5.0.0",
    "ramda": "^0.24.1",
    "raven-js": "^3.14.1",
    "react": "^15.4.2",
    "react-addons-test-utils": "^15.4.2",
    "react-async-bootstrapper": "^1.1.1",
    "react-async-component": "^1.0.0-beta.3",
    "react-clickoutside-component": "^1.0.7",
    "react-dates": "^12.0.0",
    "react-dom": "^15.4.2",
    "react-helmet": "^5.0.3",
    "react-intl": "^2.1.5",
    "react-intl-translations-manager": "^5.0.0",
    "react-lazyload": "^2.2.4",
    "react-markdown": "^2.5.0",
    "react-redux": "^5.0.3",
    "react-responsive": "^1.3.0",
    "react-router": "^4.1.1",
    "react-router-dom": "^4.1.1",
    "react-router-redux": "next",
    "react-stickynode": "^1.2.1",
    "react-test-renderer": "^15.5.4",
    "react-tooltip": "3.2.9",
    "redux": "^3.6.0",
    "redux-devtools-extension": "^2.13.0",
    "redux-observable": "^0.16.0",
    "redux-segment": "^1.4.2",
    "redux-thunk": "^2.2.0",
    "request": "^2.79.0",
    "request-promise": "^4.1.1",
    "reselect": "^3.0.0",
    "rimraf": "^2.5.4",
    "rxjs": "^5.1.0",
    "sanitize.css": "https://github.com/wanderio/sanitize.css",
    "source-map-support": "^0.4.8",
    "spected": "^0.5.0",
    "style-loader": "^0.18.0",
    "stylelint": "^8.0.0",
    "url-loader": "^0.5.7",
    "url-parse": "^1.1.9",
    "validator": "^8.0.0",
    "webpack": "^3.4.1",
    "webpack-bundle-analyzer": "^2.3.0",
    "webpack-dev-middleware": "^1.10.0",
    "webpack-hot-middleware": "^2.16.1",
    "webpack-node-externals": "^1.5.4"
  }
}

[luftywiranda13]

@cloud-walker IMO it's not a good approach to name your npm scripts like this:

"scripts": {
  "eslint": "eslint ...",
  "stylelint": "stylelint ...",
  "jest": "jest..."
}

could you try to rename those scripts, for example to be:

"scripts": {
  "lint:js": "eslint src/** tools/**",
  "lint:styles": "stylelint \"src/**/*.css\"",
}

after that, run npm run precommit again.
does lint-staged work as expected?

[cloud-walker]

It seems to work with the @luftywiranda13 advice, but to me seems a bit odd this behaviour, is documented somewhere?

[sudo-suhas]

@cloud-walker See https://github.com/okonet/lint-staged#what-commands-are-supported

[sudo-suhas]

However, we are planning to remove this behavior as it seems to cause some confusion.

[cloud-walker]

Thanks!

[luftywiranda13]

@cloud-walker

It seems to work with the @luftywiranda13 advice

glad it helps!

but to me seems a bit odd this behaviour, is documented somewhere?

hmmm maybe. Actually we have to know how lint-staged run tasks in precommit hooks.
And also read this (taken from our readme):

considering you did git add file1.ext file2.ext, lint-staged will run the following command:
npm run my-task -- file1.ext file2.ext

You get why it didn't work like you expected before?

[cloud-walker]

yes its all clear now I've switched to a different script naming, BTW I still think the feature is a bit confusing, maybe 🤔

[aleburato]

Fine for me. Renaming the prettier script is indeed a good practice on my side. I also agree implicit running npm scripts hurts more than it helps.

[ltegman]

I also had included options in the prettier script in my package.json so the fix in 4.2.3 didn't work for me. I think not running npm scripts implicitly (or having them take a backseat to similarly named bins) would be the only thing that would fix this use case for me. Renaming the script is an easy enough workaround though :)

[harrygreen]

@luftywiranda13 Thanks for that tip. Before, we had

"prettier": "prettier --list-different  '{src,__{tests,mocks}__}/**/*.js'",

Changing this to

"lint:prettier": "prettier --list-different  '{src,__{tests,mocks}__}/**/*.js'",

Solved our problem.