Skip to content

Commit

Permalink
fix(yarn4): avoid overwriting non-npm protocols (#6723)
Browse files Browse the repository at this point in the history
### Description

Fixes #6715

Yarn 4 now makes the default protocol of `npm` (e.g. `"foo": "*"` is
really `"foo": "npm:*"`) explicit in the lockfile representation. This
means that workspaces that reference a package with and without a
protocol will end up with multiple protocols for a single descriptor.

e.g. If one package has a dependency `"c": "*"` and another package in
the workspace has a dependency `"c": "workspace:*"`
In Yarn3 those result in the following descriptors: `c@*, c@workspace:*,
c@workspace:pkgs/c`
In Yarn4 those result in the following descriptors: `c@npm:*,
c@workspace:*, c@workspace:pkgs/c`

We cannot get rid of the logic for case without a protocol as that would
break our Yarn3 usage.

### Testing Instructions

Added unit test that has a lockfile with mixed protocols.

Existing unit tests verify the Yarn3 behavior is still supported.


Closes TURBO-1856

Co-authored-by: Chris Olszewski <Chris Olszewski>
  • Loading branch information
chris-olszewski authored and Zertsov committed Jan 4, 2024
1 parent 473c581 commit b2832ab
Show file tree
Hide file tree
Showing 3 changed files with 84 additions and 5 deletions.
34 changes: 34 additions & 0 deletions crates/turborepo-lockfiles/fixtures/yarn4-mixed-protocol.lock
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
# This file is generated by running "yarn install" inside your project.
# Manual changes might be lost - proceed with caution!

__metadata:
version: 8
cacheKey: 10c0

"a@workspace:pkgs/a":
version: 0.0.0-use.local
resolution: "a@workspace:pkgs/a"
dependencies:
c: "npm:*"
languageName: unknown
linkType: soft

"b@workspace:pkgs/b":
version: 0.0.0-use.local
resolution: "b@workspace:pkgs/b"
dependencies:
c: "workspace:*"
languageName: unknown
linkType: soft

"c@npm:*, c@workspace:*, c@workspace:pkgs/c":
version: 0.0.0-use.local
resolution: "c@workspace:pkgs/c"
languageName: unknown
linkType: soft

"yarn4-test@workspace:.":
version: 0.0.0-use.local
resolution: "yarn4-test@workspace:."
languageName: unknown
linkType: soft
20 changes: 20 additions & 0 deletions crates/turborepo-lockfiles/src/berry/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1028,4 +1028,24 @@ mod test {
resolve@patch:resolve@^1.22.2#~builtin<compat/resolve>"
);
}

#[test]
fn test_yarn4_mixed_protocols() {
let data =
LockfileData::from_bytes(include_bytes!("../../fixtures/yarn4-mixed-protocol.lock"))
.unwrap();
let lockfile = BerryLockfile::new(data, None).unwrap();
// make sure that npm:* protocol still resolves to workspace dependency
let a_deps = lockfile
.all_dependencies("a@workspace:pkgs/a")
.unwrap()
.unwrap();
assert_eq!(a_deps.len(), 1);
let (c_desc, version) = a_deps.into_iter().next().unwrap();
let c_pkg = lockfile
.resolve_package("pkgs/a", &c_desc, &version)
.unwrap()
.unwrap();
assert_eq!(c_pkg.key, "c@workspace:pkgs/c");
}
}
35 changes: 30 additions & 5 deletions crates/turborepo-lockfiles/src/berry/protocol_resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,13 @@ struct Key {
#[derive(Debug, Clone, Hash, PartialEq, Eq, PartialOrd, Ord, Default)]
struct Entry {
without: Option<String>,
with: Option<String>,
with: Option<RangeAndProtocol>,
}

#[derive(Debug, Clone, Hash, PartialEq, Eq, PartialOrd, Ord, Default)]
struct RangeAndProtocol {
protocol: String,
range: String,
}

impl DescriptorResolver {
Expand Down Expand Up @@ -49,9 +55,28 @@ impl Entry {
// with or without a protocol
fn insert_descriptor(&mut self, descriptor: &Descriptor) -> Option<String> {
let range = descriptor.range()?.to_string();
match descriptor.protocol().is_some() {
true => self.with.replace(range),
false => self.without.replace(range),
match descriptor.protocol() {
Some(protocol) => {
// Yarn 4 made the default npm protocol explicit in the lockfile.
// In order to return the more specific protocol we avoid overwriting other
// protocols with the now explicit npm protocol.
if protocol != "npm" || self.with.is_none() {
match self.with.replace(RangeAndProtocol {
range,
protocol: protocol.to_string(),
}) {
// We only return an ejected range if the protocol isn't the default npm
// protocol
Some(RangeAndProtocol { range, protocol }) if protocol != "npm" => {
Some(range)
}
_ => None,
}
} else {
None
}
}
None => self.without.replace(range),
}
}

Expand All @@ -61,7 +86,7 @@ impl Entry {
if self.without.is_some() && descriptor.protocol().is_none() {
self.without.as_deref()
} else {
self.with.as_deref()
self.with.as_ref().map(|x| x.range.as_str())
}
}
}
Expand Down

0 comments on commit b2832ab

Please sign in to comment.