Skip to content

fix: enable pnpm via corepack for apps that require it (e.g. drive)#1808

Open
Tguntenaar wants to merge 1 commit intofrappe:mainfrom
Bamboi-tech:fix/add-pnpm
Open

fix: enable pnpm via corepack for apps that require it (e.g. drive)#1808
Tguntenaar wants to merge 1 commit intofrappe:mainfrom
Bamboi-tech:fix/add-pnpm

Conversation

@Tguntenaar
Copy link
Copy Markdown

The drive app's frontend build runs npm run check-pnpm && pnpm install && pnpm build. In minimal Docker images, the non-root frappe user
cannot reliably npm install -g pnpm, so drive's install-pnpm.sh
silently fails and bench init aborts with exit code 127 during asset
building. I've made a PR their too so it doesn't silently fail, however that doesn't fix that there is not access to pnpm.

Enabling corepack in the base stage (where we still run as root) makes
pnpm available system-wide without requiring a separate global install.

@DanielRadlAMR
Copy link
Copy Markdown
Collaborator

why would you need pnpm in first place?

@Tguntenaar
Copy link
Copy Markdown
Author

Hi @DanielRadlAMR, thanks for your response. In this case its the frappe drive app that is using pnpm but I assume there would be others

https://github.com/frappe/drive/blob/7d22e5a9751a8781d90afc8a6f8e782ae505cbd4/package.json#L9

@DanielRadlAMR
Copy link
Copy Markdown
Collaborator

Hmm, that’s interesting—do you know why they chose pnpm?

Most other Frappe apps use Yarn, so using pnpm would create duplicate structures, which isn’t ideal IMO.

@Tguntenaar
Copy link
Copy Markdown
Author

Some of the newer apps use pnpm apparently

frappe/drive#594 (comment)

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.

2 participants