Skip to content
New issue

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

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

Already on GitHub? # to your account

fix: add dmg-builder and squirrel-windows to peerDependencies for pnpm #8052

Merged
merged 2 commits into from
Feb 17, 2024

Conversation

taozhou-glean
Copy link
Contributor

@taozhou-glean taozhou-glean commented Feb 14, 2024

seeing error with pnpm:

 ⨯ Cannot find module 'dmg-builder'
Require stack:
- /xxx/node_modules/.pnpm/app-builder-lib@24.13.0_patch_hash=rhv5z6uqia42ljlrohzdneemdi/node_modules/app-builder-lib/out/macPackager.js
- /xxx/node_modules/.pnpm/app-builder-lib@24.13.0_patch_hash=rhv5z6uqia42ljlrohzdneemdi/node_modules/app-builder-lib/out/packager.js
- /xxx/node_modules/.pnpm/app-builder-lib@24.13.0_patch_hash=rhv5z6uqia42ljlrohzdneemdi/node_modules/app-builder-lib/out/index.js
- /xxx/node_modules/.pnpm/electron-builder@24.13.0/node_modules/electron-builder/out/builder.js
- /xxx/node_modules/.pnpm/electron-builder@24.13.0/node_modules/electron-builder/out/cli/cli.js
- /xxx/node_modules/.pnpm/electron-builder@24.13.0/node_modules/electron-builder/cli.js  failedTask=build stackTrace=Error: Cannot find module 'dmg-builder'

the /.pnpm/app-builder-lib@24.13.0/node_modules/:

image

while it works if I install dmg-builder directly to the entire workspace, I suspect that without it in the dependency, it's not listed in /.pnpm/app-builder-lib@24.13.0/node_modules/ and results in the error

and honestly it is imported (tho conditionally) to the code directly:

const { DmgTarget } = require("dmg-builder")
, so seems a better fit to dependencies ? or maybe peer dependencies ? (looks like peerDependencies can work as well: https://pnpm.io/how-peers-are-resolved)

Copy link

changeset-bot bot commented Feb 14, 2024

🦋 Changeset detected

Latest commit: 2a9ce10

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 8 packages
Name Type
app-builder-lib Patch
dmg-builder Patch
electron-builder-squirrel-windows Patch
electron-builder Patch
electron-forge-maker-appimage Patch
electron-forge-maker-nsis-web Patch
electron-forge-maker-nsis Patch
electron-forge-maker-snap Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link

netlify bot commented Feb 14, 2024

Deploy Preview for car-park-attendant-cleat-11576 ready!

Name Link
🔨 Latest commit 2a9ce10
🔍 Latest deploy log https://app.netlify.com/sites/car-park-attendant-cleat-11576/deploys/65ce72c8530b6c0008ae967e
😎 Deploy Preview https://deploy-preview-8052--car-park-attendant-cleat-11576.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@taozhou-glean taozhou-glean changed the title fix: add dmg-builder and squirrel-windows to dependency for pnpm fix: add dmg-builder and squirrel-windows to peerDependencies for pnpm Feb 14, 2024
@mmaietta
Copy link
Collaborator

What version of pnpm are you using? Trying to replicate this issue but not able to atm

@taozhou-glean
Copy link
Contributor Author

What version of pnpm are you using? Trying to replicate this issue but not able to atm

8.15.1

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants