fix: packages in the workspace not being under node_modules

.changeset/nasty-mangos-yawn.md

@@ -0,0 +1,6 @@
+---
+"app-builder-lib": patch
+"builder-util": patch
+---
+
+fix: packages in the workspace not being under node_modules

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

@@ -1,4 +1,4 @@
-import { Filter, FileConsumer } from "builder-util"
+import { Filter, FileConsumer, FilterStats } from "builder-util"
 import { readlink, stat, Stats } from "fs-extra"
 import { FileMatcher } from "../fileMatcher"
 import { Packager } from "../packager"
@@ -41,7 +41,7 @@ export abstract class FileCopyHelper {
         return targetFileStat
       })
     } else {
-      const s = fileStat as any
+      const s: FilterStats = fileStat
       s.relativeLink = link
       s.linkRelativeToFile = path.relative(parent, resolvedLinkTarget)
     }

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

@@ -1,5 +1,5 @@
 import BluebirdPromise from "bluebird-lst"
-import { CONCURRENCY } from "builder-util"
+import { CONCURRENCY, FilterStats } from "builder-util"
 import { lstat, readdir, lstatSync } from "fs-extra"
 import * as path from "path"
 import { excludedNames, FileMatcher } from "../fileMatcher"
@@ -21,7 +21,7 @@
     "binding.gyp",
     ".npmignore",
     "node_gyp_bins",
   ].concat(excludedNames.split(",")))
 
 const topLevelExcludedFiles = new Set([
   "karma.conf.js",
@@ -112,7 +112,8 @@
             }
           }
 
