Repository navigation
"Experimental" fs.cp(), fs.cpSync(), fsPromises.cp() methods don't handle recursion when there is a filter #49092
Description
Version
18.17.0
Platform
Windows
Subsystem
No response
What steps will reproduce the bug?
Any simple recursive call to cp to recursively copy a director, when there is a filter that does not always return true
I used
await cp('src', 'lib', {
recursive: true,
filter: (source, _destination) => {
console.log(source, _destination, source.endsWith('.css'));
return source.endsWith('.css');
}
});to copy only .css files after creating "lib/" from "src/" with tsc in a React JSX library project, and it stopped right at the top level directory.
I also checked non-promise cp() and cpSync().
How often does it reproduce? Is there a required condition?
Always
What is the expected behavior? Why is that the expected behavior?
Expected: Recurse over all files and subdirectories no matter what the filter function returns
What do you see instead?
Recursion stops completely when the filter function returns false. The rest of the not yet visited files and sub directories are ignored and not handled.
Additional information
Note: All directories are definitely present since my example use case copies CSS files from src/ into lib/ after compiling the project with tsc. However, this leaves open the question of what would happen if the destination directory does not exist? Feature question: When using a filter it's possible to skip directories. Of course, one could ask the programmer to handle directories and return true in such cases? Or would it also be okay to add a feature to create any missing directories? Not sure how it behaves right now. In any case, that's only an additional consideration and not the issue of this... issue.
Activity
lll000111 commentedon Aug 10, 2023
Related: #44598
aduh95 commentedon Aug 10, 2023
@nodejs/fs
LiviaMedeiros commentedon Aug 11, 2023
Currently,
options.filter()is applied to both directories and files. Falsey value returned for directory means that it won't be copied nor traversed.'src'.endsWith('.css') === false, hence thesrcdirectory itself is not traversed.To achieve copying the whole directory structure but filtering non-directories, something like this would work:
await cp('src', 'lib', { recursive: true, - filter: (source, _destination) => { + filter: async (source, _destination) => { console.log(source, _destination, source.endsWith('.css')); - return source.endsWith('.css'); + return (await lstat(source)).isDirectory() || source.endsWith('.css'); } });If this is a very common usecase, we might consider adding some sort of
options.filterUsingStats()that will take objects rather than path strings. It would makeisDirectory()check simpler, and allowcp()to avoid callingstattwice on dirents that pass the filter.lll000111 commentedon Aug 11, 2023
I'd be okay with the current function — if the documentation made that clear. Right now it only says "Function to filter copied files/directories. Return true to copy the item, false to ignore it.", and while technically not traversing can be seen as ignoring, I think it should be stated explicitly.
aduh95 commentedon Aug 11, 2023
Would you like to send a PR to update the docs?
shubham9411 commentedon Aug 22, 2023
Hi @aduh95, I have added a PR for updating the docs for this. #49289
doc: update `options.filter` description for `fs.cp`
doc: update `options.filter` description for `fs.cp`
doc: update `options.filter` description for `fs.cp`
doc: update `options.filter` description for `fs.cp`
doc: update `options.filter` description for `fs.cp`