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

ci: Upgrade to pnpm 10 #12502

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

ci: Upgrade to pnpm 10 #12502

wants to merge 1 commit into from

Conversation

netroy
Copy link
Member

@netroy netroy commented Jan 8, 2025

Summary

pnpm 10 was released a month ago.
So far I have not found any regressions in our setup from this upgrade.
Once the CI is green, and if custom docker images work as expected, we can merge this.

Review / Merge checklist

  • PR title and summary are descriptive. (conventions)
  • Docs updated or follow-up ticket created.
  • Tests included.
  • PR Labeled with release/backport (if the PR is an urgent fix that needs to be backported)

@n8n-assistant n8n-assistant bot added the n8n team Authored by the n8n team label Jan 8, 2025
Copy link

codecov bot commented Jan 8, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

📢 Thoughts on this report? Let us know!

@netroy netroy marked this pull request as ready for review February 4, 2025 08:13
@@ -18,7 +18,7 @@ RUN find . -type f -name "*.ts" -o -name "*.js.map" -o -name "*.vue" -o -name "t

# Deploy the `n8n` package into /compiled
RUN mkdir /compiled
RUN NODE_ENV=production DOCKER_BUILD=true pnpm --filter=n8n --prod --no-optional deploy /compiled
RUN NODE_ENV=production DOCKER_BUILD=true pnpm --filter=n8n --prod --no-optional --legacy deploy /compiled
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does the --legacy do? Can we document that

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TBH, I don't know for sure. All I know is that pnpm 10 changed the default implementation for pnpm deploy which breaks with catalog: dependencies. Using --legacy switched back to the behavior we had on pnpm 9.
There is a discussion here.
I'm hoping that before pnpm 11 I would have made sense of this change, and can remove the flag by then.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add the link to the discussion as a comment for future reference

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

created a ticket for it.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
n8n team Authored by the n8n team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants