[Feature Request] [copy] pass srcStat and destStat to filter

[jedwards1211]

I'm willing to make a PR if you're on board with this 🙂

As mentioned in #843, when doing a copy, filter is actually called with both files and directories. This means if we only want to copy .js files, our filter has to be filter: async f => (await fs.stat(f)).isDirectory() || f.endsWith('.js'), which is cumbersome and redundant.

copy is already getting the stats of everything along the way, so if it would pass them to the filter then we could avoid getting them redundantly: filter: (src, dest, srcStat, destStat) => srcStat.isDirectory() || src.endsWith('.js')

[RyanZim]

Yeah, if we're already getting stats, no reason not to pass them that I can see; @manidlou?

[manidlou]

Yeah we already get the src and dest stats before we check the filter, so yeah we could pass them to the filter function.

[tomaszcoding]

It would help a lot, but it will not solve all the problems. In the example given by @jedwards1211 the filter function:
filter: (src, dest, srcStat, destStat) => srcStat.isDirectory() || src.endsWith('.js')
will simply copy all folders, no matter if they contain any js files or not. If I would like to copy only those folders that have relevant content I still can't do it.
I would expect that copy examines all nested files and folders but only those which match the filter are copied to appropriate folder in dest.

[RyanZim]

@tomaszcoding Unfortunately, that behavior isn't possible with our implementation, since we don't examine the contents of folders until we run them through the filter.

[jedwards1211]

I can imagine a solution where the filter function returns a special value that means read this directory but don't create the target directory in the destination yet. Then, when copying a file or directory that the filter returned true for, we would mkdirp its parent (if we haven't already in the current copy operation).

[tomaszcoding]

I can imagine a solution where the filter function returns a special value

I think filter should return a boolean (or a Promise resolving to a boolean). This is natural and this is what most people would expect. Also I wouldn't force people to care about performance if they don't have to. The filter function should be responsible for single thing i.e. filtering. Anything more than this would be to imperative in my opinion.

Also I wouldn't be so concerned about efficiency here. @RyanZim I understand you would like to implement kind of "lazy" filtering? I believe most users need the "greedy" one (in the end, copy from fs-extra was made to simplify copying recursively). This is the 2nd time I'm using fs-extra and the 1st time I'm using it with a filter and I already hit this problem. I had to use node's fs.promises.copyFile. I would make node's fs a fallback for those who really need to optimise their scripts and not vice-versa.

[RyanZim]

fs-extra has implemented "lazy" filtering from day one AFAIK.

[RyanZim]

@manidlou Given #912; should we be advocating for getting this added to fs.cp? I have no desire to deviate our implementation any further independent of Node core.

[manidlou]

@RyanZim I agree with you in terms of being aligned with node core.

However, generally speaking, having fs.cp() in node core, creates an interesting situation for us! What I mean by that is, if we want to be aligned with node core, how do we handle new feature requests? Do we direct them to node core or we still considering implementing them here at fs-extra?

[RyanZim]

I would direct them to Node core.

[RyanZim]

I'm going to close this out for now; if someone wants this feature, please lobby for its addition in Node core.