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

build: Lockfile churn #23626

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

Conversation

alexvy86
Copy link
Contributor

Description

Changes that result from adding a dependency somewhere, doing pnpm i --no-frozen-lockfile, removing the dependency, and doing pnpm i --no-frozen-lockfile again.

Seems to be a lot of churn that the upgrade to pnpm v9 should have applied but didn't. A couple breaking changes in the v9 release notes talk about peer dendencies, which (part of?) this seems to be related to.

Reviewer Guidance

The review process is outlined on this wiki page.

@Copilot Copilot bot review requested due to automatic review settings January 22, 2025 01:55

Choose a reason for hiding this comment

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

Copilot reviewed 1 out of 1 changed files in this pull request and generated no comments.

@github-actions github-actions bot added dependencies Pull requests that update a dependency file base: main PRs targeted against main branch labels Jan 22, 2025
@tylerbutler tylerbutler requested a review from a team January 22, 2025 22:42
Copy link
Member

@tylerbutler tylerbutler left a comment

Choose a reason for hiding this comment

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

These changes seem to be expected/OK given my understanding of the pnpm docs, so I'm approving. I don't see any other real options since any dep change seems destined to make these changes. That said, I suggest getting further input before merge.

@alexvy86
Copy link
Contributor Author

@jason-ha any further thoughts? I'm with Tyler, seems better to merge this on its own so we unblock other changes that touch deps, and investigate the weirdness separately. Unless we expect to be able to resolve it quick, but based on my experiments yesterday and today I don't feel very confident about that (and I'm not spending more time on it right now).

@jason-ha
Copy link
Contributor

I'm okay moving forward - commented in Teams thread.
Might also want a pnpm dedupe when cleaning up, but certainly can be separate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
base: main PRs targeted against main branch dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants