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

Unable to process files with a $ in their path #303

Closed
hollandThomas opened this issue Aug 4, 2022 · 9 comments · Fixed by #314
Closed

Unable to process files with a $ in their path #303

hollandThomas opened this issue Aug 4, 2022 · 9 comments · Fixed by #314

Comments

@hollandThomas
Copy link

Description

When there are files with a $ in their path, the dollar sign and anything that follows is interpreted as a variable, replaced with an empty string (given that the variable does not exist) and thus stripped from the path.

Steps to reproduce

Have some file with $ in its name and run lefthook on it.

some/path/to.$file.tsx becomes
some/path/to..tsx

Tools like ESLint then of course error with a message like

ESLint: 8.21.0

No files matching the pattern "some/path/to..tsx" were found.
Please check for typing mistakes in the pattern.

lint-staged has got the same bug
lint-staged/lint-staged#962

Environment

  • @evilmartians/lefthook-installer: 1.0.5
  • OS: Mac OS 12.4
  • Shell: zsh 5.8
@hollandThomas hollandThomas changed the title Unable to precess files with a $ in their path Unable to process files with a $ in their path Aug 4, 2022
@mrexox
Copy link
Member

mrexox commented Aug 7, 2022

Hey @hollandThomas ! Thank you for the feedback! Could you provide your lefthook.yml file? I have tested a few cases, and I see the following:

Using {staged_files}

pre-commit:
  commands:
    echo:
      run: echo {staged_files}
$ lefthook run pre-commit
Lefthook v1.0.5
RUNNING HOOK: pre-commit

  EXECUTE > echo
lefthook.yml some.$file.tsx


SUMMARY: (done in 0.01 seconds)
✔️  echo

Using '{staged_files}'

pre-commit:
  commands:
    echo:
      run: echo '{staged_files}'
$ lefthook run pre-commit
Lefthook v1.0.5
RUNNING HOOK: pre-commit

  EXECUTE > echo
lefthook.yml some..tsx


SUMMARY: (done in 0.01 seconds)
✔️  echo

This can be an issue. But I need to know the content of your config file to make sure I am not missing anything.

@hollandThomas
Copy link
Author

Hi @mrexox

This is my config:

pre-commit:
  commands:
    echo:
      run: echo {staged_files}
    echoQuotes:
      run: echo '{staged_files}'
    eslint:
      glob: '*.{js,ts,jsx,tsx}'
      run: npx eslint {staged_files}

It produces this output (paths abbreviated)

yarn run v1.22.17
$ /…/node_modules/.bin/lefthook run pre-commit
Lefthook v1.0.5
RUNNING HOOK: pre-commit

  EXECUTE > echo
…/service.$serviceId.tsx


  EXECUTE > echoquotes
…/service..tsx


  EXECUTE > eslint

