Nested node_modules disappears after building

[bradydowling]

• Version: 20.26.0

• Target: macOS (.dmg)

Similar to #1539, I'm working with an app where I've cloned another git repo int and I have several node_modules folders. When I build, the node_modules folder in the nested git repo disappears. This happens whether asar is enabled or disabled. It also happens regardless of whether or not the inner git repo is officially designated as a git submodule or if I've just manually cloned it in.

Here's my folder structure:

main.js (starts app and forks process from the express app's index.js)
node_modules
package.json
cloned-express-app
   |
    --node_modules
      index.js
      package.json

Here's a Stack Overflow issue I just created that may explain it better (I didn't want to copy/paste but let me know if that's preferred) and here's the repo I'm working with.

It's also worth noting that I've tried building this for Windows and my friend said it works fine on there. I haven't tested that recently though so I can't give much info on that immediately. I also haven't inspected the Resources contents of the packaged app (or whatever the .exe counterpart would be) so I can't provide info on that but I do know that when I look in the mac app's Resources directory there is no node_modules folder within the nested git repo.

[fpsqdb]

Same issue here for me.

• version: 20.26.0

• target: windows

 "files": [
            "!**/**.map",
            "!**/**.ts",
            "!node_modules",
            "package.json",
            "node_modules/semver/**",
            "node_modules/fs-extra/**",
            "node_modules/graceful-fs/**",
            "node_modules/jsonfile/**",
            "node_modules/universalify/**"
        ],

When I build, there is no node_modules folder in the package. That's not my expected, it should have those folders and files in the package.

node_modules/semver,
node_modules/fs-extra,
node_modules/graceful-fs,
node_modules/jsonfile,
node_modules/universalify

But if I downgrade to 20.15.0, it works fine.

[bradydowling]

Interesting, I'll need to give this a try, thanks for the note.

[bradydowling]

It did indeed package node_modules with this old version (20.15.0) on macOS as well, thank you @fpsqdb.

[hn3000]

In version 20.16 the way node_modules includes work was changed: explicit includes are now ignored, instead all production dependencies are used. Unfortunately, excludes are still allowed to affect the node_modules folders and that means your !node_modules takes effect.

Looking at your example above:

    "files": [
        ...,
        "!node_modules",
        ...,
        "node_modules/semver/**",
        "node_modules/fs-extra/**",
        "node_modules/graceful-fs/**",
        "node_modules/jsonfile/**",
        "node_modules/universalify/**",
        ...
    ],

The first line will prevent inclusion of anything in the node_modules folder, the other lines are ignored since version 20.16, resulting in an app.asar that does not include any node_modules.

Remove the exclusion for node_modules and newer versions than 20.15 should work fine. In fact, you should be able to remove all lines mentioning the node_modules folder.

(Edit: tried to explain in more detail.)

[fpsqdb]

@hn3000 There's a condition that when use webpack to bundle packages, so you don't need all the dependencies. And if there are some packages exclude from webpack(such as set externals in webpack config) we should add them individually, that's my situations.

[hn3000]

Then maybe a second package.json can help? https://www.electron.build/tutorials/two-package-structure

[TanninOne]

And a change like this was made in a minor update (20.15 -> 20.16)? Intentionally?

[TanninOne]

Am I missing something here? How is this bug still not addressed?
Afaict all releases post 20.15.0 are unusable and should be considered extremely dangerous, how is this not top priority?
@develar ? I don't want to be annoying, but I've not been able to produce a working build with any electron-builder after 20.15.0 and I'm starting to panic as you don't even seem to acknowledge there is a problem here.
And no, hn3000s explanation does not fully explain the problem, I get this error even entirely without any files excludes.

My project depends on semver@^5.6.0
A dependency I include (semvish) depends on semver@^4.6.3
electron-builder packages semver@^4.6.3 twice, in semvish/node_modules and the top-level node_modules, causing bugs in my main application, breaking electron-updater among other things.
And I'll just assume this is one problem of many caused by this.

[grantcv1]

My application brings together 9 independent submodules (which have a life of their own in in other apps) and each has their own node_module. I've been using a much earlier version of electron-builder (19.55.3) for over a year without any issues -- all the node_modules were copied into my installer. After upgrading to the latest version, I've lost 3 days of development trying to understand this issue. It really need to be address. Seconding what TanninOne said, without fixing this issue, electron-builder is unusable.

[theproductiveprogrammer]

Is there any update regarding this issue? Or any workaround? I've spent 2 days fiddling with various setting and nothing is working. I can't proceed to release an installer at this rate. Anyone have any suggestions?

[bradydowling]

Doesn't it work if you fix the version?

…

On Mon, Jun 10, 2019, 11:06 PM Charles ***@***.***> wrote: Is there any update regarding this issue? Or any workaround? I've spent 2 days fiddling with various setting and nothing is working. I can't proceed to release an installer at this rate. Anyone have any suggestions? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#3185?email_source=notifications&email_token=ABS4AU2KOM6CAIW3NILP74TPZ4JCBA5CNFSM4FMURQ42YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXLZX5I#issuecomment-500669429>, or mute the thread </~https://github.com/notifications/unsubscribe-auth/ABS4AU2J2QFT3QYNZSANNKDPZ4JCBANCNFSM4FMURQ4Q> .

[theproductiveprogrammer]

I tried that but when I rolled back to 20.15.0 I got a new error:

[xmldom error]  invalid doc source
@#[line:0,col:undefined]
TypeError: Cannot read property 'documentElement' of undefined
    at parse
(ai-ico/package/elife-node/node_modules/plist/lib/parse.js:68:9)
    at
ai-ico/package/elife-node/node_modules/electron-builder-lib/src/electron/electronMac.ts:51:25
From previous event:
    at createMacApp
(ai-ico/package/elife-node/node_modules/electron-builder-lib/out/electron/electronMac.js:245:17)
    at
ai-ico/package/elife-node/node_modules/electron-builder-lib/src/electron/ElectronFramework.ts:56:11
    at Generator.next (<anonymous>)
From previous event:
    at beforeCopyExtraFiles
(ai-ico/package/elife-node/node_modules/electron-builder-lib/out/electron/ElectronFramework.js:167:17)
    at beforeCopyExtraFiles
(ai-ico/package/elife-node/node_modules/electron-builder-lib/src/electron/ElectronFramework.ts:131:14)
    at
ai-ico/package/elife-node/node_modules/electron-builder-lib/src/platformPackager.ts:223:13
    at Generator.next (<anonymous>)
    at runCallback (timers.js:705:18)
    at tryOnImmediate (timers.js:676:5)
    at processImmediate (timers.js:658:5)
<snip>

So I'm sticking with the latest version. I've edited the electron-builder code for now and removed the node_modules filter but that's probably the wrong thing to do.

Do you know why electron-builder is removing node_modules? I tried looking at the code but the commenting is very sparse and I can't figure it out.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[florisporro]

Has this been fixed? If not, it's definitely something that should be addressed, it's a big headache.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[TanninOne]

Not fixed yet

[stale]

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

[karlsnyder0]

I believe the developer actively ignores this issue. Nothing will happen. This has been open for more than 2 years, yet still no acknowledgement - nothing.

Unfortunately there's nothing else but electron-builder so we have to live with this garbage great tool.

I've updated my comment and included a patch that we will now use to work around this issue.

[stale]

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

[dmnsgn]

Is this still relevant?

Yes

[bayraak]

This issue still continues and no one is taking any action on it.

[tnrich]

@karlsnyder0 if your patch is working and not breaking other stuff maybe you could submit a PR for it anyways to get more attention to a possible fix for this pretty nasty issue? Just a thought.

PS. Thanks for the patch code (I hadn't heard about patch-package, what a sweet npm package)

[tnrich]

@karlsnyder0 I take back what I said. It doesn't appear your patch is working. I had to modify it slightly so it didn't break immediately:

patches/app-builder-lib+22.9.1.patch

diff --git a/node_modules/app-builder-lib/out/util/AppFileWalker.js b/node_modules/app-builder-lib/out/util/AppFileWalker.js
index 0f9cdad..933f43d 100644
--- a/node_modules/app-builder-lib/out/util/AppFileWalker.js
+++ b/node_modules/app-builder-lib/out/util/AppFileWalker.js
@@ -88,9 +88,9 @@ function createAppFilter(matcher, packager) {
 
   const filter = matcher.createFilter();
   return (file, fileStat) => {
-    if (!nodeModulesFilter(file, fileStat)) {
-      return false;
-    }
+    if (!['www/node_modules'].some(v => file.includes(v)) && !nodeModulesFilter(file, fileStat)) {
+       return false;
+     }
 
     return filter(file, fileStat);
   };
@@ -110,7 +110,7 @@ class AppFileWalker extends FileCopyHelper {
       // /~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 (!['www/node_modules'].some(v => file.includes(v)) && file.endsWith(nodeModulesSystemDependentSuffix)) {
         return false;
       }
     } else {

In your version I was getting a str is undefined error so I changed str to file. Even with the modifications, your patch does not seem to fix the issue as I'm still getting the original module not found error:

Uncaught Exception:
Error: Cannot find module 'bio-parsers'
Require stack:
- /Users/tnrich/Sites/ove-electron/dist/mac/Open Vector Editor.app/Contents/Resources/app.asar/main.js

Any help from the maintainers on this issue would be appreciated.

Thanks!

[Julusian]

I hit this issue and was forced to figure something out because of needing to update electron and electron-builder.
My workaround is to unpack the asar, repopulate the nested node_modules and then repack the asar.
/~https://github.com/bitfocus/companion/blob/master/tools/asar-repack.js /~https://github.com/bitfocus/companion/blob/master/package.json#L7

It has some issues, but works well enough for us.

[williamoliveira]

Is this error related?

    "electron-updater": "4.3.5",
    "electron": "11.2.3",
    "electron-builder": "^22.9.1",

(node:10890) UnhandledPromiseRejectionWarning: Error: Cannot find module 'fs-extra'
Require stack:
- /tmp/.mount_myappZZtDD/resources/app.asar/node_modules/electron-updater/out/AppUpdater.js
- /tmp/.mount_myappZZtDD/resources/app.asar/node_modules/electron-updater/out/BaseUpdater.js
- /tmp/.mount_myappZZtDD/resources/app.asar/node_modules/electron-updater/out/AppImageUpdater.js
- /tmp/.mount_myappZZtDD/resources/app.asar/node_modules/electron-updater/out/main.js
- /tmp/.mount_myappZZtDD/resources/app.asar/main.js
- 
    at Module._resolveFilename (internal/modules/cjs/loader.js:972:15)
    at Function.n._resolveFilename (electron/js2c/browser_init.js:249:921)
    at Module._load (internal/modules/cjs/loader.js:848:27)
    at Function.f._load (electron/js2c/asar_bundle.js:5:12738)
    at Module.require (internal/modules/cjs/loader.js:1032:19)
    at require (internal/modules/cjs/helpers.js:72:18)
    at _fsExtra (/tmp/.mount_myappZZtDD/resources/app.asar/node_modules/electron-updater/src/AppUpdater.ts:5:1)
    at AppImageUpdater.loadUpdateConfig (/tmp/.mount_myappZZtDD/resources/app.asar/node_modules/electron-updater/src/AppUpdater.ts:469:27)
    at Lazy.<anonymous> (/tmp/.mount_myappZZtDD/resources/app.asar/node_modules/electron-updater/src/AppUpdater.ts:141:43)
    at Lazy.get value [as value] (/tmp/.mount_myappZZtDD/resources/app.asar/node_modules/lazy-val/src/main.ts:18:25)

[Raynos]

This is still a frustrating issue today.

[somebeaver]

It is well known that the "node_modules" folder cannot be renamed, so it is extremely frustrating that electron-builder makes such an opinionated decision by removing all nested node_modules folders from the app. You would think that there would be some flexibility with this option in particular, but no, everyone is forced to restructure their app or use an outdated version of electron-builder or use ugly patches. I do not understand how this is not a top priority issue.

[bedney]

So I'd be willing to make the patch that @tnrich has here into a PR for @develar. I've done other PRs that @develar has taken and are now in the codebase. Have folks found that patch to work for them? I'm willing to help out here. What do folks think?

[mmaietta]

Does anyone have a repro gist or repo that I could try locally? Something like electron-quick-start

@bedney, happy to review it

[bedney]

So it's been a while since I looked at this issue for my own purposes. I was able to workaround this by setting asar to false and using extraResources to explicitly copy the node_modules directory that I want into the proper place (which is less than ideal). I'm trying to get a handle on the real issue here which is, I think, that electron-builder is filtering out all node_modules directories in included packages (i.e subdirectories). And, furthermore, it is ignoring file pattern matching so that the behavior cannot be overridden.

Is this an accurate characterization of this problem? I'd like to help fix this, but if folks could confirm for me that this is the problem that they're seeing, we can go from there.

Thanks!

[somebeaver]

@bedney

I think, that electron-builder is filtering out all node_modules directories in included packages (i.e subdirectories)

Exactly.

And, furthermore, it is ignoring file pattern matching so that the behavior cannot be overridden.

Also correct. This wouldn't be an issue if we could just add path/to/deep/node_modules/** to our package.json's files.

[somebeaver]

@mmaietta I just put together a minimal example: /~https://github.com/somebeaver/electron-builder-issue-3185

The readme has all the info needed. It's basically just the default create-electron-app but with the contents moved into a nested node_modules, which recreates the issues.

Please let me know if there's any issues with the repo.

[TanninOne]

Is this an accurate characterization of this problem? I'd like to help fix this, but if folks could confirm for me that this is the problem that they're seeing, we can go from there.

It's been a while since I concerned myself with this issue but this wasn't exactly what I experienced.
I got builds where the modules that did get bundled were an older version than what my main project required.
See my earlier message:

My project depends on semver@^5.6.0
A dependency I include (semvish) depends on semver@^4.6.3
electron-builder packages semver@^4.6.3 twice, in semvish/node_modules and the top-level node_modules

This means that in unfortunate cases the application might run but crash due to missing api or be exposed to security issues that were supposedly fixed.
Obviously this is two years old, maybe the behavior in more recent versions has changed again.

[bedney]

@TanninOne - thanks for the report, but I don't think that's what's going on here. I've tracked down the code where this bug is happening and it is coded to specifically omit nested node_modules directories (i.e. node_modules directories that are in submodules of the project). But I'll do some testing and see if I can repro your issue.

I have a patch in hand that I'm testing that seems to do what folks here want and I'm going to test it a bit more this morning before issuing the PR.

[bedney]

All, @somebeaver, @mmaietta -

My PR is located here: #5911.

First of all, thanks @somebeaver for putting together the excellent test case. I used it a lot when testing this.

So, this patch requires some explanation because it's my best attempt to solve a less-than-ideal situation. Currently, due to #1539, all node_modules directories of submodules (but not the main node_modules directory) were omitted from applications built with electron-builder. IMO, this particular feature should have been put behind a flag, but that was not done.

So that is part of what this patch does. I have introduced a new flag, includeSubNodeModules that is false by default (to keep the behavior of the past 3 years), but when switched to true, it will include the node_modules directories of all submodules.

Now, given @somebeaver's use case, I wanted to take it a bit further. There may be cases when you want to not include all submodules node_modules directories, but only some of them. This patch does that as well. If you leave the includeSubNodeModules flag set to false (again, the default), but you specify only certain node_modules directories in your files config property, those will be included as well, as per @somebeaver's example:

    "files": [
      "*/**",
      "src/embedded-submodule/node_modules/**"
    ],

So that's it.

Now, here's the thing about this patch: I am not a TypeScript programmer (been a JavaScripter forever), so I had to struggle a bit with some of the tricks I had to do to make TypeScript cooperate. I needed access to electron-builder's 'options configuration', but that wasn't being passed to the calls where I was so I had to do some 'under cover' private member stuff (and had to instruct the TypeScript compiler not to complain at me). I don't have a good enough knowledge of this codebase to feel like I can just go change APIs at will.

Also, it also looks like I'm getting a couple of test failures. So I could use some help to fix this patch (@mmaietta or others). Lastly, the includeSubNodeModules flag also needs to be added to the documentation.

That's all. I'm hoping we can push this forward and get it into the codebase.

Cheers,

• Bill

[bedney]

A new version of this patch has been pushed and I'm actively working with the project maintainers to resolve any outstanding issues.

[bedney]

Further status update: I now have a patch that passes all of the tests (and I added a few tests to test this new capability). As soon as the maintainers pull this in, we should be good to go :-).

[bedney]

All -

Thanks to @mmaietta this patch is now in the codebase trunk! Hopefully this fixes this longstanding issue. The patch implements the functionality as detailed in #3185 (comment).

If you find other issues with including submodules node_modules directories, please open a separate issue and mention me on that issue.

Cheers!

• Bill

[mmaietta]

Unfortunately, this caused this nasty bug to appear: #6045
Basically 22.11.8-9 only work for solving this ticket's use case, but breaks previous ones
Just an FYI here in case you're adopting the latest version
Working on getting it resolved asap