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

Dependencies are not packaged by electron-builder when using pnpm #6289

Closed
Ripwords opened this issue Sep 26, 2021 · 15 comments
Closed

Dependencies are not packaged by electron-builder when using pnpm #6289

Ripwords opened this issue Sep 26, 2021 · 15 comments

Comments

@Ripwords
Copy link

Ripwords commented Sep 26, 2021

  • Electron-Builder Version: 22.11.7
  • Node Version: 16.9.1
  • Electron Version: 15.0.0
  • Electron Type (current, beta, nightly): current
  • Target: windows/nsis

I'm having issues trying to use pnpm and electron-builder. I tried multiple times to package the app using basic commands and build configurations from online guides but i would always get dependency related errors after packaging it.

Initially i thought it was my configuration error, but after copying the whole project and used npm instead, everything worked as expected.

From what i could find, it seems that electron-builder can't resolve the dependencies within the .pnpm folder, but i am not 100% certain of this.

pnpm and electron-builder are both great would love to have this fixed

Build config

image

Using pnpm

As shown in the screenshot, semver already exist in node_modules/.pnpm
image

Using npm with same exact build config

Application is built with no errors by electron-builder
image

@mmaietta
Copy link
Collaborator

@develar can you please take a look at this when you have a chance?

@cangzhang
Copy link

cangzhang commented Sep 28, 2021

Run into same problem, the node_modules folder bundled in app.asar does not contain modules installed under .pnpm/, which are symlinks created by pnpm install.

@plimeor
Copy link

plimeor commented Oct 11, 2021

Any updates?

@cah4a
Copy link

cah4a commented Nov 19, 2021

Hey guys!
Do you have any plans to support pnpm?
It reduces pain for developers that have gigabytes of node_modules placed here and there.
Would love to use it with electron too.

@mmaietta
Copy link
Collaborator

I don't have plans to implement the support for pnp. But I'd happily review PRs for it! 🙂

@cupcakearmy
Copy link

Same here, would be awesome to have support for it

@mmaietta
Copy link
Collaborator

mmaietta commented Feb 5, 2022

Quick test, can someone try setting .npmrc to copy from the global pnpm store for deps?
https://pnpm.io/npmrc#package-import-method

package-import-method=copy

@monsterkodi
Copy link

I tried the copy setting. But doesn't seem to make any difference for me.

@mmaietta
Copy link
Collaborator

mmaietta commented Feb 9, 2022

@develar really could use your help on this one. Seems to be failing with readDependencyTree in your app-builder project.
https://github.com/develar/app-builder/blob/c79345743b794ccef41493decafb0b5b2eece505/pkg/node-modules/tree.go#L41

I've tried other alternatives for reading dependency trees and they aren't viable. I'm not sure how app-builder works, but it seems like it just needs to also search node_modules/.pnpm/node_modules/* for all dependencies

@BlackHole1
Copy link
Contributor

BlackHole1 commented Feb 17, 2022

  • set node-linker=hoisted in .npmrc
  • set public-hoist-pattern = * or shamefully-hoist = true in .npmrc

Choose any of them. The problem will be solved.
Of course, it would be great if electron-builder could support it.

Update:

If your dependencies have multiple versions, it's best to add node-linker = hoisted to avoid having only one version selected during packaging. Refer to the link for more information: pnpm/pnpm#5724 (comment)

@CallanBi
Copy link

Same. Would really love to see it is supported.

@the0rem
Copy link

the0rem commented Feb 24, 2022

@BlackHole1 thanks for the suggested solution. This is probably an acceptable solution for some however the suggested approach is basically throwing away the benefits of using pnpm in the first place.

The repo's readme also links to a comment about why yarn should be used however with 129 downvotes and 94 confused emojis it's clearly not backed by the community running into these issues.

I'm not able to provide a better solution myself but I'd like to point out how much of a hurdle this is for adoption. We're not prepared to relax the reliability of our dependency system that pnpm brings to our monorepo. Changing this for a single package is an unreasonable ask. I hope there is a path forward soon; A fundamental library shouldn't have to compromise it's audience based on a package manager.

@stale
Copy link

stale bot commented Apr 28, 2022

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.

@stale stale bot added the backlog label Apr 28, 2022
@cah4a
Copy link

cah4a commented Apr 28, 2022

Any progress here?

@stale stale bot removed the backlog label Apr 28, 2022
@mmaietta
Copy link
Collaborator

Issue is in upstream dependency app-builder (node_modules/app-builder-bin)
Ref: #6289 (comment)
You're welcome to file a ticket here https://github.com/develar/app-builder

Workaround is: #6289 (comment)

@electron-userland electron-userland locked and limited conversation to collaborators Apr 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

10 participants