Skip to content

Commit 6de16a6

Browse files
jfrolichclaude
andcommitted
Rewatch: honour explicit empty features list on dependencies
compute_active_features conflated "no consumer edge found" with "consumer requested empty". A qualified dependency with `"features": []` would land in the same state as an unreached dep (requested set empty, no `any_all_request`) and trip the fallback that forced all features active. That made it impossible to express an untagged-only dependency build. Track `saw_consumer_entry` explicitly. The all-features fallback now fires only when no consumer edge was observed; an empty requested set from an explicit `"features": []` is honoured as "no feature-gated dirs". Regression test covers the explicit-empty case. Docs updated. 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 bde5469 commit 6de16a6

5 files changed

Lines changed: 78 additions & 6 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
#### :rocket: New Feature
2424

2525
- Rewatch: add `--prod` flag to `build`, `watch`, and `clean` to skip dev-dependencies and dev sources (`"type": "dev"`), enabling builds in environments where dev packages aren't installed (e.g. after `pnpm install --prod`). https://github.com/rescript-lang/rescript/pull/8347
26+
- Rewatch: feature-gated source directories. Tag a source entry with `"feature": "<name>"` and select with `--features a,b` (or per-dep in `dependencies` / `dev-dependencies`) to include optional slices of a package's source tree at build time. Top-level `features` map supports transitive implications. https://github.com/rescript-lang/rescript/pull/8379
2627
- Add `Dict.assignMany`, `Dict.concat`, `Dict.concatMany`, `Dict.concatAll`, `Array.concatAll` to the stdlib. https://github.com/rescript-lang/rescript/pull/8364
2728
- Implement `for...of` and `for await...of` loops. https://github.com/rescript-lang/rescript/pull/7887
2829
- Add support for dict spreads: `dict{...foo, "bar": 2, ...qux}`. https://github.com/rescript-lang/rescript/pull/8369

rewatch/Features.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ When consuming another ReScript package that uses features, switch the entry in
6565
Rules:
6666

6767
- **Shorthand (`"@plain/dep"`)** — the consumer wants every feature of that dependency. This is the existing behavior; nothing changes for configs that don't opt into features.
68-
- **Object with `features`** — the consumer restricts the dependency to the listed features (and whatever they transitively imply through the dependency's own `features` map).
68+
- **Object with `features`** — the consumer restricts the dependency to the listed features (and whatever they transitively imply through the dependency's own `features` map). An explicit empty list (`"features": []`) means "only untagged source dirs, no feature-gated code".
6969
- **Object without `features`** — equivalent to the shorthand. All features active.
7070

7171
When the same dependency is referenced by multiple consumers with different feature sets, the union of requests wins. If any consumer asks for all features, the dependency builds with all of its features. Features are always additive — enabling more features never removes modules, so the union is always safe.

rewatch/src/build/packages.rs

Lines changed: 67 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -780,6 +780,7 @@ pub fn compute_active_features(
780780

781781
for (package_name, package) in packages {
782782
let mut any_all_request = false;
783+
let mut saw_consumer_entry = false;
783784
let mut requested: AHashSet<String> = AHashSet::new();
784785

785786
if package.is_root {
@@ -795,6 +796,7 @@ pub fn compute_active_features(
795796
if dep.name() != package_name {
796797
continue;
797798
}
799+
saw_consumer_entry = true;
798800
match dep.features() {
799801
None => any_all_request = true,
800802
Some(list) => requested.extend(list.iter().cloned()),
@@ -811,6 +813,7 @@ pub fn compute_active_features(
811813
if dep.name() != package_name {
812814
continue;
813815
}
816+
saw_consumer_entry = true;
814817
match dep.features() {
815818
None => any_all_request = true,
816819
Some(list) => requested.extend(list.iter().cloned()),
@@ -819,9 +822,10 @@ pub fn compute_active_features(
819822
}
820823
}
821824

822-
// Defensive: a package with no consumers (shouldn't happen in practice, but don't
823-
// accidentally strip everything) is treated as all-features.
824-
if !any_all_request && requested.is_empty() {
825+
// Defensive: if no consumer edge was found at all, keep all features. An empty
826+
// `requested` set by itself is a *valid* request (`"features": []` means
827+
// "untagged only"), so only fall back when we truly observed no entries.
828+
if !saw_consumer_entry {
825829
any_all_request = true;
826830
}
827831
}
@@ -1622,6 +1626,7 @@ mod test {
16221626
modules: None,
16231627
path: PathBuf::from("."),
16241628
dirs: None,
1629+
gentype_dirs: None,
16251630
is_local_dep: true,
16261631
is_root: true,
16271632
},
@@ -1658,4 +1663,63 @@ mod test {
16581663
"prod must not inherit the dev-dependency shorthand's all-features request"
16591664
);
16601665
}
1666+
1667+
#[test]
1668+
fn compute_active_features_honours_explicit_empty_features_list() {
1669+
// Regression: `{"name": "dep", "features": []}` must be honoured as "no features, only
1670+
// untagged dirs". Previously the fallback for "no consumer edges" fired when `requested`
1671+
// was empty, forcing all features on even though the consumer explicitly asked for none.
1672+
let mut packages: AHashMap<String, super::Package> = AHashMap::new();
1673+
1674+
let mut root_config = config::tests::create_config(config::tests::CreateConfigArgs {
1675+
name: "root".to_string(),
1676+
bs_deps: vec![],
1677+
build_dev_deps: vec![],
1678+
allowed_dependents: None,
1679+
path: PathBuf::from("./rescript.json"),
1680+
});
1681+
root_config.sources = Some(config::OneOrMore::Single(config::Source::Shorthand(
1682+
"src".to_string(),
1683+
)));
1684+
root_config.dependencies = Some(vec![config::Dependency::Qualified(config::QualifiedDependency {
1685+
name: "dep".to_string(),
1686+
features: Some(vec![]),
1687+
})]);
1688+
packages.insert(
1689+
"root".to_string(),
1690+
super::Package {
1691+
name: "root".to_string(),
1692+
config: root_config,
1693+
source_folders: AHashSet::new(),
1694+
source_files: None,
1695+
namespace: super::Namespace::NoNamespace,
1696+
modules: None,
1697+
path: PathBuf::from("."),
1698+
dirs: None,
1699+
gentype_dirs: None,
1700+
is_local_dep: true,
1701+
is_root: true,
1702+
},
1703+
);
1704+
1705+
packages.insert(
1706+
"dep".to_string(),
1707+
root_package_with_features(
1708+
None,
1709+
vec![
1710+
("src", None),
1711+
("src-native", Some("native")),
1712+
("src-experimental", Some("experimental")),
1713+
],
1714+
),
1715+
);
1716+
packages.get_mut("dep").unwrap().is_root = false;
1717+
1718+
let active = super::compute_active_features(&packages, None, false).unwrap();
1719+
let dep_active = active.get("dep").unwrap();
1720+
assert!(
1721+
dep_active.is_empty(),
1722+
"an explicit empty features list should activate no feature-tagged dirs, got {dep_active:?}"
1723+
);
1724+
}
16611725
}

rewatch/src/config.rs

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1661,7 +1661,10 @@ pub mod tests {
16611661
"#;
16621662

16631663
let config = Config::new_from_json_string(json).expect("a valid json string");
1664-
assert_eq!(config.dependencies, Some(vec!["@testrepo/main".to_string()]));
1664+
assert_eq!(
1665+
config.dependencies,
1666+
Some(vec![Dependency::Shorthand("@testrepo/main".to_string())])
1667+
);
16651668
assert_eq!(config.get_deprecations(), [DeprecationWarning::BsDependencies]);
16661669
}
16671670

@@ -1714,7 +1717,10 @@ pub mod tests {
17141717
"#;
17151718

17161719
let config = Config::new_from_json_string(json).expect("a valid json string");
1717-
assert_eq!(config.dev_dependencies, Some(vec!["@testrepo/main".to_string()]));
1720+
assert_eq!(
1721+
config.dev_dependencies,
1722+
Some(vec![Dependency::Shorthand("@testrepo/main".to_string())])
1723+
);
17181724
assert_eq!(config.get_deprecations(), [DeprecationWarning::BsDevDependencies]);
17191725
}
17201726

tests/build_tests/cli_help/input.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ const buildHelp =
4444
" -a, --after-build <AFTER_BUILD> Run an additional command after build. E.g., play a sound or run a test suite when done compiling\n" +
4545
" -q, --quiet... Decrease logging verbosity\n" +
4646
' --warn-error <WARN_ERROR> Override warning configuration from rescript.json. Example: --warn-error "+3+8+11+12+26+27+31+32+33+34+35+39+44+45+110"\n' +
47+
" --features <FEATURES> Restrict the current package to a comma-separated set of features. Only source directories tagged with one of these features (plus untagged ones, and features they transitively imply through the top-level `features` map) are compiled. Omit the flag to build with all features active. Example: --features native,experimental\n" +
4748
" -n, --no-timing [<NO_TIMING>] Disable output timing [default: false] [possible values: true, false]\n" +
4849
' --prod Skip dev-dependencies and dev sources (type: "dev")\n' +
4950
" -h, --help Print help\n";

0 commit comments

Comments
 (0)