lint-staged doesn't restore unstaged changed when it fails

[egeriis]

Description

I've experienced an issue, that I'm consistently able to reproduce.

If I do a partial staging of changes, where the staged changes contain linting errors, the unstaged changes are not restored after lint-staged runs.

What's even more annoying is that the stashed changes doesn't appear in stash list, meaning the work is lost.

Appear to relate to #62

Steps to reproduce

Not sure how to best produce a small example that reproduces this issue. But these steps triggers the error for me:

1. Make changes to 2 files
   • In file A, make two changes, one of the changes should fail linting
2. Stage the changes—but only stage the change that linting would fail on in file A
3. Run lint-staged

Debug Logs

expand to view

$ lint-staged --debug
  lint-staged:bin Running `lint-staged@8.0.4` +0ms
  lint-staged:find-bin Loaded package.json using `process.cwd()` +0ms
  lint-staged Loading config using `cosmiconfig` +0ms
  lint-staged Successfully loaded config from `/Users/ronni/Development/seatgeek/seatgeek/.lintstagedrc`:
  lint-staged { concurrent: false,
  lint-staged   subTaskConcurrency: 1,
  lint-staged   linters:
  lint-staged    { '*.{css,scss}': [ 'stylelint --fix', 'git add' ],
  lint-staged      '*.js': [ 'eslint --fix', 'git add' ],
  lint-staged      '*.md': [ 'remark . --output --frail', 'git add' ] } } +17ms
  lint-staged:cfg Normalizing config +0ms
  lint-staged:cfg Validating config +1ms
Running lint-staged with the following config:
{
  concurrent: false,
  subTaskConcurrency: 1,
  linters: {
    '*.{css,scss}': [
      'stylelint --fix',
      'git add'
    ],
    '*.js': [
      'eslint --fix',
      'git add'
    ],
    '*.md': [
      'remark . --output --frail',
      'git add'
    ]
  },
  chunkSize: 9007199254740991,
  globOptions: {
    matchBase: true,
    dot: true
  },
  ignore: [],
  renderer: 'verbose'
}
  lint-staged:run Running all linter scripts +0ms
  lint-staged:run Resolved git directory to be `/Users/ronni/Development/seatgeek/seatgeek` +0ms
  lint-staged:run Loaded list of staged files in git:
  lint-staged:run [ 'clientside/js/modals/EmailModal/Success.js' ] +36ms
  lint-staged:gen-tasks Generating linter tasks +0ms
  lint-staged:cfg Normalizing config +39ms
  lint-staged:gen-tasks Generated task:
  lint-staged:gen-tasks { pattern: '*.{css,scss}',
  lint-staged:gen-tasks   commands: [ 'stylelint --fix', 'git add' ],
  lint-staged:gen-tasks   fileList: [] } +15ms
  lint-staged:gen-tasks Generated task:
  lint-staged:gen-tasks { pattern: '*.js',
  lint-staged:gen-tasks   commands: [ 'eslint --fix', 'git add' ],
  lint-staged:gen-tasks   fileList:
  lint-staged:gen-tasks    [ '/Users/ronni/Development/seatgeek/seatgeek/clientside/js/modals/EmailModal/Success.js' ] } +1ms
  lint-staged:gen-tasks Generated task:
  lint-staged:gen-tasks { pattern: '*.md',
  lint-staged:gen-tasks   commands: [ 'remark . --output --frail', 'git add' ],
  lint-staged:gen-tasks   fileList: [] } +1ms
[2018-11-27T13:52:38.563+01:00] Stashing changes... [started]
  lint-staged:git Stashing files... +0ms
  lint-staged:git Running git command [ 'write-tree' ] +1ms
  lint-staged:git Running git command [ 'add', '.' ] +28ms
  lint-staged:git Running git command [ 'write-tree' ] +196ms
  lint-staged:git Running git command [ 'read-tree', 'b8b8946f9aa6dd954161a63f4a2a8f829018af27' ] +29ms
  lint-staged:git Running git command [ 'checkout-index', '-af' ] +123ms
  lint-staged:git Done stashing files! +8s
