Skip to content

Commit

Permalink
feat(filter): no longer infer scope in filters (#8137)
Browse files Browse the repository at this point in the history
### Description

Previously we would infer scope for name filters if there was exactly
one matching package.
e.g. `turbo build --filter=ui` would run `@a/ui#build` if there was a
`@a/ui` package in the workspace

This is confusing and can result in accidentally breaking filters when a
conflicting package is added e.g. adding `@b/ui` would cause *no* tasks
to be run along with a zero exit code.

### Testing Instructions

Updated unit test to verify inference no longer works and using the
explicit package name still works.
  • Loading branch information
chris-olszewski committed May 29, 2024
1 parent 5966983 commit d77deb2
Showing 1 changed file with 10 additions and 43 deletions.
53 changes: 10 additions & 43 deletions crates/turborepo-lib/src/run/scope/filter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -542,32 +542,7 @@ fn match_package_names(
.extract_if(|e| matcher.is_match(e.as_ref()))
.collect::<HashSet<_>>();

// if we got no matches and the pattern is not scoped
// check if we have exactly one scoped package that does match
if matched_packages.is_empty()
&& !name_pattern.starts_with('@')
&& !name_pattern.starts_with('/')
{
let scoped_pattern = format!("@*/{}", name_pattern);
let scoped_matcher = SimpleGlob::new(&scoped_pattern)?;

let (first_item, multiple_matches) = {
let mut scoped_matched_packages =
entry_packages.extract_if(|e| scoped_matcher.is_match(e.as_ref())); // we can extract again since the original set is untouched
let first_item = scoped_matched_packages.next();
(first_item, scoped_matched_packages.count() > 0)
};

if multiple_matches {
// if we have more than one, we can't disambiguate
Ok(Default::default())
} else {
// otherwise we either have a match or no match
Ok(first_item.into_iter().collect())
}
} else {
Ok(matched_packages)
}
Ok(matched_packages)
}

#[derive(Debug, thiserror::Error)]
Expand Down Expand Up @@ -1031,30 +1006,22 @@ mod test {
}])
.unwrap();

assert_eq!(
packages,
vec![PackageName::Other("@foo/bar".to_string())]
.into_iter()
.collect()
assert!(
packages.is_empty(),
"expected empty set, got {:?}",
packages
);
}

#[test]
fn match_multiple_scoped() {
let resolver = make_project(
&[],
&["packages/@foo/bar", "packages/@types/bar"],
None,
TestChangeDetector::new(&[]),
);
let packages = resolver
.get_filtered_packages(vec![TargetSelector {
name_pattern: "bar".to_string(),
name_pattern: "@foo/bar".to_string(),
..Default::default()
}])
.unwrap();

assert_eq!(packages, HashSet::new());
assert_eq!(
packages,
vec![PackageName::from("@foo/bar")].into_iter().collect()
);
}

#[test_case(
Expand Down

0 comments on commit d77deb2

Please sign in to comment.