NodeModuleCopyHelper: filter() gets the parent dir, via a synchronous lstatSync, once per child entry

Version

app-builder-lib / electron-builder 26.15.7

Summary

In NodeModuleCopyHelper, the per-file filter is called with the parent directory rather than the file, from inside a per-child async pool, using the synchronous lstatSync. So fileMatched is computed identically for every child of a directory, and the same path is lstated once per entry, synchronously, on the packaging hot path.

Where

src/util/NodeModuleCopyHelper.ts, line 88:

const sortedFilePaths = await asyncPool(MAX_FILE_REQUESTS, childNames, async name => {
  const filePath = path.join(dirPath, name)
  const forceIncluded = onNodeModuleFile != null && !!onNodeModuleFile(filePath)
  if (excludedFiles.has(name) || name.startsWith("._")) {
    return null
  }
  // check if filematcher matches the files array as more important than the default excluded files.
  const fileMatched = filter != null && filter(dirPath, lstatSync(dirPath))   // <-- dirPath, per child
  if (!fileMatched || !forceIncluded || !!this.packager.config.disableDefaultIgnoredFiles) {

Two separate problems in that line

1. It passes dirPath where filePath looks intended. filePath is computed two lines above and is what the surrounding code (forceIncluded, and every name-based check below) is about. As written, filter is asked about the directory, so fileMatched has the same value for every child of that directory — the comment above it says "check if filematcher matches the files array", which reads like a per-file decision that is not being made.

2. lstatSync inside an async pool, re-stat'ing the same path. asyncPool(MAX_FILE_REQUESTS, childNames, …) exists to overlap I/O; a synchronous lstat in the callback blocks the event loop instead, and it is the same dirPath on every iteration, so the work is also redundant. Notably lstat (async) is imported at the top of this very file — line 3, import { lstat, lstatSync, readdir } from "fs-extra" — and line 88 is the only use of either.

Effect

This is on the node_modules copy path, which for us is the largest remaining phase of a Windows package: 33 s of a 98 s yarn build on windows-latest (electron-builder 26.15.7, ~9,000 archive entries). I have not isolated how much of that is this line, so I am reporting the code rather than claiming a number — the correctness half seems worth a look regardless of the cost.

Suggested fix

const fileMatched = filter != null && filter(filePath, await lstat(filePath))

— or, if the parent really is the intended argument, hoist it out of the per-child callback so it is computed once per directory with the async lstat.

Happy to supply a profile or test a patch.