"Experimental" fs.cp(), fs.cpSync(), fsPromises.cp() methods don't handle recursion when there is a filter

[lll000111]

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.

[lll000111]

Related: #44598

[aduh95]

@nodejs/fs

[LiviaMedeiros]

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 the src directory 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 make isDirectory() check simpler, and allow cp() to avoid calling stat twice on dirents that pass the filter.

[lll000111]

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]

Would you like to send a PR to update the docs?

[shubham9411]

Hi @aduh95, I have added a PR for updating the docs for this. #49289