[2018-11-27T13:52:47.335+01:00] Stashing changes... [completed]
[2018-11-27T13:52:47.336+01:00] Running linters... [started]
[2018-11-27T13:52:47.336+01:00] Running tasks for *.{css,scss} [started]
[2018-11-27T13:52:47.336+01:00] Running tasks for *.{css,scss} [skipped]
[2018-11-27T13:52:47.336+01:00] → No staged files match *.{css,scss}
[2018-11-27T13:52:47.337+01:00] Running tasks for *.js [started]
  lint-staged:make-cmd-tasks Creating listr tasks for commands [ 'eslint --fix', 'git add' ] +0ms
  lint-staged:find-bin Resolving binary for command `eslint --fix` +9s
  lint-staged:find-bin Binary for `eslint --fix` resolved to `/Users/ronni/Development/seatgeek/seatgeek/node_modules/.bin/eslint` +3ms
  lint-staged:task ✔  OS: darwin; File path chunking unnecessary +0ms
  lint-staged:find-bin Resolving binary for command `git add` +0ms
  lint-staged:find-bin Binary for `git add` resolved to `/usr/bin/git` +1ms
  lint-staged:task ✔  OS: darwin; File path chunking unnecessary +1ms
[2018-11-27T13:52:47.342+01:00] eslint --fix [started]
  lint-staged:task bin: /Users/ronni/Development/seatgeek/seatgeek/node_modules/.bin/eslint +0ms
  lint-staged:task args: [ '--fix',
  lint-staged:task   '/Users/ronni/Development/seatgeek/seatgeek/clientside/js/modals/EmailModal/Success.js' ] +0ms
  lint-staged:task opts: { reject: false } +0ms
[2018-11-27T13:52:50.881+01:00] eslint --fix [failed]
[2018-11-27T13:52:50.881+01:00] →
[2018-11-27T13:52:50.881+01:00] Running tasks for *.js [failed]
[2018-11-27T13:52:50.881+01:00] →
[2018-11-27T13:52:50.881+01:00] Running linters... [failed]
[2018-11-27T13:52:50.881+01:00] →



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

/Users/ronni/Development/seatgeek/seatgeek/clientside/js/modals/EmailModal/Success.js
4:10  error  'noop' is defined but never used  no-unused-vars

✖ 1 problem (1 error, 0 warnings)

Environment

• OS: macOS Mojave
• Node.js: v8.11.1
• lint-staged: 8.0.4

[okonet]

The best way would be if you could do a PR with a failing test that does these steps. See the gitStash spec file.

[okonet]

It doesn’t appear in the list of stashes because right now it’s not using git stash for that. It might change in the future, though I wouldn’t rely on it.

[okonet]

Looking at the debug log I see it’s missing the restoration part completely. This isn’t how it is suppose to look like and I’m not sure what’s causing it. Please try using a different node version?

[okonet]

See https://github.com/okonet/lint-staged/blob/master/test/__snapshots__/runAll.spec.js.snap

[egeriis]

@okonet Thank you for your response. I will look into this tomorrow. Could this be related to our specific linting setup? E.g. some unexpected exit codes.

[okonet]

Definitely! What codes are you using?

[egeriis]

That would be the exit code of eslint --fix—since that's what broke the chain in my reproducible example.

$ eslint --fix /Users/ronni/Development/seatgeek/seatgeek/clientside/js/modals/EmailModal.js

/Users/ronni/Development/seatgeek/seatgeek/clientside/js/modals/EmailModal.js
  10:18  error  'css' is defined but never used  no-unused-vars

✖ 1 problem (1 error, 0 warnings)

$ echo $?
1

[okonet]

That shouldn’t be a problem. Have you tried different node versions?

[egeriis]

Same issue with node 10.14.1

[okonet]

That’s very weird. Can you please ditch node_modules and reinstall everything and try out again?

[egeriis]

Ditched node_modules and did a screen recording, because it displays what I think may be the culprit of the issue here:

