fix: do not copy nested node modules to root

Close #3039, Close #3047

package.json

@@ -29,7 +29,7 @@
   "////": "All typings are added into root `package.json` to avoid duplication errors in the IDE compiler (several `node.d.ts` files).",
   "dependencies": {
     "7zip-bin": "~4.0.2",
-    "app-builder-bin": "1.9.14",
+    "app-builder-bin": "1.9.15",
     "archiver": "^2.1.1",
     "async-exit-hook": "^2.0.1",
     "aws-sdk": "^2.263.1",

packages/builder-util/package.json

@@ -11,7 +11,7 @@
     "out"
   ],
   "dependencies": {
-    "app-builder-bin": "1.9.14",
+    "app-builder-bin": "1.9.15",
     "temp-file": "^3.1.2",
     "fs-extra-p": "^4.6.1",
     "is-ci": "^1.1.0",

packages/electron-builder-lib/package.json

@@ -42,7 +42,7 @@
   "homepage": "https://github.com/electron-userland/electron-builder",
   "dependencies": {
     "7zip-bin": "~4.0.2",
-    "app-builder-bin": "1.9.14",
+    "app-builder-bin": "1.9.15",
     "async-exit-hook": "^2.0.1",
     "bluebird-lst": "^1.0.5",
     "chromium-pickle-js": "^0.2.0",

packages/electron-builder-lib/src/fileMatcher.ts

@@ -20,6 +20,22 @@ export const excludedNames = ".git,.hg,.svn,CVS,RCS,SCCS," +
 
 export const excludedExts = "iml,hprof,orig,pyc,pyo,rbc,swp,csproj,sln,suo,xproj,cc,d.ts"
 
+function ensureNoEndSlash(file: string): string {
+  if (path.sep !== "/") {
+    file = file.replace(/\//g, path.sep)
+  }
+  if (path.sep !== "\\") {
+    file = file.replace(/\\/g, path.sep)
+  }
+
+  if (file.endsWith(path.sep)) {
+    return file.substring(0, file.length - 1)
+  }
+  else {
+    return file
+  }
+}
+
 /** @internal */
 export class FileMatcher {
   readonly from: string
@@ -32,8 +48,8 @@ export class FileMatcher {
   readonly isSpecifiedAsEmptyArray: boolean
 
   constructor(from: string, to: string, readonly macroExpander: (pattern: string) => string, patterns?: Array<string> | string | null | undefined) {
-    this.from = macroExpander(from)
-    this.to = macroExpander(to)
+    this.from = ensureNoEndSlash(macroExpander(from))
+    this.to = ensureNoEndSlash(macroExpander(to))
     this.patterns = asArray(patterns).map(it => this.normalizePattern(it))
     this.isSpecifiedAsEmptyArray = Array.isArray(patterns) && patterns.length === 0
   }

packages/electron-builder-lib/src/util/AppFileCopierHelper.ts

@@ -8,6 +8,7 @@ import { excludedExts, FileMatcher } from "../fileMatcher"
 import { createElectronCompilerHost, NODE_MODULES_PATTERN } from "../fileTransformer"
 import { Packager } from "../packager"
 import { PlatformPackager } from "../platformPackager"
+import { getDestinationPath } from "./appFileCopier"
 import { AppFileWalker } from "./AppFileWalker"
 import { NodeModuleCopyHelper } from "./NodeModuleCopyHelper"
 
@@ -68,7 +69,6 @@ export async function computeFileSets(matchers: Array<FileMatcher>, transformer:
 
     const files = await walk(matcher.from, fileWalker.filter, fileWalker)
     const metadata = fileWalker.metadata
-
     fileSets.push(validateFileSet({src: matcher.from, files, metadata, transformedFiles: await transformFiles(transformer, files, metadata), destination: matcher.to}))
   }
 
@@ -119,13 +119,22 @@ export async function copyNodeModules(platformPackager: PlatformPackager<any>, m
   // mapSeries instead of map because copyNodeModules is concurrent and so, no need to increase queue/pressure
   return await BluebirdPromise.mapSeries(deps, async info => {
     const source = info.dir
+
+    let to: string
+    if (source.length > mainMatcher.from.length && source.startsWith(mainMatcher.from) && source[mainMatcher.from.length] === path.sep) {
+      to = getDestinationPath(source, {src: mainMatcher.from, destination: mainMatcher.to, files: [], metadata: null as any})
+    }
+    else {
+      to = mainMatcher.to + path.sep + "node_modules"
+    }
+
     // use main matcher patterns, so, user can exclude some files in such hoisted node modules
     // source here includes node_modules, but pattern base should be without because users expect that pattern "!node_modules/loot-core/src{,/**/*}" will work
-    const matcher = new FileMatcher(path.dirname(source), mainMatcher.to + path.sep + "node_modules", mainMatcher.macroExpander, mainMatcher.patterns)
+    const matcher = new FileMatcher(path.dirname(source), to, mainMatcher.macroExpander, mainMatcher.patterns)
     const copier = new NodeModuleCopyHelper(matcher, platformPackager.info)
     const names = info.deps
     const files = await copier.collectNodeModules(source, names, nodeModuleExcludedExts)
-    return validateFileSet({src: source, destination: matcher.to, files, transformedFiles: await transformFiles(transformer, files, copier.metadata), metadata: copier.metadata})
+    return validateFileSet({src: source, destination: to, files, transformedFiles: await transformFiles(transformer, files, copier.metadata), metadata: copier.metadata})
   })
 }
 
@@ -178,8 +187,3 @@ require('electron-compile').init(__dirname, require('path').resolve(__dirname, '
 `)
   return {src: electronCompileCache, files: cacheFiles, metadata, destination: mainFileSet.destination}
 }
-
-// sometimes, destination may not contain path separator in the end (path to folder), but the src does. So let's ensure paths have path separators in the end
-export function ensureEndSlash(s: string) {
-  return s === "" || s.endsWith(path.sep) ? s : (s + path.sep)
-}

packages/electron-builder-lib/src/util/AppFileWalker.ts

@@ -1,8 +1,7 @@
-import { FileConsumer } from "builder-util/out/fs"
-import { Stats } from "fs-extra-p"
+import { Filter, FileConsumer } from "builder-util/out/fs"
+import { readlink, stat, Stats } from "fs-extra-p"
 import { FileMatcher } from "../fileMatcher"
 import { Packager } from "../packager"
-import { NodeModuleCopyHelper } from "./NodeModuleCopyHelper"
 import * as path from "path"
 
 const nodeModulesSystemDependentSuffix = `${path.sep}node_modules`
@@ -14,10 +13,67 @@ function addAllPatternIfNeed(matcher: FileMatcher) {
   return matcher
 }
 
+export abstract class FileCopyHelper {
+  readonly metadata = new Map<string, Stats>()
+
+  protected constructor(protected readonly matcher: FileMatcher, readonly filter: Filter | null, protected readonly packager: Packager) {
+  }
+
+  protected handleFile(file: string, fileStat: Stats): Promise<Stats | null> | null {
+    if (!fileStat.isSymbolicLink()) {
+      return null
+    }
+
+    return readlink(file)
+      .then((linkTarget): any => {
+        // http://unix.stackexchange.com/questions/105637/is-symlinks-target-relative-to-the-destinations-parent-directory-and-if-so-wh
+        return this.handleSymlink(fileStat, file, path.resolve(path.dirname(file), linkTarget))
+      })
+  }
+
+  protected handleSymlink(fileStat: Stats, file: string, linkTarget: string): Promise<Stats> | null {
+    const link = path.relative(this.matcher.from, linkTarget)
+    if (link.startsWith("..")) {
+      // outside of project, linked module (https://github.com/electron-userland/electron-builder/issues/675)
+      return stat(linkTarget)
+        .then(targetFileStat => {
+          this.metadata.set(file, targetFileStat)
+          return targetFileStat
+        })
+    }
+    else {
+      (fileStat as any).relativeLink = link
+    }
+    return null
+  }
+}
+
+function createAppFilter(matcher: FileMatcher, packager: Packager): Filter | null {
+  if (packager.areNodeModulesHandledExternally) {
+    return matcher.isEmpty() ? null : matcher.createFilter()
+  }
+
+  const nodeModulesFilter: Filter = (file, fileStat) => {
+    return !(fileStat.isDirectory() && file.endsWith(nodeModulesSystemDependentSuffix))
+  }
+
+  if (matcher.isEmpty()) {
+    return nodeModulesFilter
+  }
+
+  const filter = matcher.createFilter()
+  return (file, fileStat) => {
+    if (!nodeModulesFilter(file, fileStat)) {
+      return false
+    }
+    return filter(file, fileStat)
+  }
+}
+
 /** @internal */
-export class AppFileWalker extends NodeModuleCopyHelper implements FileConsumer {
+export class AppFileWalker extends FileCopyHelper implements FileConsumer {
   constructor(matcher: FileMatcher, packager: Packager) {
-    super(addAllPatternIfNeed(matcher), packager)
+    super(addAllPatternIfNeed(matcher), createAppFilter(matcher, packager), packager)
   }
 
   // noinspection JSUnusedGlobalSymbols

packages/electron-builder-lib/src/util/NodeModuleCopyHelper.ts

@@ -1,49 +1,19 @@
 import BluebirdPromise from "bluebird-lst"
-import { CONCURRENCY, Filter } from "builder-util/out/fs"
-import { lstat, readdir, readlink, stat, Stats } from "fs-extra-p"
+import { CONCURRENCY } from "builder-util/out/fs"
+import { lstat, readdir } from "fs-extra-p"
 import * as path from "path"
 import { excludedNames, FileMatcher } from "../fileMatcher"
 import { Packager } from "../packager"
 import { resolveFunction } from "../platformPackager"
+import { FileCopyHelper } from "./AppFileWalker"
 
 const excludedFiles = new Set([".DS_Store", "node_modules" /* already in the queue */, "CHANGELOG.md", "ChangeLog", "changelog.md", "binding.gyp", ".npmignore"].concat(excludedNames.split(",")))
 const topLevelExcludedFiles = new Set(["test.js", "karma.conf.js", ".coveralls.yml", "README.md", "readme.markdown", "README", "readme.md", "readme", "test", "__tests__", "tests", "powered-test", "example", "examples"])
 
 /** @internal */
-export class NodeModuleCopyHelper {
-  readonly metadata = new Map<string, Stats>()
-  readonly filter: Filter | null
-
-  constructor(private readonly matcher: FileMatcher, protected readonly packager: Packager) {
-    this.filter = matcher.isEmpty() ? null : matcher.createFilter()
-  }
-
-  protected handleFile(file: string, fileStat: Stats): Promise<Stats | null> | null {
-    if (!fileStat.isSymbolicLink()) {
-      return null
-    }
-
-    return readlink(file)
-      .then((linkTarget): any => {
-        // http://unix.stackexchange.com/questions/105637/is-symlinks-target-relative-to-the-destinations-parent-directory-and-if-so-wh
-        return this.handleSymlink(fileStat, file, path.resolve(path.dirname(file), linkTarget))
-      })
-  }
-
-  protected handleSymlink(fileStat: Stats, file: string, linkTarget: string): Promise<Stats> | null {
-    const link = path.relative(this.matcher.from, linkTarget)
-    if (link.startsWith("..")) {
-      // outside of project, linked module (https://github.com/electron-userland/electron-builder/issues/675)
-      return stat(linkTarget)
-        .then(targetFileStat => {
-          this.metadata.set(file, targetFileStat)
-          return targetFileStat
-        })
-    }
-    else {
-      (fileStat as any).relativeLink = link
-    }
-    return null
+export class NodeModuleCopyHelper extends FileCopyHelper {
+  constructor(matcher: FileMatcher, packager: Packager) {
+    super(matcher, matcher.isEmpty() ? null : matcher.createFilter(), packager)
   }
 
   async collectNodeModules(baseDir: string, moduleNames: Iterable<string>, nodeModuleExcludedExts: Array<string>): Promise<Array<string>> {

packages/electron-builder-lib/src/util/appFileCopier.ts

@@ -5,16 +5,16 @@ import { ensureDir, readlink, Stats, symlink, writeFile } from "fs-extra-p"
 import * as path from "path"
 import { NODE_MODULES_PATTERN } from "../fileTransformer"
 import { Packager } from "../packager"
-import { ensureEndSlash, ResolvedFileSet } from "./AppFileCopierHelper"
+import { ResolvedFileSet } from "./AppFileCopierHelper"
 
 export function getDestinationPath(file: string, fileSet: ResolvedFileSet) {
   if (file === fileSet.src) {
     return fileSet.destination
   }
   else {
-    const src = ensureEndSlash(fileSet.src)
-    const dest = ensureEndSlash(fileSet.destination)
-    if (file.startsWith(src)) {
+    const src = fileSet.src
+    const dest = fileSet.destination
+    if (file.length > src.length && file.startsWith(src) && file[src.length] === path.sep) {
       return dest + file.substring(src.length)
     }
     else {

packages/electron-builder-lib/src/util/filter.ts

@@ -2,7 +2,6 @@ import { Filter } from "builder-util/out/fs"
 import { Stats } from "fs-extra-p"
 import { Minimatch } from "minimatch"
 import * as path from "path"
-import { ensureEndSlash } from "./AppFileCopierHelper"
 
 /** @internal */
 export function hasMagic(pattern: Minimatch) {
@@ -20,6 +19,11 @@ export function hasMagic(pattern: Minimatch) {
   return false
 }
 
+// sometimes, destination may not contain path separator in the end (path to folder), but the src does. So let's ensure paths have path separators in the end
+function ensureEndSlash(s: string) {
+  return s.length === 0 || s.endsWith(path.sep) ? s : (s + path.sep)
+}
+
 /** @internal */
 export function createFilter(src: string, patterns: Array<Minimatch>, excludePatterns?: Array<Minimatch> | null): Filter {
   const pathSeparator = path.sep