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

[BUG] No matching files on filenames with brackets #676

Closed
zomars opened this issue Jul 30, 2019 · 27 comments
Closed

[BUG] No matching files on filenames with brackets #676

zomars opened this issue Jul 30, 2019 · 27 comments

Comments

@zomars
Copy link

zomars commented Jul 30, 2019

Description

This is related to Next.js new dynamic routes feature.

Steps to reproduce

  1. Use this configuration in package.json:
  
  "husky": {
    "hooks": {
      "pre-commit": "lint-staged --debug"
    }
  },
  "lint-staged": {
    "*.{js,css,json,md}": [
      "prettier --write",
      "git add"
    ],
    "*.js": [
      "npm run lint:eslint",
      "git add"
    ]
  
  1. Try to commit a filename with brackets [].

Debug Logs

expand to view

husky > pre-commit (node v12.6.0)
2019-07-30T23:23:07.320Z lint-staged:bin Running `lint-staged@8.1.7`
2019-07-30T23:23:07.697Z lint-staged:find-bin Loaded package.json using `process.cwd()`
2019-07-30T23:23:07.847Z lint-staged Loading config using `cosmiconfig`
2019-07-30T23:23:07.852Z lint-staged Successfully loaded config from `OMMITED/package.json`:
{
  '*.{js,css,json,md}': [ 'prettier --write', 'git add' ],
  '*.js': [ 'npm run lint:eslint', 'git add' ]
}
2019-07-30T23:23:07.853Z lint-staged:cfg Normalizing config
2019-07-30T23:23:07.855Z lint-staged:cfg Validating config
Running lint-staged with the following config:
{
  linters: {
    '*.{js,css,json,md}': [
      'prettier --write',
      'git add'
    ],
    '*.js': [
      'npm run lint:eslint',
      'git add'
    ]
  },
  concurrent: true,
  chunkSize: 9007199254740991,
  globOptions: {
    matchBase: true,
    dot: true
  },
  ignore: [],
  subTaskConcurrency: 1,
  renderer: 'verbose',
  relative: false
}
2019-07-30T23:23:07.863Z lint-staged:run Running all linter scripts
2019-07-30T23:23:07.863Z lint-staged:run Resolved git directory to be `OMMITED`
2019-07-30T23:23:07.884Z lint-staged:run Loaded list of staged files in git:
[ 'services/enrola-webapp/pages/CRM/sales/[saleId]/index.js' ]
2019-07-30T23:23:07.884Z lint-staged:gen-tasks Generating linter tasks
2019-07-30T23:23:07.884Z lint-staged:cfg Normalizing config
2019-07-30T23:23:07.899Z lint-staged:gen-tasks Generated task: 
{
  pattern: '*.{js,css,json,md}',
  commands: [ 'prettier --write', 'git add' ],
  fileList: [
    'OMMITED/webapp/pages/CRM/sales/[saleId]/index.js'
  ]
}
2019-07-30T23:23:07.900Z lint-staged:gen-tasks Generated task: 
{
  pattern: '*.js',
  commands: [ 'npm run lint:eslint', 'git add' ],
  fileList: [
    'OMMITED/webapp/pages/CRM/sales/[saleId]/index.js'
  ]
}
Stashing changes... [started]
2019-07-30T23:23:07.935Z lint-staged:git Stashing files...
2019-07-30T23:23:07.935Z lint-staged:git Running git command [ 'write-tree' ]
2019-07-30T23:23:07.961Z lint-staged:git Running git command [ 'add', '.' ]
2019-07-30T23:23:07.984Z lint-staged:git Running git command [ 'write-tree' ]
2019-07-30T23:23:08.002Z lint-staged:git Running git command [ 'read-tree', '8abe2bba7ffe8916312b195969cc19039ca8658e' ]
2019-07-30T23:23:08.086Z lint-staged:git Running git command [ 'checkout-index', '-af' ]
2019-07-30T23:23:08.694Z lint-staged:git Done stashing files!
Stashing changes... [completed]
Running linters... [started]
Running tasks for *.{js,css,json,md} [started]
Running tasks for *.js [started]
2019-07-30T23:23:08.696Z lint-staged:make-cmd-tasks Creating listr tasks for commands [ 'prettier --write', 'git add' ]
2019-07-30T23:23:08.697Z lint-staged:find-bin Resolving binary for command `prettier --write`
2019-07-30T23:23:08.702Z lint-staged:find-bin Binary for `prettier --write` resolved to `OMMITED/node_modules/.bin/prettier`
2019-07-30T23:23:08.703Z lint-staged:task ✔  OS: darwin; File path chunking unnecessary
2019-07-30T23:23:08.703Z lint-staged:find-bin Resolving binary for command `git add`
2019-07-30T23:23:08.704Z lint-staged:find-bin Binary for `git add` resolved to `/usr/local/Cellar/git/2.22.0_1/libexec/git-core/git`
2019-07-30T23:23:08.704Z lint-staged:task ✔  OS: darwin; File path chunking unnecessary
2019-07-30T23:23:08.704Z lint-staged:make-cmd-tasks Creating listr tasks for commands [ 'npm run lint:eslint', 'git add' ]
2019-07-30T23:23:08.704Z lint-staged:find-bin Resolving binary for command `npm run lint:eslint`
2019-07-30T23:23:08.705Z lint-staged:find-bin Binary for `npm run lint:eslint` resolved to `/usr/local/bin/npm`
2019-07-30T23:23:08.705Z lint-staged:task ✔  OS: darwin; File path chunking unnecessary
2019-07-30T23:23:08.705Z lint-staged:find-bin Resolving binary for command `git add`
2019-07-30T23:23:08.705Z lint-staged:find-bin Resolving binary for `git` from cache
2019-07-30T23:23:08.705Z lint-staged:task ✔  OS: darwin; File path chunking unnecessary
prettier --write [started]
npm run lint:eslint [started]
2019-07-30T23:23:08.705Z lint-staged:task bin: OMMITED/node_modules/.bin/prettier
2019-07-30T23:23:08.706Z lint-staged:task args: [
  '--write',
  'OMMITED/webapp/pages/CRM/sales/[saleId]/index.js'
]
2019-07-30T23:23:08.706Z lint-staged:task opts: { reject: false }
2019-07-30T23:23:08.710Z lint-staged:task bin: /usr/local/bin/npm
2019-07-30T23:23:08.710Z lint-staged:task args: [
  'run',
  'lint:eslint',
  'OMMITED/webapp/pages/CRM/sales/[saleId]/index.js'
]
2019-07-30T23:23:08.710Z lint-staged:task opts: { reject: false }
prettier --write [failed]
→ 
Running tasks for *.{js,css,json,md} [failed]
→ 
npm run lint:eslint [completed]
git add [started]
2019-07-30T23:23:11.725Z lint-staged:task bin: /usr/local/Cellar/git/2.22.0_1/libexec/git-core/git
2019-07-30T23:23:11.725Z lint-staged:task args: [
  'add',
  'OMMITED/webapp/pages/CRM/sales/[saleId]/index.js'
]
2019-07-30T23:23:11.725Z lint-staged:task opts: { reject: false }
git add [completed]
Running tasks for *.js [completed]
Running linters... [failed]
Updating stash... [started]
Updating stash... [skipped]
→ Skipping stash update since some tasks exited with errors
Restoring local changes... [started]
2019-07-30T23:23:11.739Z lint-staged:git Restoring working copy
2019-07-30T23:23:11.739Z lint-staged:git Running git command [ 'read-tree', 'e0a3ad899b52438b4a0f11eadce45c90d21c1936' ]
2019-07-30T23:23:11.769Z lint-staged:git Running git command [ 'checkout-index', '-af' ]
2019-07-30T23:23:12.375Z lint-staged:git Restoring index
2019-07-30T23:23:12.376Z lint-staged:git Running git command [ 'read-tree', '8abe2bba7ffe8916312b195969cc19039ca8658e' ]
Restoring local changes... [completed]



✖ prettier --write found some errors. Please fix them and try committing again.

[error] No matching files. Patterns tried: OMMITED/webapp/pages/CRM/sales/[saleId]/index.js !**/node_modules/** !./node_modules/** !**/.{git,svn,hg}/** !./.{git,svn,hg}/**
husky > pre-commit hook failed (add --no-verify to bypass)


Environment

  • OS: macOS Mojave 10.14.5 (18F132)
  • Node.js: v8.15.0
  • lint-staged: 8.1.7
@iiroj
Copy link
Member

iiroj commented Jul 31, 2019

To me it looks like the error happens during the linter task, since the path seems to be working fine:

2019-07-30T23:23:08.705Z lint-staged:task bin: OMMITED/node_modules/.bin/prettier
2019-07-30T23:23:08.706Z lint-staged:task args: [
  '--write',
  'OMMITED/webapp/pages/CRM/sales/[saleId]/index.js'
]
prettier --write [failed]

If you run the command directly, does it work? prettier --write OMMITED/webapp/pages/CRM/sales/[saleId]/index.js?

It seems like prettier is treating the input filepath as a glob instead of a direct string, and is thus failing to match anything.

@ahmedelgabri
Copy link

Doesn't work

prettier --write ./path/to/[file].js
prettier --write ./path/to/\[file\].js

Work

prettier --write "./path/to/\[file\].js"

It seems like prettier is treating the input filepath as a glob instead of a direct string, and is thus failing to match anything.

Yeah that's what I think too & maybe the issue should be in prettier instead.

@zomars
Copy link
Author

zomars commented Jul 31, 2019

Thanks for the input. I'll continue the discussion on the prettier repo.

@zomars zomars closed this as completed Jul 31, 2019
@zomars
Copy link
Author

zomars commented Aug 1, 2019

From the prettier issue:

How could I fit escaped brackets on this glob? *.{js,css,json,md}

As @ahmedelgabri replied:

@zomars you can't. I know that in the other thread I said, it might be a prettier issue but now I think this might actually be handled on lint-staged instead. Especially after reading the prettier docs
Don't forget the quotes around the globs! The quotes make sure that Prettier expands the globs rather than your shell, for cross-platform usage. The glob syntax from the fast-glob module is used.
https://github.com/prettier/prettier/blob/master/docs/cli.md

@zomars zomars reopened this Aug 1, 2019
@iiroj
Copy link
Member

iiroj commented Aug 1, 2019

Regarding quotes, this issue is relevant: #635. Maybe it should get another look.

@langovoi
Copy link

langovoi commented Aug 6, 2019

I found temp workaround:
.lintstagedrc.js

const quote = require('shell-quote').quote;

module.exports = {
    '*.{js,jsx,ts,tsx}': filenames =>
        filenames.reduce((commands, filename) => {
            commands.push(quote(['prettier', '--write', filename]), quote(['git', 'add', filename]));

            return commands;
        }, [])
};

@ahmedelgabri
Copy link

@langovoi maybe you can try to do this here quoting the filelist https://github.com/okonet/lint-staged/blob/710be313809901e6fe3275c476b6f82241093989/src/generateTasks.js#L67 & see if it works, if it does maybe you can open a PR?

@owalerys
Copy link

@langovoi quoting seems to work for prettier but break stylelint

@borekb
Copy link

borekb commented Aug 29, 2019

This is how I currently deal with the issue: #635 (comment).

Not sure how to best solve it on the lint-staged side generically. There would probably need to be some detection that the command being executed is prettier and do some custom stuff for it.

@uriel-sf
Copy link

uriel-sf commented Sep 5, 2019

What's the status here? Running lint-staged@8.0.5 w/ node 10.15.3, but when i try the workaround #676 (comment), I get:

image

@iiroj
Copy link
Member

iiroj commented Sep 6, 2019

I just created PR #698 for escaping certain characters from the file paths, and it works for prettier+brackets at least (in the integration test).

@millette
Copy link

millette commented Sep 26, 2019

I was using something similar to #676 (comment) but I feel this was broken by #706 (lint-staged 9.4.0).

@iiroj
Copy link
Member

iiroj commented Sep 27, 2019

@millette that PR shouldn’t have affected task behaviour, only the generated title. If you can reproduce the regression, I can try to fix it.

@millette
Copy link

millette commented Sep 27, 2019

@iiroj Thanks,but I only know enough about lint-staged to be dangerous. I probably wrote my workaround wrong but too busy with other stuff to dig deeper right now and #698 might render that moot anyway :-)

@boxersb
Copy link

boxersb commented Oct 2, 2019

Hello there!
I'm currently using workaround like this.

.lintstagedrc.js

const escape = require('shell-quote').quote

function listQuote(filenames) {
  return filenames.map((filename) => `'${filename}'`)
}

function listEscape(filenames) {
  return filenames.map((filename) => escape([filename]))
}

module.exports = {
  '**/*.{js,ts,tsx}': (filenames) => [
    `eslint --fix ${listQuote(filenames).join(' ')}`,
    `prettier --write ${listQuote(listEscape(filenames)).join(' ')}`,
    `git add ${listQuote(filenames).join(' ')}`,
  ],
}

