Skip to content

Commit e8d9c22

Browse files
refactor: simplify copyDirectoryContents implementation
Replaced manual glob and loop with a single fsCopy call from fs-extra. This improves performance, robustness against EMFILE errors, and significantly simplifies the code while maintaining the same behavior. Added a learning entry to .jules/refactor.md. Co-authored-by: gonzaloriestra <14979109+gonzaloriestra@users.noreply.github.com>
1 parent 3a4bf9c commit e8d9c22

2 files changed

Lines changed: 5 additions & 17 deletions

File tree

.jules/refactor.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
## 2025-05-15 - Prefer native library utility for directory copy
2+
**Smell:** Manual loop and `glob` to copy a directory's contents when a standard utility already exists in the codebase.
3+
**Learning:** The previous manual implementation of `copyDirectoryContents` used `glob` + `Promise.all` with individual `copyFile` calls. This is not only more complex but could lead to EMFILE errors on large directories. Using `fsCopy` (which wraps `fs-extra.copy`) is simpler and more robust as it handles concurrency and path normalization natively.
4+
**Prevention:** Always check if a higher-level utility exists in the local `fs.ts` or the underlying `fs-extra` library before implementing manual directory traversal.

packages/cli-kit/src/public/node/fs.ts

Lines changed: 1 addition & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -717,21 +717,5 @@ export async function copyDirectoryContents(srcDir: string, destDir: string): Pr
717717
throw new Error(`Source directory ${srcDir} does not exist`)
718718
}
719719

720-
if (!(await fileExists(destDir))) {
721-
await mkdir(destDir)
722-
}
723-
724-
// Get all files and directories in the source directory
725-
const items = await glob(joinPath(srcDir, '**/*'))
726-
727-
const filesToCopy = []
728-
729-
for (const item of items) {
730-
const relativePath = item.replace(srcDir, '').replace(/^[/\\]/, '')
731-
const destPath = joinPath(destDir, relativePath)
732-
733-
filesToCopy.push(copyFile(item, destPath))
734-
}
735-
736-
await Promise.all(filesToCopy)
720+
await fsCopy(srcDir, destDir)
737721
}

0 commit comments

Comments
 (0)