-          return lstat(filePath).then(stat => {
+          return lstat(filePath).then((stat: FilterStats) => {
+            stat.relativeNodeModulesPath = path.join("node_modules", moduleName, path.relative(depPath, filePath))
             if (filter != null && !filter(filePath, stat)) {
               return null
             }

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

@@ -188,42 +188,24 @@ export async function computeNodeModuleFileSets(platformPackager: PlatformPackag
   const result = new Array<ResolvedFileSet>()
   let index = 0
   const NODE_MODULES = "node_modules"
-  const getRealSource = (name: string, source: string) => {
-    let parentDir = path.dirname(source)
 
-    const scopeDepth = name.split("/").length
-    // get the parent dir of the package, input: /root/path/node_modules/@electron/remote, output: /root/path/node_modules
-    for (let i = 0; i < scopeDepth - 1; i++) {
-      parentDir = path.dirname(parentDir)
-    }
-
-    // for the local node modules which is not in node modules
-    if (!parentDir.endsWith(path.sep + NODE_MODULES)) {
-      return parentDir
-    }
-
-    // use main matcher patterns,return parent dir of the node_modules, so user can exclude some files !node_modules/xxxx
-    return path.dirname(parentDir)
-  }
-
-  const collectNodeModules = async (dep: NodeModuleInfo, realSource: string, destination: string) => {
+  const collectNodeModules = async (dep: NodeModuleInfo, destination: string) => {
     const source = dep.dir
-    const matcher = new FileMatcher(realSource, destination, mainMatcher.macroExpander, mainMatcher.patterns)
+    const matcher = new FileMatcher(source, destination, mainMatcher.macroExpander, mainMatcher.patterns)
     const copier = new NodeModuleCopyHelper(matcher, platformPackager.info)
     const files = await copier.collectNodeModules(dep, nodeModuleExcludedExts)
     result[index++] = validateFileSet({ src: source, destination, files, metadata: copier.metadata })
 
     if (dep.conflictDependency) {
       for (const c of dep.conflictDependency) {
-        await collectNodeModules(c, realSource, path.join(destination, NODE_MODULES, c.name))
+        await collectNodeModules(c, path.join(destination, NODE_MODULES, c.name))
       }
     }
   }
 
   for (const dep of deps) {
     const destination = path.join(mainMatcher.to, NODE_MODULES, dep.name)
-    const realSource = getRealSource(dep.name, dep.dir)
-    await collectNodeModules(dep, realSource, destination)
+    await collectNodeModules(dep, destination)
   }
 
   return result

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

@@ -1,8 +1,6 @@
-import { Filter } from "builder-util"
-import { Stats } from "fs-extra"
+import { Filter, FilterStats } from "builder-util"
 import { Minimatch } from "minimatch"
 import * as path from "path"
-import { NODE_MODULES_PATTERN } from "../fileTransformer"
 
 /** @internal */
 export function hasMagic(pattern: Minimatch) {
@@ -25,17 +23,8 @@ function ensureEndSlash(s: string) {
   return s.length === 0 || s.endsWith(path.sep) ? s : s + path.sep
 }
 
-function getRelativePath(file: string, srcWithEndSlash: string) {
-  if (!file.startsWith(srcWithEndSlash)) {
-    const index = file.indexOf(NODE_MODULES_PATTERN)
-    if (index < 0) {
-      throw new Error(`${file} must be under ${srcWithEndSlash}`)
-    } else {
-      return file.substring(index + 1 /* leading slash */)
-    }
-  }
-
-  let relative = file.substring(srcWithEndSlash.length)
+function getRelativePath(file: string, srcWithEndSlash: string, stat: FilterStats) {
+  let relative = stat.relativeNodeModulesPath || file.substring(srcWithEndSlash.length)
   if (path.sep === "\\") {
     if (relative.startsWith("\\")) {
       // windows problem: double backslash, the above substring call removes root path with a single slash, so here can me some leftovers
@@ -54,7 +43,7 @@ export function createFilter(src: string, patterns: Array<Minimatch>, excludePat
       return true
     }
 
-    let relative = getRelativePath(file, srcWithEndSlash)
+    let relative = getRelativePath(file, srcWithEndSlash, stat)
 
     // filter the root node_modules, but not a subnode_modules (like /appDir/others/foo/node_modules/blah)
     if (relative === "node_modules") {
@@ -69,7 +58,7 @@ export function createFilter(src: string, patterns: Array<Minimatch>, excludePat
 }
 
 // https://github.com/joshwnj/minimatch-all/blob/master/index.js
-function minimatchAll(path: string, patterns: Array<Minimatch>, stat: Stats): boolean {
+function minimatchAll(path: string, patterns: Array<Minimatch>, stat: FilterStats): boolean {
   let match = false
   for (const pattern of patterns) {
     // If we've got a match, only re-test for exclusions.

packages/builder-util/src/fs.ts

@@ -19,7 +19,15 @@ export class CopyFileTransformer {
 }
 
 export type FileTransformer = (file: string) => Promise<null | string | Buffer | CopyFileTransformer> | null | string | Buffer | CopyFileTransformer
-export type Filter = (file: string, stat: Stats) => boolean
+export interface FilterStats extends Stats {
+  // relative path of the dependency(node_modules + moduleName + file)
+  // Mainly used for filter, such as files filtering and asarUnpack filtering
+  relativeNodeModulesPath?: string
+  // deal with asar unpack sysmlink
+  relativeLink?: string
+  linkRelativeToFile?: string
+}
+export type Filter = (file: string, stat: FilterStats) => boolean
 
 export function unlinkIfExists(file: string) {
   return unlink(file).catch(() => {

test/fixtures/test-app-yarn-several-workspace/packages/foo/package.json

@@ -4,7 +4,6 @@
   "main": "index.js",
   "license": "MIT",
   "dependencies": {
-    "ms": "2.0.0",
-    "electron-log": "2.2.8"
+    "ms": "2.0.0"
   }
 }

test/fixtures/test-app-yarn-several-workspace/packages/test-app/package.json

@@ -20,6 +20,7 @@
   },
   "dependencies": {
     "ms": "2.0.0",
-    "electron-log": "2.2.9"
+    "electron-log": "2.2.9",
+    "foo": "*"
   }
 }

test/snapshots/HoistedNodeModuleTest.js.snap

@@ -83,11 +83,11 @@ exports[`yarn several workspaces 2`] = `
 Object {
   "files": Object {
     "index.html": Object {
-      "offset": "19060",
+      "offset": "19187",
       "size": 841,
     },
     "index.js": Object {
-      "offset": "19901",
+      "offset": "20028",
       "size": 2501,
     },
     "node_modules": Object {
@@ -160,27 +160,156 @@ Object {
             },
           },
         },
+        "foo": Object {
+          "files": Object {
+            "package.json": Object {
+              "offset": "14750",
+              "size": 127,
+            },
+          },
+        },
         "ms": Object {
           "files": Object {
             "index.js": Object {
+              "offset": "14877",
+              "size": 2764,
+            },
+            "license.md": Object {
+              "offset": "17641",
+              "size": 1077,
+            },
+            "package.json": Object {
+              "offset": "18718",
+              "size": 469,
+            },
+          },
+        },
+      },
+    },
+    "package.json": Object {
+      "offset": "22529",
+      "size": 326,
+    },
+  },
+}
+`;
+
+exports[`yarn several workspaces and asarUnpack 1`] = `
+Object {
+  "linux": Array [],
+}
+`;
+
+exports[`yarn several workspaces and asarUnpack 2`] = `
+Object {
+  "files": Object {
+    "index.html": Object {
+      "offset": "14877",
+      "size": 841,
+    },
+    "index.js": Object {
+      "offset": "15718",
+      "size": 2501,
+    },
+    "node_modules": Object {
+      "files": Object {
+        "electron-log": Object {
+          "files": Object {
+            "LICENSE": Object {
+              "offset": "0",
+              "size": 1082,
+            },
+            "index.js": Object {
+              "offset": "1082",
+              "size": 140,
+            },
+            "lib": Object {
+              "files": Object {
+                "format.js": Object {
+                  "offset": "1222",
+                  "size": 1021,
+                },
+                "log.js": Object {
+                  "offset": "2243",
+                  "size": 877,
+                },
+                "transports": Object {
+                  "files": Object {
+                    "console.js": Object {
+                      "offset": "3120",
+                      "size": 343,
+                    },
+                    "file": Object {
+                      "files": Object {
+                        "find-log-path.js": Object {
+                          "offset": "3463",
+                          "size": 2078,
+                        },
+                        "get-app-name.js": Object {
+                          "offset": "5541",
+                          "size": 1780,
+                        },
+                        "index.js": Object {
+                          "offset": "7321",
+                          "size": 2068,
+                        },
+                      },
+                    },
+                    "log-s.js": Object {
+                      "offset": "9389",
+                      "size": 1751,
+                    },
+                    "renderer-console.js": Object {
+                      "offset": "11140",
+                      "size": 544,
+                    },
+                  },
+                },
+              },
+            },
+            "main.js": Object {
+              "offset": "11684",
+              "size": 1343,
+            },
+            "package.json": Object {
+              "offset": "13027",
+              "size": 745,
+            },
+            "renderer.js": Object {
+              "offset": "13772",
+              "size": 978,
+            },
+          },
+        },
+        "foo": Object {
+          "files": Object {
+            "package.json": Object {
               "offset": "14750",
+              "size": 127,
+            },
+          },
+        },
+        "ms": Object {
+          "files": Object {
+            "index.js": Object {
               "size": 2764,
+              "unpacked": true,
             },
             "license.md": Object {
-              "offset": "17514",
               "size": 1077,
+              "unpacked": true,
             },
             "package.json": Object {
-              "offset": "18591",
               "size": 469,
+              "unpacked": true,
             },
           },
         },
       },
     },
     "package.json": Object {
-      "offset": "22402",
-      "size": 310,
+      "offset": "18219",
+      "size": 326,
     },
   },
 }

test/src/HoistedNodeModuleTest.ts

@@ -40,6 +40,22 @@ test.ifAll("yarn several workspaces", () =>
   )
 )
 
+test.ifAll("yarn several workspaces and asarUnpack", () =>
+  assertPack(
+    "test-app-yarn-several-workspace",
+    {
+      targets: linuxDirTarget,
+      projectDir: "packages/test-app",
+      config: {
+        asarUnpack: ["**/node_modules/ms/**/*"],
+      },
+    },
+    {
+      packed: context => verifyAsarFileTree(context.getResources(Platform.LINUX)),
+    }
+  )
+)
+
 test.ifAll("yarn two package.json w/ native module", () =>
   assertPack(
     "test-app-two-native-modules",