lint-staged stringifies my commands, changing their meaning (and breaking them)

[trusktr]

For example, I have the following in package.json:

	"scripts": {
		"typecheck": "tsc -p ./tsconfig.json --noEmit && :",
		"dev-or-master": "git branch | grep \\* | cut -d ' ' -f2 | grep -E 'develop|master' > /dev/null"
	},
	"lint-staged": {
		"*.{ts,tsx}": [
			"npm run dev-or-master && npm run typecheck || :"
		],
	},

and it doesn't work because lint-staged converts into this:

git branch | grep \* | cut -d ' ' -f2 | grep -E 'develop|master' > /dev/null "&&" "npm" "run" "typecheck" "||" ":" "/Users/trusktr/src/Signafy+mapper-annotator/src/annotator/App.tsx"

As you can see there, some parts of the command are wrapped with "", which causes those parts to be treated as arguments rather than interpreted by the shell.

It would be nice if you could fix this. I've already ran into in a few times, and as a result have to find some other way to write it.

[trusktr]

I think I see what the problem is: lint-staged is not handling npm run commands in the same way that npm actually handles them.

In npm, args can only be passed in by explicitly using --. For example, to pass args to a command using npm:

npm run something -- arg1 arg2

The -- is required there, in order for arg1 and arg2 to be passed into the something command as arguments.

However, it seems that lint-staged is assuming it works like this:

npm run something arg1 arg2

without requiring the --, and plus it is not actually passing args, but simply concatenating all the parts into a strings, so it just simply won't work.

[trusktr]

Just an idea, but instead of adding "", maybe you can just make the whole thing a string and run it in a subshell? My guess npm run does something like this.

[okonet]

Yeah thanks. That’s a good catch and we should fix it

[mAAdhaTTah]

This is messing up trying to integrate sed into lint-staged, as the -i '' part of the command gets wrapped in "" and thus always creates a backup with '' as the extension.

[raajnadar]

I think no one noticed this git branch | grep \\* the \ got escaped I am facing a serious issue with JEST test.

D:\native\react-native this is the original path but in the cmd the \n is converted into line break like this

D:
ative\react-native-paper

[okonet]

Please come up with a fix and submit a PR

[raajnadar]

@okonet in which file I can find the JEST testing code?? Any idea..! I went through each file but it is hard to understand. This issue looks very much similar #486

[okonet]

Tests are located in /tests

[okonet]

Does anyone want to take this one?

[mogafk]

now i think only about one solving. It split cmd string by && and || and then use something like reverse polsky notation(in case with ()). And then execute command-by-command. But it sophisticated and overkill. Any another suggestions?

[okonet]

lint-staged is not handling npm run commands in the same way that npm actually handles them.

We removed this handling in one of the breaking releases so it's now should be done explicitly AFAIK.

I'm also not quite sure what's going on since we're not escaping anything and just passing arguments to execa. So it might be an issue in execa?

We're using https://github.com/mccormicka/string-argv to parse arguments. Probably it has an issue? We'd need a better test suite for all corner cases in order to fix it. Anyone a PR?

[okonet]

Here is the appropriate test: https://github.com/okonet/lint-staged/blob/ac8cdf19b6c5cd1664a8ab428735e84d2e2fcd92/test/findBin.spec.js#L41-L55

[iiroj]

Now that execa v2.0.0 has been released with support for execa.command, this should be simple to fix. I’ll incorporate it in #639 if it works.

[okonet]

Be aware of the fact we rely on this to be true:

Default preferLocal option to false. If you are executing locally installed binaries, you'll need to manually specify preferLocal: true (#314)

[iiroj]

I think there's something wrong with using preferLocal: true with execa v2.0.0 (sindresorhus/execa#196), so it no longer works at all. I won't touch this now because it wasn't so simple...

[iiroj]

We could try with execa's shell: true option, so that the command is executed in a shell environment. This might help, but introduces other issues so it would be nice to be behind a config option in lint-staged as well.

[okonet]

Can we verify and close this?

[iiroj]

According to #640 (comment), at least the command parsing has been improved somewhat.

If @trusktr can verify that it now works, we can close this issue.

[iiroj]

I'll close this due to inactivity. @trusktr if it still doesn't work, let's reopen and investigate.