Oops! Something went wrong! :(

ESLint: 8.21.0

No files matching the pattern "…/service..tsx" were found.
Please check for typing mistakes in the pattern.



SUMMARY: (done in 0.74 seconds)
✔️  echo
✔️  echoquotes
🥊  eslint
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

@mrexox
Copy link
Member

mrexox commented Aug 8, 2022

Thank you. Seems like there is a problem with escaping. I'll try to figure it out.

@mrexox
Copy link
Member

mrexox commented Aug 11, 2022

Hey! I will release the v1.1.0 soon, and there you can specify '{staged_files}' (in single quotes), and lefthook will surround each file with single quotes, so dollar sign won't be processed by shell. For more details see #314, and the updated docs

@mrexox mrexox linked a pull request Aug 11, 2022 that will close this issue
@hollandThomas
Copy link
Author

Hey, thanks!

The issue still persists unfortunately

pre-commit:
  parallel: true
  commands:
    eslint:
      glob: '*.{js,ts,jsx,tsx}'
      run: npx eslint '{staged_files}'
yarn run v1.22.17
$ /…/node_modules/.bin/lefthook run pre-commit
Lefthook v1.1.0
RUNNING HOOK: pre-commit

  EXECUTE > eslint

Oops! Something went wrong! :(

ESLint: 8.22.0

No files matching the pattern "…/service..tsx" were found.
Please check for typing mistakes in the pattern.



SUMMARY: (done in 1.28 seconds)
🥊  eslint
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

@mrexox
Copy link
Member

mrexox commented Aug 19, 2022

@hollandThomas , could you share a minimal example? I need the following files to reproduce the issue:

  • package.json
  • lefthook.yml
  • lefthook-local.yml if it exists

Why the file is .../service.$serviceId.tsx? What is the folder structure you have? This would also help, I guess.

@hollandThomas
Copy link
Author

It's a Remix app. Remix has a concept called "file-based routing" where certain filename-patterns have to be followed. E.g.

service.$serviceId.tsx maps to a URL like https://my-website.com/service/f7b060ac-45b2-4efe-98a8-d575bb0cddbb

To reproduce

  1. npx create-remix@latest
  2. Just the basics
  3. Remix App Server
  4. TypeScript
  5. Run npm install? => Y
  6. Create lefthook.yml (see below)
  7. Install lefthook
    npm i -D @evilmartians/lefthook-installer
    npx lefthook install
  8. Create app/routes/service.$serviceId.tsx
  9. Make a commit with that file staged

package.json

{
  "private": true,
  "sideEffects": false,
  "scripts": {
    "prepare": "lefthook install",
    "build": "remix build",
    "dev": "remix dev",
    "start": "remix-serve build"
  },
  "dependencies": {
    "@remix-run/node": "^1.6.8",
    "@remix-run/react": "^1.6.8",
    "@remix-run/serve": "^1.6.8",
    "react": "^18.2.0",
    "react-dom": "^18.2.0"
  },
  "devDependencies": {
    "@evilmartians/lefthook-installer": "^1.1.0",
    "@remix-run/dev": "^1.6.8",
    "@remix-run/eslint-config": "^1.6.8",
    "@types/react": "^18.0.15",
    "@types/react-dom": "^18.0.6",
    "eslint": "^8.22.0",
    "typescript": "^4.7.4"
  },
  "engines": {
    "node": ">=14"
  }
}

lefthook.yml

pre-commit:
  commands:
    eslint:
      glob: '*.{js,ts,jsx,tsx}'
      run: npx eslint {staged_files}

There is no lefthook-local.yml. I just did these exact steps and got the same result

yarn run v1.22.17
$ /Users/tho/Projects/lefthook-reproduction/node_modules/.bin/lefthook run pre-commit
Lefthook v1.1.0
RUNNING HOOK: pre-commit

  EXECUTE > eslint

Oops! Something went wrong! :(

ESLint: 8.22.0

No files matching the pattern "app/routes/service..tsx" were found.
Please check for typing mistakes in the pattern.

@mrexox
Copy link
Member

mrexox commented Aug 19, 2022

Thank you for such a detailed repro steps! 🙏

Just noticed that I can't lint with this command from CLI: npx eslint 'app/routes/service.$serviceId.tsx'.

$ npx eslint app/routes/service.\$serviceId.tsx # the same for 'app/routes/service.$serviceId.tsx'

Oops! Something went wrong! :(

ESLint: 8.22.0

No files matching the pattern "app/routes/service..tsx" were found.
Please check for typing mistakes in the pattern.

By the way, I found an issue in remix repo: remix-run/remix#3339.

What I noticed is that when I execute eslint binary without npx or npm wrapper, it processes well!

$ node_modules/.bin/eslint 'app/routes/service.$serviceId.tsx'
$ echo $?
0

So, my advise would be to use the following setting:

pre-commit:
  commands:
    eslint:
      glob: '*.{js,ts,jsx,tsx}'
      run: node_modules/.bin/eslint {staged_files}

This is not "cool" but this is working. I guess there is an arguments preprocessing by npm. Anyway, I can't find a way lefthook can change this 😕

@hollandThomas
Copy link
Author

Oh wow, it didn't occur to me to try and use the eslint binary directly. That does work, thank you!
It isn't cool but it gets the job done at least

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants