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

feat: support pnpm #3574

Draft
wants to merge 34 commits into
base: main
Choose a base branch
from
Draft

Conversation

goosewobbler
Copy link

@goosewobbler goosewobbler commented Apr 24, 2024

Signed-off-by: Yi Yang [email protected]

  • I have read the contribution documentation for this project.
  • I agree to follow the code of conduct that this project follows, as appropriate.
  • The changes are appropriately documented (if applicable).
  • The changes have sufficient test coverage (if applicable).
  • The testsuite passes successfully on my local machine (if applicable).

Summarize your changes:

Adding PNPM support - picking up where #3351 left off.

Related issue: #2633

@goosewobbler goosewobbler marked this pull request as ready for review April 24, 2024 16:09
@goosewobbler goosewobbler requested a review from a team as a code owner April 24, 2024 16:09
package.json Show resolved Hide resolved
@erickzhao erickzhao self-requested a review May 1, 2024 22:22
@emilbon99
Copy link

Is this PR ready to go? If so, is there an ETA on when it might get merged?

@goosewobbler
Copy link
Author

I wasn't the original author of this code but happy to do what it takes to get this one released - we use PNPM in WDIO and native support here would definitely help.

@erickzhao
Copy link
Member

I think there are some legitimate test failures that we'll have to investigate in slow-tests.

It may be that pnpm is hanging somewhere along the line? It seems like all 3 platforms are hanging and CircleCI is cutting off the CI jobs.

@erickzhao erickzhao mentioned this pull request May 21, 2024
3 tasks
@goosewobbler
Copy link
Author

goosewobbler commented May 21, 2024

It is hanging on this line, the package step of the import test.

Added more logging to core/api/package.ts and it seems this await never resolves...

@goosewobbler
Copy link
Author

goosewobbler commented May 22, 2024

More digging. All of the tests passed in d334388 and subsequently broke with the merge in cf43b13.

Debugging the utils/core-utils/src/rebuild.ts results in the following:

electron-forge:rebuild Spawning a remote rebuilder process with options: {
  buildPath: '/var/folders/g6/0tqr_59j76n0g6zxxq_0tq3c0000gn/T/electron-packager/tmp-ukJFul/Electron.app/Contents/Resources/app',
  electronVersion: '30.0.6',
  arch: 'arm64'
}
electron-forge:rebuild Received message from rebuilder: {
  msg: 'rebuild-error',
  err: {
    message: "ENOENT: no such file or directory, stat '/private/var/folders/g6/0tqr_59j76n0g6zxxq_0tq3c0000gn/T/electron-packager/tmp-ukJFul/Electron.app/Contents/Resources/app/node_modules/electron-squirrel-startup'",
    stack: "Error: ENOENT: no such file or directory, stat '/private/var/folders/g6/0tqr_59j76n0g6zxxq_0tq3c0000gn/T/electron-packager/tmp-ukJFul/Electron.app/Contents/Resources/app/node_modules/electron-squirrel-startup'"
  }
}

There are two symlinks in the node_modules dir:

lrwxr-xr-x@   electron -> .pnpm/[email protected]/node_modules/electron
lrwxr-xr-x@   electron-squirrel-startup -> .pnpm/[email protected]/node_modules/electron-squirrel-startup

I wonder if this is an issue with @electron/rebuild. Could be related to electron/rebuild#547.

@goosewobbler goosewobbler marked this pull request as draft June 11, 2024 13:36
@goosewobbler
Copy link
Author

Not sure what is going on here and at the moment I don't have the time to dedicate to it. Hopefully I get some more time, happy for someone else to pick this up though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants