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.59k stars 1.73k forks source link

ENOENT during node_modules copy #8068

Closed fabienr closed 1 month ago

fabienr commented 7 months ago

I'm new to electron & nodejs but it looks like the way node_modules is populated by npm does not match what app-builder return, example : ENOENT: no such file or directory, scandir '.../node_modules/ajv-formats/node_modules/fast-deep-equal' NOTE, node_modules/fast-deep-equal exists but outside ajv-formats.

Is there any solution to teach npm to create links ?

What does other package manager (yarn, pnpm) do in this situation (sub-depends with same version) ?

I'm not sure this is something app-builder have to handle.

I attach a quick diff which help me make progress, any better solution ? NodeModuleCopyHelper.js.patch

mmaietta commented 7 months ago

Is this your actual version? Electron-Builder Version: 3.5.10

If so, my only suggestion is to upgrade your builder version (24.x.x). Other than that, there's not really any support I can provide atm

fabienr commented 7 months ago

Sry, I typed app-builder instead of electron-builder to get the version, my bad:

Will dig into app-builder, someone also had similar problem with dependencies (wrong path), see [https://github.com/develar/app-builder/pull/93]. Then if I understand correctly this mean app-builder have to return the real path of those depends. Looks like Electron-builder just blindly copy/paste those paths.

Please let me know if someone have the same problem with this npm version. I'm guessing this is the same problem as with pnpm (hoist, see (https://github.com/electron-userland/electron-builder/issues/6289#issuecomment-1042620422)) but I have no idea howto tell npm to create links for me.

One idea is to use --install-strategy=linked (https://docs.npmjs.com/cli/v10/using-npm/config#install-strategy) but the bundle .asar copy contain only node_modules/.store which fail to resolve during execution. Perhaps nodejs should handle this on require() but a quick test tell me it's not the case, furthermore electron may bundle an older nodejs.

fabienr commented 7 months ago

@mmaietta you should try my patch on https://github.com/develar/app-builder/issues/105 to see if it fix other issues you have with latest app-builber. I just hoist the depends in app-builder tree but a proper fix would be to resolve the real directory before in order to avoid doing duplicate scan. Then let me know how/if we can close my issue.

mmaietta commented 6 months ago

Sorry, I tried it and it didn't work. You can test the patch in the app-builder repo directly by running go test ./... (either from root or from node-modules dir for isolated testing)

Basically, their package resolution doesn't work on the latest app-builder version, so I can't upgrade electron-builder to use it yet. Once their tests are passing, it should work directly in electron-builder

fabienr commented 6 months ago

All the test pass on my side, even without the 3 lines diff I wrote. My real test was a port of stretchly.

Btw I notice the version return by the command line does not match the github version. I'm testing 4.2.0.

I also add a yarn test (yar-demo, same package.json, pre-generated yarn.lock). Not sure about github ci and so but you need to manually populate the node_modules under each test (cd pkg/node-modules/npm-demo && npm ci). Or * install --frozen-lockfile. Even if you update the lock the test should pass furthermore it does not exercise hoist module.

ok github.com/develar/app-builder/pkg/node-modules 8.213s

What I'm not able is to run electron-builder test and this specific one.

FAIL src/HoistedNodeModuleTest.ts (24.672 s) ENOENT: no such file or directory, scandir '/tmp/et-db411cd21f3bbd882f3a5a30c58db6ff/t-SSzica/test-project-1/packages/test-app/node_modules/foo'

Looks like I need tsc and patch for puppeteer to build/run those test on OpenBSD. I had similar issue with pnpm but I finally fetch from npm registry which works oob without any build stage. I may fix that later but that's not even on my todo for now.

Please tell me what happens with the diff I wrote for app-builder : TEST_FILES=HoistedNodeModuleTest pnpm run test-linux (quote) . Also a ls -R .../test-project-1/packages/test-app/node_modules & app-builder node-dep-tree --dir .../test-project-1/packages/test-app/ can help to understand. Not sure but maybe you will need USE_SYSTEM_APP_BUILDER=true to force the system version (which I install with the diff on my side).

If I manage to run this test even by hand, I will let you know.

github-actions[bot] commented 2 months ago

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 30 days.

github-actions[bot] commented 1 month ago

This issue was closed because it has been stalled for 30 days with no activity.