Skip to content

Commit

Permalink
fix(pnpm): correctly parse dependency paths with nested peer dependec…
Browse files Browse the repository at this point in the history
…ies (#8003)

### Description

Should address the reported issues in #7993 

I missed a change to the dependency path format where nested peer
dependencies are now shown e.g. if a has peer dependency b and b has
peer dependency c, then the resulting dependency path will be
`a@1.0.0(b@1.0.0(c@1.0.0))`.

Not sure when dependency path parsing got simplified, but I believe I
missed the host segment getting dropped which greatly simplifies the
format. The new parsing logic is a faithful port of the current JS
parsing code:
https://github.com/pnpm/pnpm/blob/main/packages/dependency-path/src/index.ts#L91

### Testing Instructions

Added unit test for parsing a dependency path that contains a nested
peer dependency. Existing test suite.


Closes TURBO-2848
  • Loading branch information
chris-olszewski authored Apr 19, 2024
1 parent 07c8ac8 commit bc7b17a
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 23 deletions.
69 changes: 47 additions & 22 deletions crates/turborepo-lockfiles/src/pnpm/dep_path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,16 @@ use nom::{

use super::SupportedLockfileVersion;

#[derive(Debug, thiserror::Error)]
pub enum Error {
#[error(transparent)]
Nom(#[from] nom::error::Error<String>),
#[error("dependency path '{0}' contains no '@'")]
MissingAt(String),
#[error("dependency path '{0}' has an empty version following '@'")]
MissingVersion(String),
}

#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)]
pub struct DepPath<'a> {
// todo we possibly keep the full string here for 0-cost serialization
Expand All @@ -27,17 +37,14 @@ impl<'a> DepPath<'a> {
}
}

pub fn parse(
version: SupportedLockfileVersion,
input: &'a str,
) -> Result<Self, nom::error::Error<String>> {
let (_, dep_path) = match version {
SupportedLockfileVersion::V7AndV9 => parse_dep_path_v7(input),
SupportedLockfileVersion::V5 | SupportedLockfileVersion::V6 => parse_dep_path(input),
pub fn parse(version: SupportedLockfileVersion, input: &'a str) -> Result<Self, Error> {
match version {
SupportedLockfileVersion::V7AndV9 => parse_dep_path_v9(input),
SupportedLockfileVersion::V5 | SupportedLockfileVersion::V6 => {
let (_, dep_path) = parse_dep_path(input).map_err(|e| e.to_owned()).finish()?;
Ok(dep_path)
}
}
.map_err(|e| e.to_owned())
.finish()?;
Ok(dep_path)
}

pub fn with_host(mut self, host: Option<&'a str>) -> Self {
Expand Down Expand Up @@ -88,16 +95,30 @@ fn parse_dep_path(i: &str) -> IResult<&str, DepPath> {
))
}

fn parse_dep_path_v7(i: &str) -> IResult<&str, DepPath> {
let (i, name) = parse_name(i)?;
let (i, _) = nom::character::complete::one_of("/@")(i)?;
let (i, version) = parse_version(i)?;
let (i, peer_suffix) = opt(parse_new_peer_suffix)(i)?;
let (_, _) = nom::combinator::eof(i)?;
Ok((
"",
DepPath::new(name, version).with_peer_suffix(peer_suffix),
))
fn parse_dep_path_v9(input: &str) -> Result<DepPath, Error> {
if input.is_empty() {
return Err(Error::MissingAt(input.to_owned()));
}
let sep_index = input[1..]
.find('@')
.ok_or_else(|| Error::MissingAt(input.to_owned()))?
+ 1;
// Need to check if sep_index is valid index
if sep_index >= input.len() {
return Err(Error::MissingVersion(input.to_owned()));
}
let (name, version) = input.split_at(sep_index);
let version = &version[1..];

let (version, peer_suffix) = match version.find('(') {
Some(paren_index) if version.ends_with(')') => {
let (version, peer_suffix) = version.split_at(paren_index);
(version, Some(peer_suffix))
}
_ => (version, None),
};

Ok(DepPath::new(name, version).with_peer_suffix(peer_suffix))
}

fn parse_host(i: &str) -> IResult<&str, Option<&str>> {
Expand Down Expand Up @@ -170,9 +191,13 @@ mod tests {
#[test_case("foo@1.0.0", DepPath::new("foo", "1.0.0") ; "basic v7")]
#[test_case("@scope/foo@1.0.0", DepPath::new("@scope/foo", "1.0.0") ; "scope v7")]
#[test_case("foo@1.0.0(bar@1.2.3)", DepPath::new("foo", "1.0.0").with_peer_suffix(Some("(bar@1.2.3)")) ; "peer v7")]
#[test_case(
"eslint-module-utils@2.8.0(@typescript-eslint/parser@6.12.0(eslint@8.57.0)(typescript@5.3.3))",
DepPath::new("eslint-module-utils", "2.8.0").with_peer_suffix(Some("(@typescript-eslint/parser@6.12.0(eslint@8.57.0)(typescript@5.3.3))"))
; "nested peer deps"
)]
fn dep_path_parse_v7_tests(s: &str, expected: DepPath) {
let (rest, actual) = parse_dep_path_v7(s).unwrap();
assert_eq!(rest, "");
let actual = parse_dep_path_v9(s).unwrap();
assert_eq!(actual, expected);
}

Expand Down
2 changes: 1 addition & 1 deletion crates/turborepo-lockfiles/src/pnpm/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use crate::Lockfile;
#[derive(Debug, thiserror::Error)]
pub enum Error {
#[error("error parsing dependency path: {0}")]
DependencyPath(#[from] nom::error::Error<String>),
DependencyPath(#[from] dep_path::Error),
#[error("Unable to find '{0}' other than reference in dependenciesMeta")]
MissingInjectedPackage(String),
#[error("unsupported lockfile version: {0}")]
Expand Down

0 comments on commit bc7b17a

Please sign in to comment.