Merge pull request #857 from okonet/fix-relative-add

fix: resolve matched files to cwd instead of gitDir before adding

lib/chunkFiles.js

@@ -33,14 +33,15 @@ function chunkArray(arr, chunkCount) {
  * @returns {Array<Array<String>>}
  */
 module.exports = function chunkFiles({ files, baseDir, maxArgLength = null, relative = false }) {
+  const normalizedFiles = files.map((file) =>
+    normalize(relative || !baseDir ? file : path.resolve(baseDir, file))
+  )
+
   if (!maxArgLength) {
     debug('Skip chunking files because of undefined maxArgLength')
-    return [files]
+    return [normalizedFiles] // wrap in an array to return a single chunk
   }
 
-  const normalizedFiles = files.map((file) =>
-    normalize(relative || !baseDir ? file : path.resolve(baseDir, file))
-  )
   const fileListLength = normalizedFiles.join(' ').length
   debug(
     `Resolved an argument string length of ${fileListLength} characters from ${normalizedFiles.length} files`

lib/gitWorkflow.js

@@ -3,7 +3,6 @@
 const debug = require('debug')('lint-staged:git')
 const path = require('path')
 
-const chunkFiles = require('./chunkFiles')
 const execGit = require('./execGit')
 const { readFile, unlink, writeFile } = require('./file')
 const {
@@ -63,15 +62,14 @@ const handleError = (error, ctx, symbol) => {
 }
 
 class GitWorkflow {
-  constructor({ allowEmpty, gitConfigDir, gitDir, matchedFiles, maxArgLength }) {
+  constructor({ allowEmpty, gitConfigDir, gitDir, matchedFileChunks }) {
     this.execGit = (args, options = {}) => execGit(args, { ...options, cwd: gitDir })
     this.deletedFiles = []
     this.gitConfigDir = gitConfigDir
     this.gitDir = gitDir
     this.unstagedDiff = null
     this.allowEmpty = allowEmpty
-    this.matchedFiles = matchedFiles
-    this.maxArgLength = maxArgLength
+    this.matchedFileChunks = matchedFileChunks
 
     /**
      * These three files hold state about an ongoing git merge
@@ -245,19 +243,12 @@ class GitWorkflow {
    */
   async applyModifications(ctx) {
     debug('Adding task modifications to index...')
-    // `matchedFiles` includes staged files that lint-staged originally detected and matched against a task.
-    // Add only these files so any 3rd-party edits to other files won't be included in the commit.
-    const files = Array.from(this.matchedFiles)
-    // Chunk files for better Windows compatibility
-    const matchedFileChunks = chunkFiles({
-      baseDir: this.gitDir,
-      files,
-      maxArgLength: this.maxArgLength,
-    })
 
+    // `matchedFileChunks` includes staged files that lint-staged originally detected and matched against a task.
+    // Add only these files so any 3rd-party edits to other files won't be included in the commit.
     // These additions per chunk are run "serially" to prevent race conditions.
     // Git add creates a lockfile in the repo causing concurrent operations to fail.
-    for (const files of matchedFileChunks) {
+    for (const files of this.matchedFileChunks) {
       await this.execGit(['add', '--', ...files])
     }
 

lib/runAll.js

@@ -193,7 +193,16 @@ const runAll = async (
     return ctx
   }
 
-  const git = new GitWorkflow({ allowEmpty, gitConfigDir, gitDir, matchedFiles, maxArgLength })
+  // Chunk matched files for better Windows compatibility
+  const matchedFileChunks = chunkFiles({
+    // matched files are relative to `cwd`, not `gitDir`, when `relative` is used
+    baseDir: cwd,
+    files: Array.from(matchedFiles),
+    maxArgLength,
+    relative: false,
+  })
+
+  const git = new GitWorkflow({ allowEmpty, gitConfigDir, gitDir, matchedFileChunks })
 
   const runner = new Listr(
     [

test/chunkFiles.spec.js

@@ -1,8 +1,11 @@
+import normalize from 'normalize-path'
+import path from 'path'
+
 import chunkFiles from '../lib/chunkFiles'
 
 describe('chunkFiles', () => {
   const files = ['example.js', 'foo.js', 'bar.js', 'foo/bar.js']
-  const baseDir = '/opt/git/example.git'
+  const baseDir = normalize('/opt/git/example.git')
 
   it('should default to sane value', () => {
     const chunkedFiles = chunkFiles({ baseDir, files: ['foo.js'], relative: true })
@@ -26,4 +29,9 @@ describe('chunkFiles', () => {
       [files[2], files[3]],
     ])
   })
+
+  it('should resolve paths when relative: false', () => {
+    const chunkedFiles = chunkFiles({ baseDir, files, relative: false })
+    expect(chunkedFiles).toEqual([files.map((file) => normalize(path.resolve(baseDir, file)))])
+  })
 })

test/runAll.spec.js

@@ -201,4 +201,33 @@ describe('runAll', () => {
       LOG [SUCCESS] Cleaning up..."
     `)
   })
+
+  it('should resolve matched files to cwd when using relative option', async () => {
+    // A staged file inside test/, which will be our cwd
+    getStagedFiles.mockImplementationOnce(async () => ['test/foo.js'])
+
+    // We are only interested in the `matchedFileChunks` generation
+    let expected
+    const mockConstructor = jest.fn(({ matchedFileChunks }) => (expected = matchedFileChunks))
+    GitWorkflow.mockImplementationOnce(mockConstructor)
+
+    const mockTask = jest.fn(() => ['echo "sample"'])
+
+    // actual cwd
+    const cwd = process.cwd()
+    // For the test, set cwd in test/
+    const innerCwd = path.join(cwd, 'test/')
+    try {
+      // Run lint-staged in `innerCwd` with relative option
+      // This means the sample task will receive `foo.js`
+      await runAll({ config: { '*.js': mockTask }, stash: false, relative: true, cwd: innerCwd })
+    } catch {} // eslint-disable-line no-empty
+
+    // task received relative `foo.js`
+    expect(mockTask).toHaveBeenCalledTimes(1)
+    expect(mockTask).toHaveBeenCalledWith(['foo.js'])
+    // GitWorkflow received absolute `test/foo.js`
+    expect(mockConstructor).toHaveBeenCalledTimes(1)
+    expect(expected).toEqual([[normalize(path.join(cwd, 'test/foo.js'))]])
+  })
 })