electron-userland / electron-builder

A complete solution to package and build a ready for distribution Electron app with “auto update” support out of the box
https://www.electron.build
MIT License
13.62k stars 1.74k forks source link

app.asar size is huge with electron-builder 22.11.8 #6045

Closed MaartenBent closed 3 years ago

MaartenBent commented 3 years ago

I use electron-builder --dir to create a package. After changing electron-builder in devDependencies from latest to next, the size of my app.asar is huge. With latest/22.11.7 it is around 35MB. With next/22.11.8 it is around 70MB, and there is an additional directory app.asar.unpacked of 311MB (it contains folders 7zip-bin, app-builder-bin and electron.

When setting asar to false, the node_modules inside resources/app changes from 20MB to 342MB.

Could it be all devDependencies are packaged as well?

mmaietta commented 3 years ago

That sounds like it may be related to this PR: https://github.com/electron-userland/electron-builder/pull/5911 Can you try explicitly setting includeSubNodeModules: false? Curious where the bug is here.

Also could you provide your electron-builder config and your dependencies list? If you're able to provide a test repo, such as using electron-quickstart (https://github.com/electron/electron-quick-start-typescript), that would help immensely in tracking this bug.

MaartenBent commented 3 years ago

Adding includeSubNodeModules: false does not have any effect.

I couldn't reproduce it with electron-quickstart combined with electron-builder, so I stripped my package.json until the problem would go away. It seems to be caused by a file pattern I added. If I extend the default pattern with two extra lines (2nd and 3th) it happens.

      "**/*",
      "!**/node_modules/**/{demo,build,deps,doc,docs,samples,benchmark,scss}/**/*",
      "**/node_modules/**/build/Release/*.node",
      "!**/node_modules/*/{CHANGELOG.md,README.md,README,readme.md,readme}",
      ...

Note: the 3th line causes it. It also happens when using a direct path, e.g. "node_modules/better-sqlite3/build/Release/better_sqlite3.node".

My goal is to exclude some subdirectories from all the modules, but keep the *.node files. It works with 22.11.7 so I would think these patterns are OK.

When I add these patterns to electron-quickstart I can reproduce the problem.

package.json for electron-quick-start ```json { "name": "electron-quick-start", "version": "1.0.0", "description": "A minimal Electron application", "main": "main.js", "scripts": { "start": "electron .", "pack": "electron-builder --dir", "dist": "electron-builder" }, "repository": "https://github.com/electron/electron-quick-start", "keywords": [ "Electron", "quick", "start", "tutorial", "demo" ], "author": "GitHub", "license": "CC0-1.0", "devDependencies": { "electron": "^13.1.6", "electron-builder": "next" }, "build": { "asar": true, "appId": "my.app.id", "files": [ "**/*", "!**/node_modules/**/{demo,build,deps,doc,docs,samples,benchmark,scss}/**/*", "**/node_modules/**/build/Release/*.node", "!**/node_modules/*/{CHANGELOG.md,README.md,README,readme.md,readme}", "!**/node_modules/*/{test,__tests__,tests,powered-test,example,examples}", "!**/node_modules/*.d.ts", "!**/node_modules/.bin", "!**/*.{iml,o,hprof,orig,pyc,pyo,rbc,swp,csproj,sln,xproj}", "!.editorconfig", "!**/._*", "!**/{.DS_Store,.git,.hg,.svn,CVS,RCS,SCCS,.gitignore,.gitattributes}", "!**/{__pycache__,thumbs.db,.flowconfig,.idea,.vs,.nyc_output}", "!**/{appveyor.yml,.travis.yml,circle.yml}", "!**/{npm-debug.log,yarn.lock,.yarn-integrity,.yarn-metadata.json}" ] } } ```
mmaietta commented 3 years ago

Hmmm, can you try just using this?

"files": [
      "**/*",
      "**/node_modules/**/build/Release/*.node",
      "!**/node_modules/**/{demo,build,deps,doc,docs,samples,benchmark,scss}/**/*"
]

I'm wondering if the negation regexes are being assumed to be in a specific order. Also, I think the default glob pattern is always applied so you shouldn't need to specify/extend them. The documentation states: https://www.electron.build/configuration/contents.html#files

Default pattern */ is not added to your custom if some of your patterns is not ignore (i.e. not starts with !). package.json and /node_modules//* (only production dependencies will be copied) is added to your custom in any case. All default ignores are added in any case — you don’t need to repeat it if you configure own patterns.

MaartenBent commented 3 years ago

Hmmm, can you try just using this?

This doesn't work. The *.node files are not included anymore since the order matters, see this description. And all the other extra files are still there.

It seems as soon as there is a not-ignore pattern containing node_modules, all the files are being packaged. For example, adding "node_modules/test.txt" anywhere in files triggers the problem.

The only work-around I found so far is not using "**/node_modules/**/build/Release/*.node" and use this instead:

      {
        "from": "node_modules/better-sqlite3/build/Release",
        "to": "node_modules/better-sqlite3/build/Release",
        "filter": "*.node"
      },

Also, I think the default glob pattern is always applied so you shouldn't need to specify/extend them

Looks like it is. I can simplify the test case with electron-quick-start to just

    "files": [
      "**/*",
      "node_modules/test.txt"
    ]
bedney commented 3 years ago

@mmaietta -

Let me know if I can help out here.

The default setting for includeSubNodeModules is false, so specifying it as such would have no effect.

And, yes, order matters on globbing patterns - instead of doing some sort of specificity sorting :-(.

Cheers,

mmaietta commented 3 years ago

I'm trying to expand our current unit tests to cover and resolve this. Currently, I've added "node_modules/test.txt" to the corresponding ignoreTest.ts tests. If you're willing to give it a shot, here's the patch I'm running with for unit tests. Then I'm also using it with electron-quick-start repo with the config that @MaartenBent provided (thank you!).

patch.diff
diff --git a/test/src/ignoreTest.ts b/test/src/ignoreTest.ts
index 5452b3eb..3e7b40c0 100644
--- a/test/src/ignoreTest.ts
+++ b/test/src/ignoreTest.ts
@@ -114,7 +114,8 @@ test.ifDevOrLinuxCi(
       targets: Platform.LINUX.createTarget(DIR_TARGET),
       config: {
         asar: false,
-        includeSubNodeModules: false  //  defaults to false too
+        includeSubNodeModules: false,
+        files: ["**/*", "node_modules/test.txt"],
       },
     },
     {
@@ -127,12 +128,14 @@ test.ifDevOrLinuxCi(
               ...data.dependencies,
             }
           }),
+          outputFile(path.join(projectDir, "node_modules", "test.txt"), "data"),
           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"), "{}"),
         ])
       },
       packed: context => {
         return Promise.all([
+          assertThat(path.join(context.getResources(Platform.LINUX, archFromString(process.arch)), "app", "node_modules", "test.txt")).isFile(),
           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(),
         ])
@@ -148,7 +151,8 @@ test.ifDevOrLinuxCi(
       targets: Platform.LINUX.createTarget(DIR_TARGET),
       config: {
         asar: false,
-        includeSubNodeModules: true
+        includeSubNodeModules: true,
+        files: ["**/*", "node_modules/test.txt"]
       },
     },
     {
@@ -161,12 +165,14 @@ test.ifDevOrLinuxCi(
               ...data.dependencies,
             }
           }),
