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

fix: run task command on files deep copy #1217

Merged
merged 1 commit into from
Oct 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/sixty-stingrays-glow.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'lint-staged': patch
---

Previously it was possible for a function task to mutate the list of staged files passed to the function, and accidentally affect the generation of other tasks. This is now fixed by passing a copy of the original file list instead.
4 changes: 3 additions & 1 deletion lib/makeCmdTasks.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,9 @@ export const makeCmdTasks = async ({ commands, cwd, files, gitDir, shell, verbos
for (const cmd of commandArray) {
// command function may return array of commands that already include `stagedFiles`
const isFn = typeof cmd === 'function'
const resolved = isFn ? await cmd(files) : cmd

/** Pass copy of file list to prevent mutation by function from config file. */
const resolved = isFn ? await cmd([...files]) : cmd

const resolvedArray = Array.isArray(resolved) ? resolved : [resolved] // Wrap non-array command as array

Expand Down
21 changes: 21 additions & 0 deletions test/unit/makeCmdTasks.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -124,4 +124,25 @@ describe('makeCmdTasks', () => {
Function task should return a string or an array of strings"
`)
})

it('should prevent function from mutating original file list', async () => {
const files = ['test.js']

const res = await makeCmdTasks({
commands: (stagedFiles) => {
/** Array.splice() mutates the array */
stagedFiles.splice(0, 1)
expect(stagedFiles).toEqual([])
return stagedFiles.map((file) => `test ${file}`)
},
gitDir,
files,
})

/** Because function mutated file list, it was empty and no tasks were created... */
expect(res.length).toBe(0)

/** ...but the original file list was not mutated */
expect(files).toEqual(['test.js'])
})
})