fix: support including node_modules in other subdirectories

.changeset/tricky-files-speak.md

@@ -0,0 +1,6 @@
+---
+"app-builder-lib": patch
+"builder-util": patch
+---
+
+support including node_modules in other subdirectories

packages/app-builder-lib/scheme.json

@@ -7142,11 +7142,6 @@
       "description": "Whether to include PDB files.",
       "type": "boolean"
     },
-    "includeSubNodeModules": {
-      "default": false,
-      "description": "Whether to include *all* of the submodules node_modules directories",
-      "type": "boolean"
-    },
     "launchUiVersion": {
       "description": "*libui-based frameworks only* The version of LaunchUI you are packaging for. Applicable for Windows only. Defaults to version suitable for used framework version.",
       "type": [

packages/app-builder-lib/src/configuration.ts

@@ -183,12 +183,6 @@ export interface CommonConfiguration {
   readonly removePackageKeywords?: boolean
 }
 export interface Configuration extends CommonConfiguration, PlatformSpecificBuildOptions, Hooks {
-  /**
-   * Whether to include *all* of the submodules node_modules directories
-   * @default false
-   */
-  includeSubNodeModules?: boolean
-
   /**
    * Whether to use [electron-compile](http://github.com/electron/electron-compile) to compile app. Defaults to `true` if `electron-compile` in the dependencies. And `false` if in the `devDependencies` or doesn't specified.
    */

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

@@ -163,7 +163,19 @@ export function getMainFileMatchers(
     patterns.push("package.json")
   }
 
-  customFirstPatterns.push("!**/node_modules")
+  let insertExculdeNodeModulesIndex = -1
+  for (let i = 0; i < patterns.length; i++) {
+    if (!patterns[i].startsWith("!") && (patterns[i].includes("/node_modules") || patterns[i].includes("node_modules/"))) {
+      insertExculdeNodeModulesIndex = i
+      break
+    }
+  }
+
+  if (insertExculdeNodeModulesIndex !== -1) {
+    patterns.splice(insertExculdeNodeModulesIndex, 0, ...["!**/node_modules/**"])
+  } else {
+    customFirstPatterns.push("!**/node_modules/**")
+  }
 
   // https://github.com/electron-userland/electron-builder/issues/1482
   const relativeBuildResourceDir = path.relative(matcher.from, buildResourceDir)

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

@@ -4,8 +4,6 @@ import { FileMatcher } from "../fileMatcher"
 import { Packager } from "../packager"
 import * as path from "path"
 
-const nodeModulesSystemDependentSuffix = `${path.sep}node_modules`
-
 function addAllPatternIfNeed(matcher: FileMatcher) {
   if (!matcher.isSpecifiedAsEmptyArray && (matcher.isEmpty() || matcher.containsOnlyIgnore())) {
     matcher.prependPattern("**/*")
@@ -55,22 +53,7 @@ function createAppFilter(matcher: FileMatcher, packager: Packager): Filter | nul
   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 !!packager.config.includeSubNodeModules
-    }
-    return filter(file, fileStat)
-  }
+  return matcher.createFilter()
 }
 
 /** @internal */
@@ -85,18 +68,8 @@ export class AppFileWalker extends FileCopyHelper implements FileConsumer {
   // eslint-disable-next-line @typescript-eslint/no-unused-vars
   consume(file: string, fileStat: Stats, parent: string, siblingNames: Array<string>): any {
     if (fileStat.isDirectory()) {
-      // https://github.com/electron-userland/electron-builder/issues/1539
-      // but do not filter if we inside node_modules dir
-      // update: solution disabled, node module resolver should support such setup
-      if (file.endsWith(nodeModulesSystemDependentSuffix)) {
-        if (!this.packager.config.includeSubNodeModules) {
-          const matchesFilter = this.matcherFilter(file, fileStat)
-          if (!matchesFilter) {
-            // Skip the file
-            return false
-          }
-        }
-      }
+      const matchesFilter = this.matcherFilter(file, fileStat)
+      return !matchesFilter
     } else {
       // save memory - no need to store stat for directory
       this.metadata.set(file, fileStat)

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

@@ -54,7 +54,15 @@ export function createFilter(src: string, patterns: Array<Minimatch>, excludePat
       return true
     }
 
-    const relative = getRelativePath(file, srcWithEndSlash)
+    let relative = getRelativePath(file, srcWithEndSlash)
+
+    // filter the root node_modules, but not a subnode_modules (like /appDir/others/foo/node_modules/blah)
+    if (relative === "node_modules") {
+      return false
+    } else if (relative.endsWith("/node_modules")) {
+      relative += "/"
+    }
+
     // https://github.com/electron-userland/electron-builder/issues/867
     return minimatchAll(relative, patterns, stat) && (excludePatterns == null || stat.isDirectory() || !minimatchAll(relative, excludePatterns, stat))
   }

packages/builder-util/src/fs.ts

@@ -88,9 +88,9 @@ export async function walk(initialDirPath: string, filter?: Filter | null, consu
           }
 
           const consumerResult = consumer == null ? null : consumer.consume(filePath, stat, dirPath, childNames)
-          if (consumerResult === false) {
+          if (consumerResult === true) {
             return null
-          } else if (consumerResult == null || !("then" in consumerResult)) {
+          } else if (consumerResult === false || consumerResult == null || !("then" in consumerResult)) {
             if (stat.isDirectory()) {
               dirs.push(name)
               return null

test/snapshots/ignoreTest.js.snap

@@ -6,18 +6,6 @@ Object {
 }
 `;
 
-exports[`copied all submodule node_modules 1`] = `
-Object {
-  "linux": Array [],
-}
-`;
-
-exports[`copied no submodule node_modules 1`] = `
-Object {
-  "linux": Array [],
-}
-`;
-
 exports[`copied select submodule node_modules 1`] = `
 Object {
   "linux": Array [],

test/src/ignoreTest.ts

@@ -83,131 +83,200 @@ test.ifNotCiMac(
       targets: Platform.LINUX.createTarget(DIR_TARGET),
       config: {
         asar: false,
+        files: ["**/*", "**/submodule-1-test/node_modules/**"],
       },
     },
     {
+      isInstallDepsBefore: true,
       projectDirCreated: projectDir => {
         return Promise.all([
           modifyPackageJson(projectDir, data => {
             data.devDependencies = {
-              "@electron/osx-sign": "*",
+              "semver": "6.3.1",
               ...data.devDependencies,
             }
           }),
-          outputFile(path.join(projectDir, "node_modules", "@electron/osx-sign", "package.json"), "{}"),
         ])
       },
       packed: context => {
         return Promise.all([
-          assertThat(path.join(context.getResources(Platform.LINUX), "app", "node_modules", "@electron/osx-sign")).doesNotExist(),
-          assertThat(path.join(context.getResources(Platform.LINUX), "app", "ignoreMe")).doesNotExist(),
+          assertThat(path.join(context.getResources(Platform.LINUX), "app", "node_modules", "semver")).doesNotExist(),
         ])
       },
     }
   )
 )
 
 test.ifDevOrLinuxCi(
-  "copied no submodule node_modules",
+  "copied sub node_modules of the rootDir/node_modules",
   app(
     {
       targets: Platform.LINUX.createTarget(DIR_TARGET),
       config: {
         asar: false,
-        includeSubNodeModules: false,
+        files: ["**/*", "**/submodule-1-test/node_modules/**"],
       },
     },
     {
+      isInstallDepsBefore: true,
       projectDirCreated: projectDir => {
         return Promise.all([
           modifyPackageJson(projectDir, data => {
             data.dependencies = {
-              "submodule-1-test": "*",
-              "submodule-2-test": "*",
+              "electron-updater": "6.3.9",
+              "semver":"6.3.1",
               ...data.dependencies,
             }
           }),
-          outputFile(path.join(projectDir, "node_modules", "submodule-1-test", "node_modules", "package.json"), "{}"),
-          outputFile(path.join(projectDir, "node_modules", "submodule-2-test", "node_modules", "package.json"), "{}"),
+          outputFile(path.join(projectDir, "submodule-1-test", "node_modules", "package.json"), "{}"),
+          outputFile(path.join(projectDir, "others", "node_modules", "package.json"), "{}"),
         ])
       },
       packed: context => {
         return Promise.all([
-          assertThat(path.join(context.getResources(Platform.LINUX, archFromString(process.arch)), "app", "node_modules", "submodule-1-test", "node_modules")).doesNotExist(),
-          assertThat(path.join(context.getResources(Platform.LINUX, archFromString(process.arch)), "app", "node_modules", "submodule-2-test", "node_modules")).doesNotExist(),
+          assertThat(path.join(context.getResources(Platform.LINUX, archFromString(process.arch)), "app", "node_modules", "electron-updater", "node_modules")).isDirectory(),
+          assertThat(path.join(context.getResources(Platform.LINUX, archFromString(process.arch)), "app", "others", "node_modules")).doesNotExist(),
+          assertThat(path.join(context.getResources(Platform.LINUX, archFromString(process.arch)), "app", "submodule-1-test", "node_modules")).isDirectory(),
+          assertThat(
+            path.join(context.getResources(Platform.LINUX, archFromString(process.arch)), "app", "submodule-1-test", "node_modules", "package.json")
+          ).isFile(),
         ])
       },
     }
   )
 )
 
 test.ifDevOrLinuxCi(
-  "copied all submodule node_modules",
+  "Don't copy sub node_modules of the other dir instead of rootDir",
   app(
     {
       targets: Platform.LINUX.createTarget(DIR_TARGET),
       config: {
         asar: false,
-        includeSubNodeModules: true,
       },
     },
     {
       projectDirCreated: projectDir => {
         return Promise.all([
           modifyPackageJson(projectDir, data => {
             data.dependencies = {
-              "submodule-1-test": "*",
-              "submodule-2-test": "*",
               ...data.dependencies,
             }
           }),
-          outputFile(path.join(projectDir, "node_modules", "submodule-1-test", "node_modules", "package.json"), "{}"),
-          outputFile(path.join(projectDir, "node_modules", "submodule-2-test", "node_modules", "package.json"), "{}"),
+          outputFile(path.join(projectDir, "others", "node_modules", "package.json"), "{}"),
+          outputFile(path.join(projectDir, "others", "test1", "package.json"), "{}"),
+          outputFile(path.join(projectDir, "others", "submodule-2-test", "node_modules", "package.json"), "{}"),
+          outputFile(path.join(projectDir, "others", "submodule-2-test", "test2", "package.json"), "{}"),
         ])
       },
       packed: context => {
         return Promise.all([
-          assertThat(path.join(context.getResources(Platform.LINUX, archFromString(process.arch)), "app", "node_modules", "submodule-1-test", "node_modules")).isDirectory(),
-          assertThat(path.join(context.getResources(Platform.LINUX, archFromString(process.arch)), "app", "node_modules", "submodule-2-test", "node_modules")).isDirectory(),
+          assertThat(path.join(context.getResources(Platform.LINUX, archFromString(process.arch)), "app", "others", "node_modules")).doesNotExist(),
+          assertThat(path.join(context.getResources(Platform.LINUX, archFromString(process.arch)), "app", "others", "test1")).isDirectory(),
+          assertThat(path.join(context.getResources(Platform.LINUX, archFromString(process.arch)), "app", "others", "test1", "package.json")).isFile(),
+          assertThat(path.join(context.getResources(Platform.LINUX, archFromString(process.arch)), "app", "others", "submodule-2-test", "node_modules")).doesNotExist(),
+          assertThat(path.join(context.getResources(Platform.LINUX, archFromString(process.arch)), "app", "others", "submodule-2-test", "test2")).isDirectory(),
+          assertThat(path.join(context.getResources(Platform.LINUX, archFromString(process.arch)), "app", "others", "submodule-2-test", "test2", "package.json")).isFile(),
         ])
       },
     }
   )
 )
 
-test.skip.ifDevOrLinuxCi(
+test.ifDevOrLinuxCi(
   "copied select submodule node_modules",
   app(
     {
       targets: Platform.LINUX.createTarget(DIR_TARGET),
       config: {
         asar: false,
-        files: ["**/*", "*/submodule-1-test/node_modules/**"],
+        // should use **/ instead of */, 
+        // we use the related path to match, so the relative path is submodule-1-test/node_modules
+        // */ will not match submodule-1-test/node_modules 
+        files: ["**/*", "**/submodule-1-test/node_modules/**"],
       },
     },
     {
       projectDirCreated: projectDir => {
         return Promise.all([
           modifyPackageJson(projectDir, data => {
             data.dependencies = {
-              "submodule-1-test": "*",
-              "submodule-2-test": "*",
               ...data.dependencies,
             }
           }),
-          outputFile(path.join(projectDir, "node_modules", "submodule-1-test", "node_modules", "package.json"), "{}"),
-          outputFile(path.join(projectDir, "node_modules", "submodule-2-test", "node_modules", "package.json"), "{}"),
+          outputFile(path.join(projectDir, "submodule-1-test", "node_modules", "package.json"), "{}"),
+          outputFile(path.join(projectDir, "submodule-2-test", "node_modules", "package.json"), "{}"),
         ])
       },
       packed: context => {
         return Promise.all([
-          assertThat(path.join(context.getResources(Platform.LINUX, archFromString(process.arch)), "app", "node_modules", "submodule-1-test", "node_modules")).isDirectory(),
+          assertThat(path.join(context.getResources(Platform.LINUX, archFromString(process.arch)), "app", "submodule-1-test", "node_modules")).isDirectory(),
           assertThat(
-            path.join(context.getResources(Platform.LINUX, archFromString(process.arch)), "app", "node_modules", "submodule-1-test", "node_modules", "package.json")
+            path.join(context.getResources(Platform.LINUX, archFromString(process.arch)), "app", "submodule-1-test", "node_modules", "package.json")
           ).isFile(),
-          assertThat(path.join(context.getResources(Platform.LINUX, archFromString(process.arch)), "app", "node_modules", "submodule-2-test", "node_modules")).doesNotExist(),
+          assertThat(path.join(context.getResources(Platform.LINUX, archFromString(process.arch)), "app", "submodule-2-test", "node_modules")).doesNotExist(),
+        ])
+      },
+    }
+  )
+)
+
+test.ifDevOrLinuxCi(
+  "cannot copied select submodule node_modules by */",
+  app(
+    {
+      targets: Platform.LINUX.createTarget(DIR_TARGET),
+      config: {
+        asar: false,
+        files: ["**/*", "*/submodule-1-test/node_modules/**"],
+      },
+    },
+    {
+      projectDirCreated: projectDir => {
+        return Promise.all([
+          modifyPackageJson(projectDir, data => {
+            data.dependencies = {
+              ...data.dependencies,
+            }
+          }),
+          outputFile(path.join(projectDir, "submodule-1-test", "node_modules", "package.json"), "{}"),
+        ])
+      },
+      packed: context => {
+        return Promise.all([
+          assertThat(path.join(context.getResources(Platform.LINUX, archFromString(process.arch)), "app", "submodule-1-test", "node_modules")).doesNotExist(),
         ])
       },
     }
   )
 )
+
+test.ifDevOrLinuxCi(
+  "cannot copied select submodule node_modules by **/submodule-1-test/node_modules",
+  app(
+    {
+      targets: Platform.LINUX.createTarget(DIR_TARGET),
+      config: {
+        asar: false,
+        files: ["**/*", "**/submodule-1-test/node_modules"],
+      },
+    },
+    {
+      projectDirCreated: projectDir => {
+        return Promise.all([
+          modifyPackageJson(projectDir, data => {
+            data.dependencies = {
+              ...data.dependencies,
+            }
+          }),
+          outputFile(path.join(projectDir, "submodule-1-test", "node_modules", "package.json"), "{}"),
+        ])
+      },
+      packed: context => {
+        return Promise.all([
+          assertThat(path.join(context.getResources(Platform.LINUX, archFromString(process.arch)), "app", "submodule-1-test", "node_modules")).doesNotExist(),
+        ])
+      },
+    }
+  )
+)