Skip to content

Commit bde5469

Browse files
jfrolichclaude
andcommitted
Rewatch: skip dev-dependency feature requests in --prod
compute_active_features scanned both dependencies and dev-dependencies on every consumer regardless of the active build mode. In --prod, a dep reached via dependencies with a restricted feature list could still have a shorthand dev-dependencies entry on the same consumer flip any_all_request = true and force all features active — pulling in feature-gated code that --prod is supposed to exclude. Match read_dependencies's rule: dev-dependency edges only contribute to a dep's active feature set when the consumer is local and we're not in --prod. Regression test covers both modes. Signed-off-by: Jaap Frolich <jaap@tella.com> Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Jaap Frolich <jfrolich@gmail.com>
1 parent b42718d commit bde5469

1 file changed

Lines changed: 106 additions & 17 deletions

File tree

rewatch/src/build/packages.rs

Lines changed: 106 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -766,9 +766,15 @@ fn collect_gentype_source_dirs(package: &Package) -> Vec<PathBuf> {
766766
/// or no consumer requests anything, the dep builds with all of its declared features.
767767
/// Otherwise we take the union of specific feature requests and pass it through the dep's
768768
/// own `features` map for transitive expansion.
769+
///
770+
/// `dev-dependencies` only contribute when that edge is actually traversed by
771+
/// `read_dependencies` — i.e. when the consumer is a local dep and we're not in `--prod`. This
772+
/// keeps dev-only feature requests (e.g. a shorthand `dev-dependencies` entry that would flip
773+
/// everything to "all features") from leaking into production builds.
769774
pub fn compute_active_features(
770775
packages: &AHashMap<String, Package>,
771776
cli_features: Option<&Vec<String>>,
777+
prod: bool,
772778
) -> Result<AHashMap<String, AHashSet<String>>> {
773779
let mut result: AHashMap<String, AHashSet<String>> = AHashMap::new();
774780

@@ -783,20 +789,32 @@ pub fn compute_active_features(
783789
}
784790
} else {
785791
for consumer in packages.values() {
786-
let deps_iter = consumer
787-
.config
788-
.dependencies
789-
.as_ref()
790-
.into_iter()
791-
.flatten()
792-
.chain(consumer.config.dev_dependencies.as_ref().into_iter().flatten());
793-
for dep in deps_iter {
794-
if dep.name() != package_name {
795-
continue;
792+
// `dependencies` always contribute.
793+
if let Some(deps) = consumer.config.dependencies.as_ref() {
794+
for dep in deps {
795+
if dep.name() != package_name {
796+
continue;
797+
}
798+
match dep.features() {
799+
None => any_all_request = true,
800+
Some(list) => requested.extend(list.iter().cloned()),
801+
}
796802
}
797-
match dep.features() {
798-
None => any_all_request = true,
799-
Some(list) => requested.extend(list.iter().cloned()),
803+
}
804+
// `dev-dependencies` only contribute when that edge is actually traversed:
805+
// local consumer, not `--prod`. Matches `read_dependencies`.
806+
if consumer.is_local_dep
807+
&& !prod
808+
&& let Some(dev_deps) = consumer.config.dev_dependencies.as_ref()
809+
{
810+
for dep in dev_deps {
811+
if dep.name() != package_name {
812+
continue;
813+
}
814+
match dep.features() {
815+
None => any_all_request = true,
816+
Some(list) => requested.extend(list.iter().cloned()),
817+
}
800818
}
801819
}
802820
}
@@ -837,7 +855,7 @@ pub fn make(
837855
) -> Result<AHashMap<String, Package>> {
838856
let mut map = read_packages(project_context, show_progress, prod)?;
839857

840-
let active_features = compute_active_features(&map, cli_features)?;
858+
let active_features = compute_active_features(&map, cli_features, prod)?;
841859

842860
// Drop source directories whose feature tag is not in the package's active set.
843861
// Untagged source dirs remain; they're included regardless of the feature selection.
@@ -1488,7 +1506,7 @@ mod test {
14881506
),
14891507
);
14901508

1491-
let active = super::compute_active_features(&packages, None).unwrap();
1509+
let active = super::compute_active_features(&packages, None, false).unwrap();
14921510
let root = active.get("root").unwrap();
14931511
assert!(root.contains("native"));
14941512
assert!(root.contains("full"));
@@ -1515,7 +1533,7 @@ mod test {
15151533
);
15161534

15171535
let cli = vec!["full".to_string()];
1518-
let active = super::compute_active_features(&packages, Some(&cli)).unwrap();
1536+
let active = super::compute_active_features(&packages, Some(&cli), false).unwrap();
15191537
let root = active.get("root").unwrap();
15201538
assert!(root.contains("full"));
15211539
assert!(root.contains("native"));
@@ -1565,8 +1583,79 @@ mod test {
15651583
// Flip the dep's is_root flag to false since root_package_with_features sets it to true.
15661584
packages.get_mut("dep").unwrap().is_root = false;
15671585

1568-
let active = super::compute_active_features(&packages, None).unwrap();
1586+
let active = super::compute_active_features(&packages, None, false).unwrap();
1587+
let dep_active = active.get("dep").unwrap();
1588+
assert!(dep_active.contains("native"));
1589+
}
1590+
1591+
#[test]
1592+
fn compute_active_features_prod_ignores_dev_dependency_feature_requests() {
1593+
// Regression: a dep that appears in both `dependencies` (restricted to `native`) and
1594+
// `dev-dependencies` (shorthand = all features). In `--prod` the dev edge isn't
1595+
// traversed by `read_dependencies`, so its broader feature request must not flip the
1596+
// dep's active set to "all features". Only the non-dev restriction applies.
1597+
let mut packages: AHashMap<String, super::Package> = AHashMap::new();
1598+
1599+
let mut root_config = config::tests::create_config(config::tests::CreateConfigArgs {
1600+
name: "root".to_string(),
1601+
bs_deps: vec![],
1602+
build_dev_deps: vec![],
1603+
allowed_dependents: None,
1604+
path: PathBuf::from("./rescript.json"),
1605+
});
1606+
root_config.sources = Some(config::OneOrMore::Single(config::Source::Shorthand(
1607+
"src".to_string(),
1608+
)));
1609+
root_config.dependencies = Some(vec![config::Dependency::Qualified(config::QualifiedDependency {
1610+
name: "dep".to_string(),
1611+
features: Some(vec!["native".to_string()]),
1612+
})]);
1613+
root_config.dev_dependencies = Some(vec![config::Dependency::Shorthand("dep".to_string())]);
1614+
packages.insert(
1615+
"root".to_string(),
1616+
super::Package {
1617+
name: "root".to_string(),
1618+
config: root_config,
1619+
source_folders: AHashSet::new(),
1620+
source_files: None,
1621+
namespace: super::Namespace::NoNamespace,
1622+
modules: None,
1623+
path: PathBuf::from("."),
1624+
dirs: None,
1625+
is_local_dep: true,
1626+
is_root: true,
1627+
},
1628+
);
1629+
1630+
packages.insert(
1631+
"dep".to_string(),
1632+
root_package_with_features(
1633+
None,
1634+
vec![
1635+
("src", None),
1636+
("src-native", Some("native")),
1637+
("src-experimental", Some("experimental")),
1638+
],
1639+
),
1640+
);
1641+
packages.get_mut("dep").unwrap().is_root = false;
1642+
1643+
// Non-prod: dev edge IS traversed → all features active.
1644+
let active = super::compute_active_features(&packages, None, /* prod */ false).unwrap();
15691645
let dep_active = active.get("dep").unwrap();
15701646
assert!(dep_active.contains("native"));
1647+
assert!(
1648+
dep_active.contains("experimental"),
1649+
"non-prod should honour dev-dependency shorthand = all features"
1650+
);
1651+
1652+
// Prod: dev edge is NOT traversed → only the `dependencies` restriction applies.
1653+
let active_prod = super::compute_active_features(&packages, None, /* prod */ true).unwrap();
1654+
let dep_active_prod = active_prod.get("dep").unwrap();
1655+
assert!(dep_active_prod.contains("native"));
1656+
assert!(
1657+
!dep_active_prod.contains("experimental"),
1658+
"prod must not inherit the dev-dependency shorthand's all-features request"
1659+
);
15711660
}
15721661
}

0 commit comments

Comments
 (0)