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

IntelliJ and auto-fix code leaving staged and not staged files #151

Closed
samit4me opened this Issue Apr 27, 2017 · 38 comments

Comments

Projects
None yet
@samit4me

samit4me commented Apr 27, 2017

Added lint-staged to a repo at work today using the following config:

"lint-staged": {
  "*.js": ["eslint --fix", "git add"]
}

Everyone was testing it out by intentionally introducing lint errors (e.g removing a semicolon) and then commit the change just to see how awesome the auto-fix feature is.

This was going great until IntelliJ users (both Windows and Linux) started reporting issues, whereby staged and not staged files were intermittently getting left behind even though the corrected files were actually committed successfully.

After digging into the issue as a non-IntelliJ user I discovered that IntelliJ doesn't git add or git commit -a but it commit's files directly, something like this:

git -c core.quotepath=false commit --only -F /tmp/git-commit-msg-.txt -- path/to/file.js

So with this knowledge, I managed to reproduce the issue reliably with git via the command line using these steps:

  1. Remove a semicolon from a JS file that gets linted and save.
  2. Run git commit -m "Test" path/to/file.js
  3. The file is fixed (semicolon added) and committed.
  4. If I run git status the same file is both staged and not staged.
  5. If I run git diff the staged file is GOOD (has semicolon) and the not staged file is BAD (NO semicolon).

These staged and not staged files can be cleaned up via git reset and maybe that could be added as a "postcommit" using husky but I'm not sure if this is a good idea and I'm definitely NOT a Git Guru, so if anyone has experience with this or can help in any way it will be greatly appreciated.

Demo Repo:
I created a small repo to help with further testing https://github.com/samit4me/lint-staged-demo.

We are still using lint-staged as it's tremendously helpful even without the auto-fix / auto-prettier functionality. Thank you for the library it is awesome 👏 👏 👏

@tmos

This comment has been minimized.

tmos commented Apr 27, 2017

Just ran into the same issue, 2h after you ! Some help would be really appreciated 👍
Thanks again for the lib !

@okonet

This comment has been minimized.

Owner

okonet commented Apr 28, 2017

Hey @samit4me! That's a dream report! Thanks for taking time and investigating it.

Reading git documentation it seems clear now why it acts like this:

When files are given on the command line, the command commits the contents of the named files, without recording the changes already staged. The contents of these files are also staged for the next commit on top of what have been staged before.

See https://git-scm.com/docs/git-commit#git-commit---only

So, basically, WebStorm doen's use staged commits (or index). This is causing the this staging behavior after the commit.

I'm not sure how to resolve this in the library but it's an interesting case either way. I'm using IntelliJ IDEA myself but I use CLI to with with Git.

So you might want to explore the postcommit hook with husky that resets staged files. I'll give this a thought myself but not sure if I can come up with any solutions soon.

@samit4me

This comment has been minimized.

samit4me commented Apr 29, 2017

Thanks @okonet! The problem seems to occur with or without the --only flag and appears to be related to using git add inside of a pre-commit hook. I asked the question on stackoverflow to see if any Git Masters can help, see Using git add inside a pre-commit hook.

As for the postcommit solution, it certainly works, I quickly threw together a script (see below) which I shamelessly copied/pasted/modified from your library and hooked it up with the husky postcommit script.

'use strict';

const path = require('path');
const sgf = require('staged-git-files');
const appRoot = require('app-root-path');
const execa = require('execa');
const findBin = require('lint-staged/src/findBin');

const packageJson = require(appRoot.resolve('package.json'));
const cwd = process.cwd();

sgf('ACM', (err, files) => {
  if (err) {
    console.error(err);
  }

  const resolvedFiles = files.map((file) => {
    const absolute = path.resolve(cwd, file.filename);
    const relative = path.relative(cwd, absolute);
    return relative;
  });
  console.log('>>>>>>', resolvedFiles);

  const res = findBin('git reset', resolvedFiles, {});
  execa(res.bin, res.args, {})
    .then(() => { console.log('postcommit complete'); })
    .catch((err) => { console.error('Something went wrong in postcommit'); });
});

It does have issues though. If you have other staged files (e.g. somefile.js) and you commit a single file directly (e.g. git commit -m "Test" index.js) index.js is committed and reset (which is good), but the other staged file somefile.js is also reset which isn't what I wanted. This seems to be related to the library staged-git-files. Strangely that library only reports index.js on precommit and both files on postcommit.

Did you want any doc's added so other IntelliJ users don't have this problem?

@okonet

This comment has been minimized.

Owner

okonet commented Apr 29, 2017

I have an ongoing work of abstracting the git workflow to even remove the need of git add in the config. This is not an easy problem as it turns out because I want to support partially staged files. When it's done it might also solve this issue.

@samit4me

This comment has been minimized.

samit4me commented Apr 29, 2017

That sounds great, removing the git commands from the lint-staged config would be fantastic.

@jharris4

This comment has been minimized.

jharris4 commented Apr 30, 2017

I just ran into this exact issue after trying to integrate prettier via a pre-commit hook in a project with WebStorm.

After committing a change to a file, I get both a staged and unstaged copy of the file.

Interestingly though, the unstaged file appears to be identical to the staged file (at least from the WebStorm UI). If I run git add again, the unstaged file magically disappears.

@okonet

This comment has been minimized.

Owner

okonet commented Apr 30, 2017

For now I can only suggest to use CLI to commit files when using pre-commit hook. But I'd be like to resolve this issue somehow. If someone has an idea, please create a PR.

@jharris4

This comment has been minimized.

jharris4 commented Apr 30, 2017

I'm trying to get some traction on getting some kind of 'stage prior to commit` option added to IntelliJ/JetBrains: https://youtrack.jetbrains.com/issue/IDEA-135454

@shai32

This comment has been minimized.

shai32 commented May 22, 2017

same issue

@roytz

This comment has been minimized.

roytz commented May 22, 2017

Same here.. Did a workaround using git add on the postcommit..

@GeeWee

This comment has been minimized.

GeeWee commented May 29, 2017

Same. Used git reset as a postcommit hook. Also webstorm integration with Webstorm.

@shai32

This comment has been minimized.

shai32 commented Jun 16, 2017

will this be fixed?

@okonet

This comment has been minimized.

Owner

okonet commented Jun 16, 2017

Don't think so. Try postcommit hook workaround.

azu added a commit to azu/faao that referenced this issue Jun 17, 2017

@shai32

This comment has been minimized.

shai32 commented Jun 19, 2017

sound like something that need a fix.

@samit4me

This comment has been minimized.

samit4me commented Jun 19, 2017

After investigating the issue and looking at the name of this project "lint-staged" I don't believe it should be resolved by this library. As the name implies this library lint's staged files but the problem is that IntelliJ (webstorm etc.) does not stage files before committing them, it commits them directly.

IntelliJ is paid/supported software and this awesome library is free and available for everyone to both use and edit by the awesome @okonet.

For the above reasons, I believe this problem should be resolved by IntelliJ, but judging by the ticket @jharris4 posted it doesn't look to be happening.If you agree, please go and show your support on the ticket https://youtrack.jetbrains.com/issue/IDEA-135454.

Should we document this in the README as a known issue, with the recommendation of the postcommit hook or not using IntelliJ git?

@quanru

This comment has been minimized.

quanru commented Jun 22, 2017

Mac has the same problem with git version 2.11.0 (Apple Git-81) when i use package.json config, but it is done well when i use .lintstagedrc.

@okonet

This comment has been minimized.

Owner

okonet commented Jun 23, 2017

@quanru not sure how is this related to IntelliJ IDEA issue. Can you elaborate, please?

@quanru

This comment has been minimized.

quanru commented Jun 23, 2017

@okonet it is recoverd after restrat, sorry.

@slorber

This comment has been minimized.

slorber commented Jul 18, 2017

Hey all.

I've been having this issue for some weeks now and used the manual add / revert trick to solve the problem. It's annoying but it works.

Can someone explain to me how to automate this workaroud?

Tried that but it does not work:

    "precommit": "lint-staged",
    "postcommit": "git add"

Also I'd like to mention that for me there's another strange behavior when doing a commit/push with Intellij and there's a rebase/merge needed. I'm not totally sure what happens there but I end up with files not formatted properly after the first commit and need to commit them another time.

@martintyler

This comment has been minimized.

martintyler commented Jul 18, 2017

This is also affecting my team - i'm using a quick script as a postcommit, to avoid affecting files you are not committing with a full git reset.

It does: git show --pretty="" --name-only
then just git resets those files

bapjiws pushed a commit to bapjiws/react-ts-boilerplate that referenced this issue Aug 4, 2017

@ron23

This comment has been minimized.

ron23 commented Aug 18, 2017

Same boat here. The git reset does work, but like mentioned, if you have a staging file (x.js), and you commit another file (y.js), the reset will unstage x.js which isn't what we want. I don't want to create unexpected changes in how git works just because of Jetbrain. Devs that doesn't even use Jetbrain shouldn't be seeing unexpected behavior because of them.

Is there any other solution you can point me to that can run lint/prettier before the code hits PR?

@Arhane

This comment has been minimized.

Contributor

Arhane commented Aug 28, 2017

@slorber
If you make this
"postcommit": "git add ."
Your solution will work.

@slorber

This comment has been minimized.

slorber commented Aug 28, 2017

thanks @andrew1024 will try that. Isn't there a risk of adding unstaged files too?

@Arhane

This comment has been minimized.

Contributor

Arhane commented Aug 28, 2017

Yeah, there is. But between reseting files and adding all files, adding seems more harmless.
Besides, usually I add all files and it does not matter much to me.
Maybe your workflow differs much from mine and in that case my solution is not solition at all.

@tobilen tobilen referenced this issue Sep 2, 2017

Merged

Add prettier #182

7 of 7 tasks complete
@filipsuk

This comment has been minimized.

filipsuk commented Sep 19, 2017

I ended up using --list-different prettier parameter in my precommit hook and removed --write parameter. So if there is some formatting to do, the commit fails and you have to run prettier yourself (e.g. via 'external tool'). Webstorm issues were driving me mad.

@slorber

This comment has been minimized.

slorber commented Sep 29, 2017

Thanks @filipsuk I'll try your solution that seems nice. However I prefer using file watchers instead of external tools explicit call

@slorber

This comment has been minimized.

slorber commented Oct 24, 2017

@Limess can you give us some feedback on your workaround here please?

Financial-Times/cmdb.js@a338ab3

@igoradamenko

This comment has been minimized.

igoradamenko commented Feb 16, 2018

@taylorham

This comment has been minimized.

Contributor

taylorham commented Feb 26, 2018

This fix is working for me until JetBrains figures it out:

package.json

{
  "scripts": {
    "precommit": "lint-staged",
    "postcommit": "git update-index -g"
  }
}

Suggested in this YouTrack issue.

@okonet

This comment has been minimized.

Owner

okonet commented Feb 27, 2018

@taylorham mind creating a PR to the docs for all new users who might be looking for this?

@stsiarzhanau

This comment has been minimized.

stsiarzhanau commented Feb 27, 2018

For our project (where some guys work in WebStorm and some use other editors along with git CLI) this also works fine:

{
  "scripts": {
    "precommit": "lint-staged",
    "postcommit": "git update-index -g"
  }
}

Thanks to Réda Housni Alaoui for the solution.

Vote for adding this to the docs, cause the issue seems to be a quite common one.

@taylorham

This comment has been minimized.

Contributor

taylorham commented Feb 27, 2018

I'll make a PR tonight!

@taylorham

This comment has been minimized.

Contributor

taylorham commented Feb 28, 2018

I did not make that PR!

Got swamped last night, sorry. I'll try to bang it out tonight.

@taylorham

This comment has been minimized.

Contributor

taylorham commented Mar 1, 2018

#408 is (finally) ready for review. Thanks for the opportunity to contribute!

@okonet okonet closed this in #408 Mar 8, 2018

Limess added a commit to Financial-Times/cmdb.js that referenced this issue Mar 28, 2018

@bosdo bosdo referenced this issue May 1, 2018

Merged

Pre-commit hooks #53

@JonasGroeger

This comment has been minimized.

JonasGroeger commented Aug 10, 2018

According to https://youtrack.jetbrains.com/issue/IDEA-135454 this issue will be fixed in IntelliJ IDEA 2018.3

@slorber

This comment has been minimized.

slorber commented Aug 12, 2018

great, if someone test 2018.3 please tell us if it works fine here

@spawnia

This comment has been minimized.

spawnia commented Sep 10, 2018

I am on PHPStorm EAP 2018.3 and neither the native solution nor the fix are working for me.

@alistair-hmh

This comment has been minimized.

alistair-hmh commented Sep 13, 2018

Is there a planned release date for 2018.3?

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