Updating stash...
Restoring local changes...

This shows up while the linting is running. That seems wrong.

screengrab

---

Detailed description:

• First, notice how file is partially staged
  • What is staged, has a linting error import {get} should be import { get }
• When linting fails, a new diff appears unstaged, which is the lint-fixed code but without rest of the diff

$ cat node_modules/lint-staged/package.json | grep version
  "version": "8.0.4",
$ node -v
v10.14.1

[okonet]

I'm not sure I get what you're saying. From what I see in the screen record it's not clear how it did end up in this state. Updating stash... should only run when there are no linting errors. Restoring happens if there is a stash.

Please create a repo with a reproducible case so I can try myself.

[egeriis]

@okonet I'll see if I am able to produce a repo with this issue, that I can share publicly.

The screengrab I posted appear to be incomplete though. Here's the non-gif: https://www.dropbox.com/s/fmd1zsfnviszsko/lint-staged.mov?dl=0

Could this be related to having multiple linting steps?

[okonet]

Hmm, this is definitively not how it suppose to work... I'm not sure what's causing it yet so please help me to find the root cause. I've written tests for this behavior but something with your setup still skips the restoration step.

[Zdend]

I had this issue as well and after various tests I found the reason!

Whenever my lint-staged config sets concurrent: false unstaged files don't get restored after linting process errors out. Leaving out the concurrent option or setting it to true solves the problem! I didn't check the code of lint-staged but hopefully this should help to solve the underlying cause. Awesome tool btw - @okonet!

Ok

module.exports = {
    concurrent: true,
    linters: {
        'src/*.js': [
            'npx eslint'
        ]
    },
}

Fails

module.exports = {
    concurrent: false,
    linters: {
        'src/*.js': [
            'npx eslint'
        ]
    },
}

It seems like that adding exitOnError: false in listrBaseOptions solves the problem - not sure though whether it will still respect other requirements.

https://github.com/okonet/lint-staged/blob/master/src/runAll.js#L60

It is the same issue as #601

[okonet]

That’s great news! I think we should remove this option since it actually does more harm than good at this point. Thought?

[okonet]

Please can everyone confirm if that’s the case or if the problem still remain?

[egeriis]

Confirmed that @Zdend's workaround works for us as well.

@okonet As one of the authors, do you have any hints to why this happens? I'll be happy to open a PR to fix the issue if you have any pointers. I assume there's a reason this option was introduced at one point, so might be worthwhile looking into keeping it?

Edit: Just looked through our commit history to see if I could figure out why we have concurrent: false, it appears we had git lock issues prior to adding it. Is that a known issue?

[ayan4m1]

@okonet Yes, I had this happen to me in the last 30 minutes with lint-staged 8.1.3 on Windows 10. Lost an hour of work. I will try concurrent: true and update this.

[okonet]

We didn’t release anything related to this issue yet.

[ayan4m1]

I have just experienced this issue with concurrent: true in my .lintstagedrc.

[egeriis]

I have just experienced this issue with concurrent: true in my .lintstagedrc.

Can you reproduce with verbose output and post it here?

We should all be able to agree, that the mechanics here are so dangerous that we should add better fail safes. The git commands could fail for various reasons, unrelated to this tool.

[aaronjensen]

I just lost work as well, I'm not setting concurrent at all currently. In my case, my index.lock was in use for some reason:

  ❯ Running linters...
    ↓ Running tasks for *.{md,html,rb} [skipped]
   
    ❯ Running tasks for *.{js,ts,tsx}
      ✔ eslint_d --fix
      ✖ git add
  ↓ Updating stash... [skipped]
   
  ✖ Restoring local changes...
   
