lint-staged
lint-staged copied to clipboard
fix: run task command on files deep copy
This prevents the task command to change the files array by inadvertence. It happened to me and I took a long moment to diagnose the issue.
Task in generateTasks
{
pattern: 'apps/dummy/dummy/**/*.js',
commands: [Function: apps/dummy/dummy/**/*.js],
fileList: [
'/Users/lounesksouri/dummy/apps/dummy/dummy/src/config.js'
]
}
Task in runAll after makeCmdTasks
{
pattern: 'apps/dummy/dummy/**/*.js',
commands: [Function: apps/dummy/dummy/**/*.js],
fileList: []
}
So you're saying something can mutate the list of files? Do you have a reproduction case? I'm wondering how this is possible. Also let's create an issue for this first and reference the PR.
Also before we can merge it we would need a test for this.
So you're saying something can mutate the list of files? Do you have a reproduction case? I'm wondering how this is possible.
Yeah, I faced this issue running Array.prototype.splice
on a reference to filenames
in a subsequent function. This method directly modifies the object.
Here is an example:
const divideArray = (arr, size) => {
return new Array(Math.ceil(arr.length / size))
.fill()
.map((_) => arr.splice(0, size))
}
const splitCommand = (filenames, commandFormat) => {
const splitFilenames = divideArray(filenames, 5)
return splitFilenames.map((files) => `${commandFormat} ${files.join(' ')}`)
}
module.exports = {
'*.js': (filenames) => [
"pnpm prettier --write '*.js'",
...splitCommand(filenames, 'pnpm run --silent eslint --fix'),
],
}
This basically outputs the same command applied to a subset of the filenames. If there are 10 changed files, the splitCommand
will output an array of commands applied to files 5 by 5.
You can see that the files
has changed by logging the variabe right before and after the following line: https://github.com/okonet/lint-staged/blob/17c51aff00ea73f9588132c28eedbce535ee1ad8/lib/makeCmdTasks.js#L46
Not sure to have the time to add a test this week.
Deep copying the array indeed solves the issue:
const divideArray = (arr, size) => {
const arrCopy = [...arr]
return new Array(Math.ceil(arrCopy.length / size))
.fill()
.map((_) => arrCopy.splice(0, size))
}
But it would be definitely better if the library handled that case internally.
Ping @louneskmt I rebased this PR and added a changeset file!
Thanks for the unit test and the merge!
Thanks @louneskmt for your contribution!