+          outputFile(path.join(projectDir, "node_modules", "test.txt"), "data"),
           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"), "{}"),
         ])
       },
       packed: context => {
         return Promise.all([
+          assertThat(path.join(context.getResources(Platform.LINUX, archFromString(process.arch)), "app", "node_modules", "test.txt")).isFile(),
           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(),
         ])
@@ -182,10 +188,7 @@ test.ifDevOrLinuxCi(
       targets: Platform.LINUX.createTarget(DIR_TARGET),
       config: {
         asar: false,
-        files: [
-          "**/*",
-          "*/submodule-1-test/node_modules/**",
-        ],
+        files: ["**/*", "**/submodule-1-test/node_modules/**"],
       },
     },
     {
@@ -198,14 +201,18 @@ test.ifDevOrLinuxCi(
               ...data.dependencies,
             }
           }),
+          outputFile(path.join(projectDir, "node_modules", "test.txt"), "data"),
           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"), "{}"),
         ])
       },
       packed: context => {
         return Promise.all([
+          assertThat(path.join(context.getResources(Platform.LINUX, archFromString(process.arch)), "app", "node_modules", "test.txt")).doesNotExist(),
           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-1-test", "node_modules", "package.json")).isFile(),
+          assertThat(
+            path.join(context.getResources(Platform.LINUX, archFromString(process.arch)), "app", "node_modules", "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(),
         ])
       },

Related: I use an alias for syncing my electron builder output to my local project instead of yalc. Seems to provide more consistent results

alias syncEB1="rsync -upaRv --include='*.js' --exclude='*.d.ts' --exclude='*.map'  <full_path_to>/electron-builder/packages/./*/out node_modules/"
bedney commented 3 years ago

Firstly, I apologize for not being able to look at this more closely. Thank you @mmaietta for your patch!

So, right, obviously the file walker will not know which node modules came from dependencies vs. devDependencies.

@mmaietta - do you think it's worth trying to add code that would read package.json and compare the first segment of what comes after node_modules to the dependencies and devDependencies sections and try to filter out the devDependencies ones?

mmaietta commented 3 years ago

do you think it's worth trying to add code that would read package.json and compare the first segment of what comes after node_modules to the dependencies and devDependencies sections and try to filter out the devDependencies ones?

I think it's that app-builder-bin automatically does that already

That being said, this is probably the file we'd need to make the changes to. https://github.com/electron-userland/electron-builder/blob/e101e8d190d7e3046222e88c32a62d727dadd808/packages/app-builder-lib/src/util/NodeModuleCopyHelper.ts#L10-L14 https://github.com/electron-userland/electron-builder/blob/e101e8d190d7e3046222e88c32a62d727dadd808/packages/app-builder-lib/src/util/NodeModuleCopyHelper.ts#L36-L41

mmaietta commented 3 years ago

@MaartenBent 22.11.10 has been published https://github.com/electron-userland/electron-builder/releases/tag/22.11.10

MaartenBent commented 3 years ago

Thanks. I can confirm the problem is fixed with this version.