�[?25hError: Command failed: git read-tree 0cc7fd5559794ac448c1b90e0b97eda7b272ca88
fatal: Unable to create '/Users/aaronjensen/Source/project/.git/index.lock': File exists.

Another git process seems to be running in this repository, e.g.
an editor opened by 'git commit'. Please make sure all processes
are terminated then try again. If it still fails, a git process
may have crashed in this repository earlier:
remove the file manually to continue.

One question is, is the work really gone? Does it store it in a tmp directory somewhere? Would it be possible to do that and print the directory until this issue is completely fixed? My work wasn't a lot, but it's scary.

[okonet]

No, the work isn’t gone and there is a way to print a command to restore it but since I still don’t know how to reproduce the issue I can’t really approach it. Also, since I never experienced the issue myself (and I’m running 🚫💩 lint-staged on all my repos) I guess there is something specific what combines all those cases. But the information in this issue isn’t enough to see the pattern. That’s why I need everyone’s help.

[okonet]

One input I’ve got is related to lock files and it seems to be the case but I need to understand better what’s going on

[aaronjensen]

Here's a way to reproduce it:

module.exports = {
  concurrent: true,
  linters: {
    '*.{md,html,rb}': ['touch .git/index.lock', 'git add'],
  },
}

Make 2 changes in a .md file, stage one hunk and commit. You'll lose the other.

Note that you'll have to manually delete .git/index.lock afterwards.

This repro is a hack to simulate what I believe is happening here. Something (maybe magit for me, or my prompt) is locking git in the midst of lint-staged doing its thing. It's a race condition and only happens sometimes. If it happens though, I get the above error and my work is "gone". (luckily it's usually in my undo buffer in emacs, but I have to remember which files were changed).

Hope that helps!

[ayan4m1]

Sorry that I don't have verbose repro info, I am away from my work computer and can't set up a repro easily here. I will say that to @aaronjensen 's point it only seems to occur for me if webpack-dev-server is running in the background.

[Ghost2016]

I just lost work as well, but after reading the main source code of the project, I found that excute git checkout-index -af would get all work back, although it could not print the error msg!

[Ghost2016]

It is weird that I did excute git checkout-index -af for times in lint-staged but can't work. Out of it, excuting the cmd, it worked, and got all the lost work.

[Ghost2016]

And sometimes, you may do it for times to got all the lost work like this(due to permission denied):

And I hope this could help solving...

[okonet]

@iiroj do you think we could try repro this using your new test suit?

[iiroj]

@okonet sure, I can try.

Still, creating a stash before running , and restoring it on success should be pretty straightforward.

Let me try and reproduce this over the weekend.

[okonet]

My hope is that if you’d “lock” some files for writing, you’d be able to repro it in the test. After that we could figure out a strategy for the fix.

[iiroj]

I re-created the test case in PR #664, and it does cause loss of modifications. For what it's worth, I've opened another PR #663 that creates a git stash first, so the original state can be restored.

[okonet]

🎉 This issue has been resolved in version 9.5.0-beta.1 🎉

The release is available on:

• npm package (@beta dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[okonet]

🎉 This issue has been resolved in version 10.0.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[WagnerMoreira]

this is still happening on lint-staged@12.3.7 with node@v16.13.0, just happened to me on OSX and I lost a lot of code changes on several files.

my precommit hook

#!/bin/sh
. "$(dirname "$0")/_/husky.sh"

npm run lint:staged

my .lintstagedrc

{
  "*.rb": [
    "bundle exec rubocop -c .rubocop.yml --debug --display-cop-names"
  ],
  "*.{js,jsx}": [
    "npx eslint --config client/.eslintrc.js",
    "npx prettier . --write"
  ],
  "*.scss": [
    "npx stylelint --config .stylelintrc --fix  --custom-syntax postcss-scss"
  ]
}

any ideas?

[braidn]

@WagnerMoreira also struggling with code being staged and not popped properly when rubocop fails. Are you getting a PID error as well?

[y3fers0n]

Happened to me today, all changes lost after lint-staged failed

[iiroj]

@y3fers0n this is a pretty old issue... can you open a new one with debug logs? Did you find your changes from the git stash list?

[y3fers0n]

@y3fers0n this is a pretty old issue... can you open a new one with debug logs? Did you find your changes from the git stash list?

Yes, I did found the changes there, thank you