fix: packages in the workspace not being under node_modules (electron-userland#8576)

fix develar/app-builder#141

reproduced error info
```
  ● yarn several workspaces

    /tmp/et-db411cd21f3bbd882f3a5a30c58db6ff/t-kIKFAh/test-project-2/packages/foo/package.json must be under /tmp/et-db411cd21f3bbd882f3a5a30c58db6ff/t-kIKFAh/test-project-2/packages/test-app/

      30 |     const index = file.indexOf(NODE_MODULES_PATTERN)
      31 |     if (index < 0) {
    > 32 |       throw new Error(`${file} must be under ${srcWithEndSlash}`)
         |             ^
      33 |     } else {
      34 |       return file.substring(index + 1 /* leading slash */)
      35 |     }

      at getRelativePath (../packages/app-builder-lib/src/util/filter.ts:32:13)
      at AsarPackager.unpackPattern (../packages/app-builder-lib/src/util/filter.ts:57:20)
      at AsarPackager.createPackageFromFiles (../packages/app-builder-lib/src/asar/asarUtil.ts:129:82)
      at AsarPackager.pack (../packages/app-builder-lib/src/asar/asarUtil.ts:49:41)
      at ../packages/app-builder-lib/src/platformPackager.ts:439:11
          at async Promise.all (index 0)
      at AsyncTaskManager.awaitTasks (../packages/builder-util/src/asyncTaskManager.ts:65:25)
      at LinuxPackager.doPack (../packages/app-builder-lib/src/platformPackager.ts:293:5)
      at LinuxPackager.pack (../packages/app-builder-lib/src/platformPackager.ts:138:5)
      at Packager.doBuild (../packages/app-builder-lib/src/packager.ts:459:9)
      at executeFinally (../packages/builder-util/src/promise.ts:12:14)
      at Packager.build (../packages/app-builder-lib/src/packager.ts:393:31)
      at packAndCheck (src/helpers/packTester.ts:188:41)
      at src/helpers/packTester.ts:132:36
      at executeFinally (../packages/builder-util/src/promise.ts:12:14)
```

Currently, the returned node_modules paths are no longer symlinks, but
resolved paths. For example, packages in pnpm workspace/yarn workspace
are not under node_modules.

All previous getRealSource/getRelativePath methods were based on
node_modules in the path, which was relatively tricky.


https://github.com/electron-userland/electron-builder/blob/a25d04d5a8e58b447f0462673a4362414da9ed27/packages/app-builder-lib/src/util/appFileCopier.ts#L191-L203


https://github.com/electron-userland/electron-builder/blob/a25d04d5a8e58b447f0462673a4362414da9ed27/packages/app-builder-lib/src/util/filter.ts#L28-L36




Solution: Directly include relativeNodeModulesPath in the file stats.
This is simple and easy to understand.

---------

Co-authored-by: Mike Maietta <[email protected]>

Author: beyondkmp

Diff for: .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

Diff for: 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)
     }

Diff for: 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"
@@ -112,7 +112,8 @@ export class NodeModuleCopyHelper extends FileCopyHelper {
             }
           }
 
-          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
             }

Diff for: 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

Diff for: 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.

Diff for: 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(() => {

Diff for: 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"
   }
 }

Diff for: 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": "*"
   }
 }

Diff for: 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,
     },
   },
 }

Diff for: 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",