Main problem is..
Prettier need escapes and quotes filenames, but eslint and git add are not.

And, using these commands for all staged files are cause massive cli logs.
I just want to use like 'lint-stagedfield onpackage.json`, so I wrote workaround above.

@tphan18
Copy link

tphan18 commented Nov 13, 2019

Or how the Next.js team handles this issue https://github.com/zeit/next.js/blob/canary/lint-staged.config.js

@okonet
Copy link
Collaborator

okonet commented Nov 13, 2019

I'd love us to hide this complexity but we'd need a really comprehensive test suite for that. Ans also since it's tool related, maybe we could provide the function with escaped file names arguments.

@iiroj thoughts?

@Murillo2380
Copy link

Murillo2380 commented Dec 23, 2019

@okonet maybe a special command to scape characters? Imagine something like this on package-json

  "lint-staged": {
    "--escape *.{js,jsx}": [
      "prettier --write",
      "git add"
    ],
    "*.{js,jsx}": [
      "npm run lint:eslint",
      "git add"
    ]

the --escape is basically saying to escape any special characters before giving it to prettier, but I'm afraid this would work for prettier --write but will fail for git add.

Or maybe:

  "lint-staged": {
    "*.{js,jsx}": [
      "--escape",
      "prettier --write",
      "npm run lint:eslint"
      "git add"
    ]

In this case, the --escape on the command chain would just be saying to escape the input before the next command.

Just brainstorming. The former could open doors for some other commands. I fixed my problem by following the suggestion in #676 (comment)

@mbrowne
Copy link

mbrowne commented Jan 15, 2020

Thanks for the workaround for prettier. Between that and upgrading to the latest version of stylelint, I was able to create a config that works for prettier, eslint, and stylelint. Here's my .lintstagedrc.js in case it helps someone:

const escape = require('shell-quote').quote

module.exports = {
    '*.{ts,tsx,js,json,css}': filenames => [
        ...filenames.map(
            filename => `prettier --check "${escape([filename])}"`
        ),
        ...filenames.map(filename => `git add "${filename}"`),
    ],
    '*.{ts,tsx,js}': ['eslint'],
    '*.{ts,tsx,css}': ['stylelint'],
}

@rahul3103
Copy link

When its going to be fixed any idea

@iiroj
Copy link
Member

iiroj commented Jan 24, 2020

This is not a simple issue to fix because of cross-platform support requirements. Ideally I would like to solve this by wrapping filepath arguments inside quotes. Unfortunately I can't estimate a timeline, because I'm not sure how to fix it yet.

@iiroj
Copy link
Member

iiroj commented Jan 25, 2020

I cannot find a way to simply pass quoted strings to the spawned tasks. Execa seems to escape as \” which doesn’t work with at least prettier.

I even re-implemented what execa does using native child_process but it has the same behaviour. This can be circumvented using the shell option, but we don’t want that because it’s slower and insecure by design.

I think the only way to get this working is by escaping individual characters like brackets, so #698 should be looked into again.

@MatejBransky
Copy link

MatejBransky commented Mar 2, 2020

Neither of above worked for us. We've found out that it's possible to bypass this problem by wraping square brackets with square brackets (matcher of any char in seq).

lint-staged.config.js:

const escape = require('shell-quote').quote;
const isWin = process.platform === 'win32';

module.exports = {
	'**/*.{js,jsx,ts,tsx,json}': filenames => {
		const escapedFileNames = filenames
			// this will wrap all "[" "]" square brackets with another square brackets ([ => [[]) so [...customer].tsx will be processed to [[]...customer[]].tsx
			.map(filename => `"${isWin ? filename.replace(/\[|\]/g, '[$&]') : escape([filename])}"`)
			.join(' ');
		return [
			`prettier --with-node-modules --ignore-path='./.gitignore' --write ${escapedFileNames}`,
			`eslint --no-ignore --max-warnings=0 --fix ${filenames.map(f => `"${f}"`).join(' ')}`,
			`git add ${escapedFileNames}`
		];
	},
	'**/*.{json,md,mdx,css,html,yml,yaml,scss}': filenames => {
		const escapedFileNames = filenames
			.map(filename => `"${isWin ? filename.replace(/\[|\]/g, '[$&]') : escape([filename])}"`)
			.join(' ');
		return [
			`prettier --with-node-modules --ignore-path='./.gitignore' --write ${escapedFileNames}`,
			`git add ${escapedFileNames}`
		];
	}
};

@okonet
Copy link
Collaborator

okonet commented Nov 6, 2020

This has been fixed in v2 of Prettier. Closing.

@okonet okonet closed this as completed Nov 6, 2020
@thomaszdxsn
Copy link

it's not only prettier issue, it should escape correctly

@iiroj
Copy link
Member

iiroj commented Nov 20, 2020

@thomaszdxsn Feel free to open a PR, any help is appreciated! So far we haven't been able to find a consistent cross-platform method for escaping paths that works with execa, with or without using shell.

@belgattitude
Copy link

belgattitude commented Jul 11, 2021

If someone still impacted by this especially on Windows and/or monorepos, take a look at this example:

https://github.com/belgattitude/nextjs-monorepo-example/blob/951c75f3c5ef0ccca511dd0519984754053857cd/lint-staged.config.base.js

It's basically escaping the filenames using shell-quote like previously commented. Note that in this scenario ts, tsx, js, jsx are prettyfied through eslint-prettier-config (up to you to change).

const escape = require('shell-quote').quote;
const { CLIEngine } = require('eslint');

const cli = new CLIEngine({});
const isWin = process.platform === 'win32';

const escapeFileNamesForPrettier = (filenames) =>
  filenames
    .map((filename) => `"${isWin ? filename : escape([filename])}"`)
    .join(' ');

module.exports = {
  '**/*.{js,jsx,ts,tsx}': (filenames) => {
    return [
      `eslint --rule 'react-hooks/exhaustive-deps: off' --max-warnings=25 --fix ${filenames
        .filter((file) => !cli.isPathIgnored(file))
        .map((f) => `"${f}"`)
        .join(' ')}`,
    ];
  },
  '**/*.{json,md,mdx,css,html,yml,yaml,scss}': (filenames) => {
    return [`prettier --write ${escapeFileNamesForPrettier(filenames)}`];
  },
};

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment