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

Bump yarn to 4.5.3 #11123

Merged
merged 2 commits into from
Dec 13, 2024
Merged

Bump yarn to 4.5.3 #11123

merged 2 commits into from
Dec 13, 2024

Conversation

jeffwidman
Copy link
Member

What are you trying to accomplish?

I'm debugging in this area, and want to make sure I'm working off the latest upstream changes.

@yeikel
Copy link
Contributor

yeikel commented Dec 13, 2024

Closes #8265

@jeffwidman
Copy link
Member Author

jeffwidman commented Dec 13, 2024

This is the failing test:

context "with a yarn berry lockfile" do
context "when updating a dependency with a peer requirement" do
let(:project_name) { "yarn_berry/peer_dependency" }
let(:latest_allowable_version) { Gem::Version.new("16.3.1") }
let(:dependency) do
Dependabot::Dependency.new(
name: "react-dom",
version: "15.2.0",
package_manager: "npm_and_yarn",
requirements: [{
file: "package.json",
requirement: "^15.2.0",
groups: ["dependencies"],
source: { type: "registry", url: "https://registry.npmjs.org" }
}]
)
end
it { is_expected.to eq(Gem::Version.new("15.2.0")) }
end
end

Full commit including lockfiles: 1dcda58

Interestingly, it doesn't fail in Yarn 4.3.1 as seen in:

Poking through the changelog between 4.3.1 and 4.5.3, this seems to be relevant upstream PR:

My understanding of peer dependency handling in Yarn is hazy at best, but after reading the PR description, it looks like the algorithm changed and now the peer dependency can be updated and not necessarily held back.

That would explain this test failure:

  1) Dependabot::NpmAndYarn::UpdateChecker::VersionResolver#latest_resolvable_version with a yarn berry lockfile when updating a dependency with a peer requirement is expected to eq #<Gem::Version "15.2.0">
     Failure/Error: it { is_expected.to eq(Gem::Version.new("15.2.0")) }

       expected: #<Gem::Version "15.2.0">
            got: #<Gem::Version "16.3.1">

       (compared using ==)

       Diff:
       @@ -1 +1 @@
       -Gem::Version.new("15.2.0")
       +Gem::Version.new("16.3.1")
     # ./spec/dependabot/npm_and_yarn/update_checker/version_resolver_spec.rb:873:in `block (5 levels) in <top (required)>'
     # /home/dependabot/common/spec/spec_helper.rb:66:in `block (2 levels) in <top (required)>'
     # /home/dependabot/dependabot-updater/vendor/ruby/3.3.0/gems/webmock-3.24.0/lib/webmock/rspec.rb:39:in `block (2 levels) in <top (required)>'

@jeffwidman jeffwidman marked this pull request as ready for review December 13, 2024 02:00
@jeffwidman jeffwidman requested a review from a team as a code owner December 13, 2024 02:00
@jeffwidman
Copy link
Member Author

@yeikel does ☝️ sound reasonable to you? I am not very familiar with Yarn Berry or peer dependencies in the yarn ecosystem...

Copy link
Contributor

@yeikel yeikel left a comment

Choose a reason for hiding this comment

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

@yeikel does ☝️ sound reasonable to you? I am not very familiar with Yarn Berry or peer dependencies in the yarn ecosystem...

Yes it does. Your analysis seems correct to me.

This is the failing test: 

https://github.com/dependabot/dependabot-core/blob/8f037cf1be97f2a0c1f383d74479ebe2a48e0c17/npm_and_yarn/spec/dependabot/npm_and_yarn/update_checker/version_resolver_spec.rb#L855-L875

Full commit including lockfiles: 1dcda58

Interestingly, it doesn't fail in Yarn `4.3.1` as seen in:
* #8265

Poking through the changelog between 4.3.1 and 4.5.3, this seems to be relevant upstream PR:
* yarnpkg/berry#6517

My understanding of peer dependency handling in Yarn is hazy at best, but after reading the PR description, it _looks_ like the algorithm changed and now the peer dependency can be updated and not necessarily held back. 

That would explain [this test failure](https://github.com/dependabot/dependabot-core/actions/runs/12307737164/job/34351931150?pr=11123#step:5:56):
```
  1) Dependabot::NpmAndYarn::UpdateChecker::VersionResolver#latest_resolvable_version with a yarn berry lockfile when updating a dependency with a peer requirement is expected to eq #<Gem::Version "15.2.0">
     Failure/Error: it { is_expected.to eq(Gem::Version.new("15.2.0")) }

       expected: #<Gem::Version "15.2.0">
            got: #<Gem::Version "16.3.1">

       (compared using ==)

       Diff:
       @@ -1 +1 @@
       -Gem::Version.new("15.2.0")
       +Gem::Version.new("16.3.1")
     # ./spec/dependabot/npm_and_yarn/update_checker/version_resolver_spec.rb:873:in `block (5 levels) in <top (required)>'
     # /home/dependabot/common/spec/spec_helper.rb:66:in `block (2 levels) in <top (required)>'
     # /home/dependabot/dependabot-updater/vendor/ruby/3.3.0/gems/webmock-3.24.0/lib/webmock/rspec.rb:39:in `block (2 levels) in <top (required)>'
```
@jeffwidman jeffwidman merged commit 21cb492 into main Dec 13, 2024
66 checks passed
@jeffwidman jeffwidman deleted the bump-yarn-to-4.5.3 branch December 13, 2024 19:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants