Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ENOENT during node_modules copy #8068

Closed
fabienr opened this issue Feb 20, 2024 · 7 comments
Closed

ENOENT during node_modules copy #8068

fabienr opened this issue Feb 20, 2024 · 7 comments
Labels

Comments

@fabienr
Copy link

fabienr commented Feb 20, 2024

  • Electron-Builder Version: 24.9.1
  • Node Version: v18.19.0
  • Electron Version: v27.1.3
  • Electron Type (current, beta, nightly):

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
Copy link
Collaborator

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
Copy link
Author

fabienr commented Feb 21, 2024

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

  • Electron-Builder Version: 24.9.1
  • app-builder Version: 3.5.10

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.

  • npm Version : 10.2.3

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 (#6289 (comment))) 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
Copy link
Author

fabienr commented Feb 23, 2024

@mmaietta you should try my patch on develar/app-builder#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
Copy link
Collaborator

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
Copy link
Author

fabienr commented Feb 27, 2024

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.

Copy link
Contributor

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 github-actions bot added the Stale label Jul 15, 2024
Copy link
Contributor

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

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Aug 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants