electron / forge

:electron: A complete tool for building and publishing Electron applications
https://electronforge.io
MIT License
6.51k stars 521 forks source link

Support pnpm #2633

Open mika76 opened 2 years ago

mika76 commented 2 years ago

Pre-flight checklist

Problem description

Using pnpm does not seem to work with electron-forge. An error is thrown when executing pnpx electron-forge import and I do not see any mention of pnpm in the repository and docs.

It would be nice if it worked as pnpm is becoming more and more popular.

Proposed solution

Allow pnpm to work and show working in docs along npm and yarn.

Alternatives considered

npm and yarn

Additional information

No response

renhiyama commented 2 years ago

any ETA?

RIP21 commented 2 years ago

Not only import but also the packaging is not working. This is needed to be fixed. It's impossible to use in Monorepos for now :(

AlexanderMac commented 2 years ago

Try the solution from electron-builder: https://www.electron.build/index.html#note-for-pnpm

My .npmrc file in the project root looks so:

node-linker=hoisted
public-hoist-pattern=*
ghost commented 2 years ago

Try the solution from electron-builder: https://www.electron.build/index.html#note-for-pnpm

My .npmrc file in the project root looks so:

node-linker=hoisted
public-hoist-pattern=*

Not work.Error info:

    at makeError (C:\Users\user\AppData\Roaming\npm\node_modules\pnpm\dist\pnpm.cjs:22287:17)
    at handlePromise (C:\Users\user\AppData\Roaming\npm\node_modules\pnpm\dist\pnpm.cjs:22858:33)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)
    at async Object.handler [as dlx] (C:\Users\user\AppData\Roaming\npm\node_modules\pnpm\dist\pnpm.cjs:199133:7)
    at async C:\Users\user\AppData\Roaming\npm\node_modules\pnpm\dist\pnpm.cjs:206494:21
    at async run (C:\Users\user\AppData\Roaming\npm\node_modules\pnpm\dist\pnpm.cjs:206465:34)
    at async runPnpm (C:\Users\user\AppData\Roaming\npm\node_modules\pnpm\dist\pnpm.cjs:206686:5)
    at async C:\Users\user\AppData\Roaming\npm\node_modules\pnpm\dist\pnpm.cjs:206678:7

Env: OS:Windows 10 Node Version:v16.17.1 pnpm Version:7.13.2

BlackHole1 commented 2 years ago

I will investigate this issue in the next few days and would prioritize investigating: whether forge can work with pnpm hoist. https://github.com/electron-userland/electron-builder/issues/6289#issuecomment-1042620422

If we need to get forge to actually support pnpm, it's technically possible to do so, it's just going to take a lot of time. I don't want to provide an approximate completion time for this, but I will raise its priority. πŸ˜ƒ

MarshallOfSound commented 2 years ago

This isn't just a case of supporting pnpm with hoist enabled in this repository, several core dependencies do package traversal. The key ones being:

Galactus depends on flora-colossus so if support is added correctly in the former galactus may Just Workℒ️

Then there's the stuff in this repository, most of which I believe is tied into the yarn-or-npm helper and routed into various helpers throughout the codebase

itsdouges commented 1 year ago

There's two aspects for pnpm support.

  1. Getting electron forge CLI working with it
  2. Getting electron packager working with it

Sounds like (1) can be resolved if flora-colossus is fixed, can you confirm @MarshallOfSound? Could there be an alternate library we can move to that uses the standard node resolution algorithm (as that should just work, no?).

For (2) - that's more tricky but considering it doesn't even support npm workspaces by default we have more problems. I'd say considering it has a workaround (implementing your own packageAfterCopy with prune: false) you could do the same for pnpm.

Is there any appetite from the core Electron team to look at both pnpm + workspace (for any package manger) support?

erickzhao commented 1 year ago

I think @BlackHole1 (who is also a pnpm maintainer) was investigating this issue. Not sure where we ended up!

BlackHole1 commented 1 year ago

Thank you for the reminder, @erickzhao! I apologize for delaying this task πŸ™‡β€β™‚οΈ. Recently, I've been putting my energy into entrepreneurship-related matters and haven't had any extra time to investigate this task, leading me to forget about it 🫨. I will complete my current work as soon as possible to have additional time to conduct the investigation.

sibelius commented 1 year ago

what is missing here?

erickzhao commented 1 year ago

We currently have an open PR attached to this issue @sibelius! If you have any input to give as a pnpm user, would be appreciated.

Everspace commented 1 year ago

@erickzhao Which PR is it? What needs to be done still on it?

GitMurf commented 1 year ago

@erickzhao Which PR is it? What needs to be done still on it?

I assume this one? Although it seems the initial author randomly closed it? https://github.com/electron/forge/pull/3351

cc @erickzhao

LZQCN commented 1 year ago

Is it possible to modify some settings to prevent electron-forge from bundling dependencies, and instead manually bundle dependencies using esbuild or webpack, thereby avoiding dependence on any packages in node_modules? I think this way can bypass the problem of electron-forge being unable to handle pnpm workspace dependencies.

BlackHole1 commented 1 year ago

@LZQCN You can temporarily bypass this issue by configuring pnpm. https://github.com/electron-userland/electron-builder/issues/6289#issuecomment-1042620422

LZQCN commented 1 year ago

Is it possible to modify some settings to prevent electron-forge from bundling dependencies, and instead manually bundle dependencies using esbuild or webpack, thereby avoiding dependence on any packages in node_modules? I think this way can bypass the problem of electron-forge being unable to handle pnpm workspace dependencies.

I did it. see the monorepos project ai-zen/live2d-copilot.

I use esbuild to bundle dependencies, and avoided the electron-packager copying node_modules by using the following settings:

module.exports = {
  packagerConfig: {
    prune: false,
    ignore: [/node_modules/],
  },
}
toFrankie commented 11 months ago

πŸ‘€

SaigyoujiYuyuko233 commented 10 months ago

I use pnpm to install vue and other deps. Too many deps in node_module result npm install electron-squirrel-startup to take 4 hours. (Yes, 4 hours. And I don't really need squirrel)

edit: Changing line 66 in file node_modules\.pnpm\@electron-forge+core-utils@7.2.0\node_modules\yarn-or-npm to return hasYarn() ? 'yarn' : 'pnpm'; (yes, replace npm to pnpm) will solve this issue.

pnpm just took 6 seconds.

TechQuery commented 7 months ago

Too many deps in node_module result npm install electron-squirrel-startup to take 4 hours.

When I pnpx Electron Forge CLI to init a project scaffold, it uses Yarn 1 to install dependencies unfinished in hours.

So I build an Electron scaffold with PNPM 9: idea2app/Electron-Parcel-PNPM.tsx#1

yuyuko233 commented 6 months ago

Any news?

erickzhao commented 6 months ago

I think #3574 is the candidate PR moving forward but it will require a thorough review and I think is currently blocked on failing CI. Please follow along there for progress!

Since most maintainers are not PNPM users, we want to be careful with our approach to merging the code. In particular, Electron's approach to packaging applications assumes that node_modules are stored on disk in the traditional npm format, and things might break in subtle ways when using PNPM's symlink model.

Thanks for your patience, everyone. πŸ™‡

Everspace commented 6 months ago

Thank you for the update and your maintenance which is voluntary.

On Tue., May 21, 2024, 11:21 a.m. Erick Zhao, @.***> wrote:

I think #3574 https://github.com/electron/forge/pull/3574 is the candidate PR moving forward but it will require a thorough review and I think is currently blocked on failing CI. Please follow along there for progress!

Since most maintainers are not PNPM users, we want to be careful with our approach to merging the code. In particular, Electron's approach to packaging applications assumes that node_modules are stored on disk in the traditional npm format, and things might break in subtle ways when using PNPM's symlink model.

Thanks for your patience, everyone. πŸ™‡

β€” Reply to this email directly, view it on GitHub https://github.com/electron/forge/issues/2633#issuecomment-2123189117, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABZV3O722XK4SPVG5WKG2QDZDOGBLAVCNFSM5JRNV57KU5DIOJSWCZC7NNSXTN2JONZXKZKDN5WW2ZLOOQ5TEMJSGMYTQOJRGE3Q . You are receiving this because you commented.Message ID: @.***>

bayoudhi commented 5 months ago

Any updates?

goosewobbler commented 5 months ago

Not much to update here that I didn't already mention on the PR. I haven't had time to work on this and it doesn't look like I will have much time going forward. I picked up where the previous PR left off, made a small amount of progress. I was testing some more changes locally including fixes for the slow tests which have some issues in their current state. I updated the PR with these - apart from that, someone who is familiar with the codebase should probably take a look at this.

I suspect we should fix the dependencies (@electron/packager, flora-colossus) first before doing any more on this PR.

goosewobbler commented 5 months ago

I've been converting wdio-electron-service to use Turborepo and PNPM workspaces, Forge kept erroring during build with missing deps.

In case it helps anyone, I gained limited success with bundling and also node-linker=hoisted, the latter of which fixed the forge cases but broke everything else. Now we're using Yarn (classic) to build the forge apps and PNPM everywhere else, nice and hacky but it does work.

ghost commented 2 months ago

open issue, 71+ upvotes. 0 fix

bibibala commented 1 month ago

am waiting for.......

grifotv commented 1 month ago

Has anyone tried uninstalling yarn globally? https://stackoverflow.com/questions/72201239/pnpm-install-usage-error-this-project-is-configured-to-use-yarn/78518215#78518215

rtritto commented 1 month ago

Has anyone tried uninstalling yarn globally? https://stackoverflow.com/questions/72201239/pnpm-install-usage-error-this-project-is-configured-to-use-yarn/78518215#78518215

@grifotv we're waiting #3209 for a long time...

sebbean commented 6 days ago

i'm here with ya'll!