diff --git a/CHANGELOG.md b/CHANGELOG.md index d92fa1dfcd..b5f2895636 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,7 @@ #### :rocket: New Feature - 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 +- Rewatch: feature-gated source directories. Tag a source entry with `"feature": ""` 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 - Add `Dict.assignMany`, `Dict.concat`, `Dict.concatMany`, `Dict.concatAll`, `Array.concatAll` to the stdlib. https://github.com/rescript-lang/rescript/pull/8364 - Implement `for...of` and `for await...of` loops. https://github.com/rescript-lang/rescript/pull/7887 - Add support for dict spreads: `dict{...foo, "bar": 2, ...qux}`. https://github.com/rescript-lang/rescript/pull/8369 diff --git a/rewatch/CompilerConfigurationSpec.md b/rewatch/CompilerConfigurationSpec.md index 49a14b01f5..3b21be0d33 100644 --- a/rewatch/CompilerConfigurationSpec.md +++ b/rewatch/CompilerConfigurationSpec.md @@ -13,7 +13,10 @@ This document contains a list of all config parameters with remarks, and whether | sources | array of Source | | [x] | | ignored-dirs | array of string | | [_] | | dependencies | array of string | | [x] | +| dependencies | array of Dependency | See [Features.md](./Features.md). rewatch extension. | [x] | | dev-dependencies | array of string | | [x] | +| dev-dependencies | array of Dependency | See [Features.md](./Features.md). rewatch extension. | [x] | +| features | map of string to array | See [Features.md](./Features.md). rewatch extension. | [x] | | generators | array of Rule-Generator | | [_] | | cut-generators | boolean | | [_] | | jsx | JSX | | [x] | @@ -34,10 +37,11 @@ This document contains a list of all config parameters with remarks, and whether ### Source -| Parameter | JSON type | Remark | Implemented? | -| ---------------- | ------------------------ | ------ | :----------: | -| dir | string | | [x] | -| type | "dev" | | [x] | +| Parameter | JSON type | Remark | Implemented? | +| ---------------- | ------------------------ | ------------------------------------------------------- | :----------: | +| dir | string | | [x] | +| type | "dev" | | [x] | +| feature | string | See [Features.md](./Features.md). rewatch extension. | [x] | | files | array of string | | [_] | | files | File-Object | | [_] | | generators | array of Build-Generator | | [_] | diff --git a/rewatch/Features.md b/rewatch/Features.md new file mode 100644 index 0000000000..ac20e2b875 --- /dev/null +++ b/rewatch/Features.md @@ -0,0 +1,112 @@ +# Features + +Features let a package declare optional parts of its source tree that can be included or excluded at build time. Use them to ship a library with optional backends, experimental modules, or platform-specific code without paying the compile cost when a consumer doesn't need them. + +Features are a `rewatch` extension and are not part of the legacy `bsb` build-configuration spec. + +## Tagging a source directory + +Add a `feature` property to any entry in `sources`. The directory is included in the build only when the feature is active for that package. + +```json +{ + "name": "@example/lib", + "sources": [ + { "dir": "src" }, + { "dir": "src-native", "feature": "native" }, + { "dir": "src-experimental", "feature": "experimental" } + ] +} +``` + +Untagged source directories (no `feature`) are always compiled. A tagged source's `feature` cascades down into nested `subdirs`: a child that doesn't declare its own `feature` inherits the parent's. + +## Declaring feature relationships + +A top-level `features` map declares names and optional implications. Listing a feature here is only required when you want one feature to imply another; leaf features can stay undeclared and still work as source-dir tags. + +```json +{ + "features": { + "full": ["native", "experimental"] + } +} +``` + +With the above, requesting `full` transitively enables `native` and `experimental`. Cycles (e.g. `a -> b -> a`) are rejected at build time with a clear error. + +## Selecting features on the command line + +When you run `rewatch build` or `rewatch watch`, pass `--features` to restrict compilation to a specific set. Without the flag, every feature is active and the whole source tree compiles: + +``` +rewatch build # all features active (default) +rewatch build --features native # only untagged + native +rewatch build --features native,full # multiple features; also expands `full` +``` + +The CLI flag applies only to the **current package** — the one you're building from. It does not flow down to dependencies; each dependency's active feature set comes from its consumer declarations (see below). + +Passing an empty value (`--features ""` or `--features ,`) is rejected. Omit the flag to mean "all features". + +## Restricting a dependency's features + +When consuming another ReScript package that uses features, switch the entry in `dependencies` or `dev-dependencies` from the shorthand string to an object form and list which features you want: + +```json +{ + "dependencies": [ + "@plain/dep", + { "name": "@example/lib", "features": ["native"] } + ] +} +``` + +Rules: + +- **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. +- **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". +- **Object without `features`** — equivalent to the shorthand. All features active. + +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. + +## Interaction with other flags + +- **`type: "dev"` and `--prod`** are orthogonal to features. A source directory may declare both `type: "dev"` and a `feature`; it will only build when both filters pass (not in `--prod`, and the feature is active). +- **`rewatch clean`** ignores `--features` and always cleans the full set of build artifacts across every feature-gated directory. This keeps `clean` predictable regardless of which features happen to be active. + +## How incremental builds handle feature changes + +Toggling a feature off between builds removes its source files from the build's view. The next `rewatch build` sees the shrunken file set and cleans up the corresponding artifacts (`.mjs`, `.cmj`, etc.) through the same diff mechanism that handles deleted source files. + +For `rewatch watch`, a change to `features` in `rescript.json` triggers a full rebuild that recomputes the active set and re-registers watches on the active source directories. The CLI `--features` flag is evaluated once at watcher start; to change it you must restart the watcher. + +## Validating feature names + +- Unknown feature names in CLI input or source tags are accepted as leaf features (they simply match nothing unless a source directory is tagged with that exact name). +- Cycles in the top-level `features` map are a hard error that names the cycle participants. + +## Example + +```json +{ + "name": "@example/lib", + "sources": [ + { "dir": "src" }, + { "dir": "src-native", "feature": "native" }, + { "dir": "src-web", "feature": "web" }, + { "dir": "src-experimental", "feature": "experimental" } + ], + "features": { + "all-backends": ["native", "web"] + }, + "dependencies": [ + "@plain/dep", + { "name": "@other/heavy", "features": ["native"] } + ] +} +``` + +- `rewatch build` at `@example/lib` — compiles every source dir; `@other/heavy` builds with just its `native` feature because that's all the consumer requested; `@plain/dep` builds with all of its features. +- `rewatch build --features all-backends` — compiles `src`, `src-native`, `src-web`; skips `src-experimental`. +- `rewatch build --features experimental` — compiles `src`, `src-experimental`; skips the backends. diff --git a/rewatch/src/build.rs b/rewatch/src/build.rs index 497559b181..53d61de980 100644 --- a/rewatch/src/build.rs +++ b/rewatch/src/build.rs @@ -165,6 +165,7 @@ pub fn get_compiler_info(project_context: &ProjectContext) -> Result, filter: &Option, @@ -173,12 +174,13 @@ pub fn initialize_build( plain_output: bool, warn_error: Option, prod: bool, + features: Option>, ) -> Result { let project_context = ProjectContext::new(path)?; let compiler = get_compiler_info(&project_context)?; let timing_clean_start = Instant::now(); - let packages = packages::make(filter, &project_context, show_progress, prod)?; + let packages = packages::make(filter, &project_context, show_progress, prod, features.as_ref())?; let compiler_check = verify_compiler_info(&packages, &compiler); @@ -192,6 +194,7 @@ pub fn initialize_build( packages, compiler, warn_error, + features, ); packages::parse_packages(&mut build_state)?; @@ -589,6 +592,7 @@ pub fn build( plain_output: bool, warn_error: Option, prod: bool, + features: Option>, ) -> Result { let default_timing: Option = if no_timing { Some(std::time::Duration::new(0.0 as u64, 0.0 as u32)) @@ -604,6 +608,7 @@ pub fn build( plain_output, warn_error, prod, + features, ) .with_context(|| "Could not initialize build")?; diff --git a/rewatch/src/build/build_types.rs b/rewatch/src/build/build_types.rs index 75bed1f10c..4d1f3f8bf0 100644 --- a/rewatch/src/build/build_types.rs +++ b/rewatch/src/build/build_types.rs @@ -126,6 +126,9 @@ pub struct BuildCommandState { pub build_state: BuildState, // Command-line --warn-error flag override (takes precedence over rescript.json config) pub warn_error_override: Option, + // Command-line --features override. `None` means all features are active; `Some(list)` + // restricts the root package to those features (and whatever they transitively imply). + pub features: Option>, } #[derive(Debug, Clone)] @@ -177,11 +180,13 @@ impl BuildCommandState { packages: AHashMap, compiler: CompilerInfo, warn_error_override: Option, + features: Option>, ) -> Self { Self { root_folder, build_state: BuildState::new(project_context, packages, compiler), warn_error_override, + features, } } @@ -189,6 +194,10 @@ impl BuildCommandState { self.warn_error_override.clone() } + pub fn get_features(&self) -> Option> { + self.features.clone() + } + pub fn module_name_package_pairs(&self) -> Vec<(String, String)> { self.build_state .modules diff --git a/rewatch/src/build/clean.rs b/rewatch/src/build/clean.rs index 73e77987dd..2d5b3d1342 100644 --- a/rewatch/src/build/clean.rs +++ b/rewatch/src/build/clean.rs @@ -335,7 +335,10 @@ pub fn cleanup_after_build(build_state: &BuildCommandState) { pub fn clean(path: &Path, show_progress: bool, plain_output: bool, prod: bool) -> Result<()> { let project_context = ProjectContext::new(path)?; let compiler_info = build::get_compiler_info(&project_context)?; - let packages = packages::make(&None, &project_context, show_progress, prod)?; + // `clean` always acts on the full set of source directories regardless of which features are + // active. We explicitly pass `None` so every tagged source folder is included and its + // artifacts can be removed, even for features the user hasn't enabled for this build. + let packages = packages::make(&None, &project_context, show_progress, prod, None)?; let timing_clean_compiler_assets = Instant::now(); if !plain_output && show_progress { diff --git a/rewatch/src/build/compile.rs b/rewatch/src/build/compile.rs index c646309ef7..27e07c5663 100644 --- a/rewatch/src/build/compile.rs +++ b/rewatch/src/build/compile.rs @@ -561,7 +561,10 @@ pub fn compiler_args( .dependencies .iter() .flatten() - .filter_map(|name| pkgs.get(name).map(|pkg| (name.clone(), pkg.path.clone()))) + .filter_map(|dep| { + let name = dep.name(); + pkgs.get(name).map(|pkg| (name.to_string(), pkg.path.clone())) + }) .collect::>() }); resolved.unwrap_or_default() @@ -684,20 +687,16 @@ fn get_dependency_paths( packages: &Option<&AHashMap>, is_file_type_dev: bool, ) -> Vec { - let normal_deps = config - .dependencies - .clone() - .unwrap_or_default() + let normal_deps: Vec = config + .get_dependency_names() .into_iter() .map(DependentPackage::Normal) .collect(); // We can only access dev dependencies for source_files that are marked as "type":"dev" - let dev_deps = if is_file_type_dev { + let dev_deps: Vec = if is_file_type_dev { config - .dev_dependencies - .clone() - .unwrap_or_default() + .get_dev_dependency_names() .into_iter() .map(DependentPackage::Dev) .collect() diff --git a/rewatch/src/build/deps.rs b/rewatch/src/build/deps.rs index c89f5898a0..b80fc4c99a 100644 --- a/rewatch/src/build/deps.rs +++ b/rewatch/src/build/deps.rs @@ -37,12 +37,9 @@ fn get_dep_modules( // Get the list of allowed dependency packages for this package let allowed_dependencies: AHashSet = package .config - .dependencies - .as_ref() - .unwrap_or(&vec![]) - .iter() - .chain(package.config.dev_dependencies.as_ref().unwrap_or(&vec![]).iter()) - .cloned() + .get_dependency_names() + .into_iter() + .chain(package.config.get_dev_dependency_names()) .collect(); deps.iter() diff --git a/rewatch/src/build/packages.rs b/rewatch/src/build/packages.rs index 2584abf190..f606f152cb 100644 --- a/rewatch/src/build/packages.rs +++ b/rewatch/src/build/packages.rs @@ -239,7 +239,12 @@ fn get_source_dirs(source: config::Source, sub_path: Option) -> AHashSe .unwrap_or(vec![]) .par_iter() .map(|subsource| { - get_source_dirs(subsource.set_type(source.get_type()), Some(sub_path.to_owned())) + get_source_dirs( + subsource + .set_type(source.get_type()) + .set_feature(source.get_feature()), + Some(sub_path.to_owned()), + ) }) .collect::>>() .into_iter() @@ -298,14 +303,11 @@ fn read_dependencies( is_local_dep: bool, prod: bool, ) -> Vec { - let mut dependencies = package_config.dependencies.to_owned().unwrap_or_default(); + let mut dependencies: Vec = package_config.get_dependency_names(); // Concatenate dev dependencies if is_local_dep is true and not in prod mode - if is_local_dep - && !prod - && let Some(dev_deps) = package_config.dev_dependencies.to_owned() - { - dependencies.extend(dev_deps); + if is_local_dep && !prod { + dependencies.extend(package_config.get_dev_dependency_names()); } dependencies @@ -753,6 +755,94 @@ fn collect_gentype_source_dirs(package: &Package) -> Vec { out } +/// Computes the active feature set for every package. Rules: +/// +/// - The root package's active set comes from the `--features` CLI flag. When the flag is +/// absent, all features declared in the root config are active (default = all). +/// - For every other package, we scan every consumer (any package that lists this one in its +/// `dependencies` or `dev-dependencies`). A shorthand entry (or an object entry without a +/// `features` field) means the consumer wants all features of the dep. An entry with a +/// `features` list means the consumer wants exactly those. When any consumer requests "all" +/// or no consumer requests anything, the dep builds with all of its declared features. +/// Otherwise we take the union of specific feature requests and pass it through the dep's +/// own `features` map for transitive expansion. +/// +/// `dev-dependencies` only contribute when that edge is actually traversed by +/// `read_dependencies` — i.e. when the consumer is a local dep and we're not in `--prod`. This +/// keeps dev-only feature requests (e.g. a shorthand `dev-dependencies` entry that would flip +/// everything to "all features") from leaking into production builds. +pub fn compute_active_features( + packages: &AHashMap, + cli_features: Option<&Vec>, + prod: bool, +) -> Result>> { + let mut result: AHashMap> = AHashMap::new(); + + for (package_name, package) in packages { + let mut any_all_request = false; + let mut saw_consumer_entry = false; + let mut requested: AHashSet = AHashSet::new(); + + if package.is_root { + match cli_features { + None => any_all_request = true, + Some(list) => requested.extend(list.iter().cloned()), + } + } else { + for consumer in packages.values() { + // `dependencies` always contribute. + if let Some(deps) = consumer.config.dependencies.as_ref() { + for dep in deps { + if dep.name() != package_name { + continue; + } + saw_consumer_entry = true; + match dep.features() { + None => any_all_request = true, + Some(list) => requested.extend(list.iter().cloned()), + } + } + } + // `dev-dependencies` only contribute when that edge is actually traversed: + // local consumer, not `--prod`. Matches `read_dependencies`. + if consumer.is_local_dep + && !prod + && let Some(dev_deps) = consumer.config.dev_dependencies.as_ref() + { + for dep in dev_deps { + if dep.name() != package_name { + continue; + } + saw_consumer_entry = true; + match dep.features() { + None => any_all_request = true, + Some(list) => requested.extend(list.iter().cloned()), + } + } + } + } + + // Defensive: if no consumer edge was found at all, keep all features. An empty + // `requested` set by itself is a *valid* request (`"features": []` means + // "untagged only"), so only fall back when we truly observed no entries. + if !saw_consumer_entry { + any_all_request = true; + } + } + + let closure = if any_all_request { + package.config.collect_declared_features() + } else { + config::resolve_active_features(&requested, package.config.features.as_ref()) + .map_err(|e| anyhow!("Invalid features for package '{}': {}", package_name, e))? + }; + + result.insert(package_name.clone(), closure); + } + + Ok(result) +} + /// Make turns a folder, that should contain a config, into a tree of Packages. /// It does so in two steps: /// 1. Get all the packages parsed, and take all the source folders from the config @@ -765,8 +855,21 @@ pub fn make( project_context: &ProjectContext, show_progress: bool, prod: bool, + cli_features: Option<&Vec>, ) -> Result> { - let map = read_packages(project_context, show_progress, prod)?; + let mut map = read_packages(project_context, show_progress, prod)?; + + let active_features = compute_active_features(&map, cli_features, prod)?; + + // Drop source directories whose feature tag is not in the package's active set. + // Untagged source dirs remain; they're included regardless of the feature selection. + for (package_name, package) in map.iter_mut() { + if let Some(active) = active_features.get(package_name) { + package + .source_folders + .retain(|source| source.is_feature_enabled(active)); + } + } /* Once we have the deduplicated packages, we can add the source files for each - to minimize * the IO */ @@ -1081,8 +1184,8 @@ pub fn validate_packages_dependencies(packages: &AHashMap) -> b let mut detected_unallowed_dependencies: AHashMap = AHashMap::new(); for (package_name, package) in packages { - let dependencies = &package.config.dependencies.to_owned().unwrap_or(vec![]); - let dev_dependencies = &package.config.dev_dependencies.to_owned().unwrap_or(vec![]); + let dependencies = &package.config.get_dependency_names(); + let dev_dependencies = &package.config.get_dev_dependency_names(); [ ("dependencies", dependencies), @@ -1354,4 +1457,269 @@ mod test { write_pkg_json(temp_dir.path(), r#"{"name":"x"}"#); assert_eq!(read_issue_tracker_url(temp_dir.path()), None); } + + fn root_package_with_features( + features_map: Option>>, + tagged_sources: Vec<(&str, Option<&str>)>, + ) -> super::Package { + let sources: Vec = tagged_sources + .into_iter() + .map(|(dir, feature)| { + config::Source::Qualified(config::PackageSource { + dir: dir.to_string(), + subdirs: None, + type_: None, + feature: feature.map(|s| s.to_string()), + }) + }) + .collect(); + let mut config = config::tests::create_config(config::tests::CreateConfigArgs { + name: "root".to_string(), + bs_deps: vec![], + build_dev_deps: vec![], + allowed_dependents: None, + path: PathBuf::from("./rescript.json"), + }); + config.sources = Some(config::OneOrMore::Multiple(sources)); + config.features = features_map; + super::Package { + name: "root".to_string(), + config, + source_folders: AHashSet::new(), + source_files: None, + namespace: super::Namespace::NoNamespace, + modules: None, + path: PathBuf::from("."), + dirs: None, + gentype_dirs: None, + is_local_dep: true, + is_root: true, + } + } + + #[test] + fn compute_active_features_returns_all_when_cli_absent() { + let mut packages: AHashMap = AHashMap::new(); + let mut features_map = std::collections::HashMap::new(); + features_map.insert("full".to_string(), vec!["native".to_string()]); + packages.insert( + "root".to_string(), + root_package_with_features( + Some(features_map), + vec![("src", None), ("src-native", Some("native"))], + ), + ); + + let active = super::compute_active_features(&packages, None, false).unwrap(); + let root = active.get("root").unwrap(); + assert!(root.contains("native")); + assert!(root.contains("full")); + } + + #[test] + fn compute_active_features_honours_cli_restriction_and_expands_transitive() { + let mut packages: AHashMap = AHashMap::new(); + let mut features_map = std::collections::HashMap::new(); + features_map.insert( + "full".to_string(), + vec!["native".to_string(), "experimental".to_string()], + ); + packages.insert( + "root".to_string(), + root_package_with_features( + Some(features_map), + vec![ + ("src", None), + ("src-native", Some("native")), + ("src-experimental", Some("experimental")), + ], + ), + ); + + let cli = vec!["full".to_string()]; + let active = super::compute_active_features(&packages, Some(&cli), false).unwrap(); + let root = active.get("root").unwrap(); + assert!(root.contains("full")); + assert!(root.contains("native")); + assert!(root.contains("experimental")); + } + + #[test] + fn compute_active_features_dep_uses_consumer_restriction() { + // Root consumes a dep with a restricted feature set. + let mut packages: AHashMap = AHashMap::new(); + + let mut root_config = config::tests::create_config(config::tests::CreateConfigArgs { + name: "root".to_string(), + bs_deps: vec![], + build_dev_deps: vec![], + allowed_dependents: None, + path: PathBuf::from("./rescript.json"), + }); + root_config.sources = Some(config::OneOrMore::Single(config::Source::Shorthand( + "src".to_string(), + ))); + root_config.dependencies = Some(vec![config::Dependency::Qualified(config::QualifiedDependency { + name: "dep".to_string(), + features: Some(vec!["native".to_string()]), + })]); + packages.insert( + "root".to_string(), + super::Package { + name: "root".to_string(), + config: root_config, + source_folders: AHashSet::new(), + source_files: None, + namespace: super::Namespace::NoNamespace, + modules: None, + path: PathBuf::from("."), + dirs: None, + gentype_dirs: None, + is_local_dep: true, + is_root: true, + }, + ); + + packages.insert( + "dep".to_string(), + root_package_with_features(None, vec![("src", None), ("src-native", Some("native"))]), + ); + // Flip the dep's is_root flag to false since root_package_with_features sets it to true. + packages.get_mut("dep").unwrap().is_root = false; + + let active = super::compute_active_features(&packages, None, false).unwrap(); + let dep_active = active.get("dep").unwrap(); + assert!(dep_active.contains("native")); + } + + #[test] + fn compute_active_features_prod_ignores_dev_dependency_feature_requests() { + // Regression: a dep that appears in both `dependencies` (restricted to `native`) and + // `dev-dependencies` (shorthand = all features). In `--prod` the dev edge isn't + // traversed by `read_dependencies`, so its broader feature request must not flip the + // dep's active set to "all features". Only the non-dev restriction applies. + let mut packages: AHashMap = AHashMap::new(); + + let mut root_config = config::tests::create_config(config::tests::CreateConfigArgs { + name: "root".to_string(), + bs_deps: vec![], + build_dev_deps: vec![], + allowed_dependents: None, + path: PathBuf::from("./rescript.json"), + }); + root_config.sources = Some(config::OneOrMore::Single(config::Source::Shorthand( + "src".to_string(), + ))); + root_config.dependencies = Some(vec![config::Dependency::Qualified(config::QualifiedDependency { + name: "dep".to_string(), + features: Some(vec!["native".to_string()]), + })]); + root_config.dev_dependencies = Some(vec![config::Dependency::Shorthand("dep".to_string())]); + packages.insert( + "root".to_string(), + super::Package { + name: "root".to_string(), + config: root_config, + source_folders: AHashSet::new(), + source_files: None, + namespace: super::Namespace::NoNamespace, + modules: None, + path: PathBuf::from("."), + dirs: None, + gentype_dirs: None, + is_local_dep: true, + is_root: true, + }, + ); + + packages.insert( + "dep".to_string(), + root_package_with_features( + None, + vec![ + ("src", None), + ("src-native", Some("native")), + ("src-experimental", Some("experimental")), + ], + ), + ); + packages.get_mut("dep").unwrap().is_root = false; + + // Non-prod: dev edge IS traversed → all features active. + let active = super::compute_active_features(&packages, None, /* prod */ false).unwrap(); + let dep_active = active.get("dep").unwrap(); + assert!(dep_active.contains("native")); + assert!( + dep_active.contains("experimental"), + "non-prod should honour dev-dependency shorthand = all features" + ); + + // Prod: dev edge is NOT traversed → only the `dependencies` restriction applies. + let active_prod = super::compute_active_features(&packages, None, /* prod */ true).unwrap(); + let dep_active_prod = active_prod.get("dep").unwrap(); + assert!(dep_active_prod.contains("native")); + assert!( + !dep_active_prod.contains("experimental"), + "prod must not inherit the dev-dependency shorthand's all-features request" + ); + } + + #[test] + fn compute_active_features_honours_explicit_empty_features_list() { + // Regression: `{"name": "dep", "features": []}` must be honoured as "no features, only + // untagged dirs". Previously the fallback for "no consumer edges" fired when `requested` + // was empty, forcing all features on even though the consumer explicitly asked for none. + let mut packages: AHashMap = AHashMap::new(); + + let mut root_config = config::tests::create_config(config::tests::CreateConfigArgs { + name: "root".to_string(), + bs_deps: vec![], + build_dev_deps: vec![], + allowed_dependents: None, + path: PathBuf::from("./rescript.json"), + }); + root_config.sources = Some(config::OneOrMore::Single(config::Source::Shorthand( + "src".to_string(), + ))); + root_config.dependencies = Some(vec![config::Dependency::Qualified(config::QualifiedDependency { + name: "dep".to_string(), + features: Some(vec![]), + })]); + packages.insert( + "root".to_string(), + super::Package { + name: "root".to_string(), + config: root_config, + source_folders: AHashSet::new(), + source_files: None, + namespace: super::Namespace::NoNamespace, + modules: None, + path: PathBuf::from("."), + dirs: None, + gentype_dirs: None, + is_local_dep: true, + is_root: true, + }, + ); + + packages.insert( + "dep".to_string(), + root_package_with_features( + None, + vec![ + ("src", None), + ("src-native", Some("native")), + ("src-experimental", Some("experimental")), + ], + ), + ); + packages.get_mut("dep").unwrap().is_root = false; + + let active = super::compute_active_features(&packages, None, false).unwrap(); + let dep_active = active.get("dep").unwrap(); + assert!( + dep_active.is_empty(), + "an explicit empty features list should activate no feature-tagged dirs, got {dep_active:?}" + ); + } } diff --git a/rewatch/src/cli.rs b/rewatch/src/cli.rs index 2a7fc40100..dd680c7d74 100644 --- a/rewatch/src/cli.rs +++ b/rewatch/src/cli.rs @@ -202,6 +202,40 @@ pub struct WarnErrorArg { pub warn_error: Option, } +fn validate_features_string(s: &str) -> Result { + let trimmed_parts: Vec<&str> = s.split(',').map(str::trim).filter(|p| !p.is_empty()).collect(); + if trimmed_parts.is_empty() { + return Err( + "--features must not be empty. Omit the flag to build with all features active.".to_string(), + ); + } + Ok(s.to_string()) +} + +#[derive(Args, Debug, Clone)] +pub struct FeaturesArg { + /// 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 + #[arg(long = "features", value_parser = validate_features_string)] + pub features: Option, +} + +impl FeaturesArg { + /// Splits the raw `--features` value into a list of feature names. Returns `None` when the + /// flag was omitted. + pub fn parsed(&self) -> Option> { + self.features.as_ref().map(|s| { + s.split(',') + .map(|p| p.trim().to_string()) + .filter(|p| !p.is_empty()) + .collect() + }) + } +} + #[derive(Args, Debug, Clone)] pub struct BuildArgs { #[command(flatten)] @@ -216,6 +250,9 @@ pub struct BuildArgs { #[command(flatten)] pub warn_error: WarnErrorArg, + #[command(flatten)] + pub features: FeaturesArg, + /// Disable output timing #[arg(short, long, default_value_t = false, num_args = 0..=1)] pub no_timing: bool, @@ -422,6 +459,79 @@ mod tests { let err = parse_with_default_from(&args).expect_err("expected clap to report invalid utf8"); assert_eq!(err.kind(), ErrorKind::InvalidUtf8); } + + // --features flag tests. + #[test] + fn build_features_flag_is_parsed() { + let cli = parse(&["rescript", "build", "--features", "native,experimental"]) + .expect("expected build command"); + match cli.command { + Command::Build(build_args) => assert_eq!( + build_args.features.parsed(), + Some(vec!["native".to_string(), "experimental".to_string()]) + ), + other => panic!("expected build command, got {other:?}"), + } + } + + #[test] + fn build_features_flag_defaults_to_none() { + let cli = parse(&["rescript", "build"]).expect("expected build command"); + match cli.command { + Command::Build(build_args) => assert!(build_args.features.parsed().is_none()), + other => panic!("expected build command, got {other:?}"), + } + } + + #[test] + fn build_features_flag_rejects_empty_string() { + let err = parse(&["rescript", "build", "--features", ""]) + .expect_err("expected empty --features to fail parsing"); + let rendered = err.to_string(); + assert!( + rendered.contains("must not be empty"), + "unexpected error: {rendered}" + ); + } + + #[test] + fn watch_features_flag_is_parsed() { + let cli = parse(&["rescript", "watch", "--features", "native"]).expect("expected watch command"); + match cli.command { + Command::Watch(watch_args) => { + assert_eq!(watch_args.features.parsed(), Some(vec!["native".to_string()])) + } + other => panic!("expected watch command, got {other:?}"), + } + } + + #[test] + fn features_flag_round_trips_through_build_to_watch_args() { + let cli = parse(&["rescript", "build", "--features", "a,b"]).expect("expected build command"); + match cli.command { + Command::Build(build_args) => { + let watch_args: WatchArgs = build_args.into(); + assert_eq!( + watch_args.features.parsed(), + Some(vec!["a".to_string(), "b".to_string()]) + ); + } + other => panic!("expected build command, got {other:?}"), + } + } + + #[test] + fn build_features_flag_strips_whitespace() { + let cli = parse(&["rescript", "build", "--features", " native , experimental "]) + .expect("expected build command"); + match cli.command { + Command::Build(build_args) => assert_eq!( + build_args.features.parsed(), + Some(vec!["native".to_string(), "experimental".to_string()]) + ), + other => panic!("expected build command, got {other:?}"), + } + } } #[derive(Args, Clone, Debug)] @@ -438,6 +548,9 @@ pub struct WatchArgs { #[command(flatten)] pub warn_error: WarnErrorArg, + #[command(flatten)] + pub features: FeaturesArg, + /// Clear terminal screen before each rebuild in interactive watch mode. #[arg(long, default_value_t = false)] pub clear_screen: bool, @@ -454,6 +567,7 @@ impl From for WatchArgs { filter: build_args.filter, after_build: build_args.after_build, warn_error: build_args.warn_error, + features: build_args.features, clear_screen: false, prod: build_args.prod, } @@ -534,3 +648,11 @@ impl Deref for WarnErrorArg { &self.warn_error } } + +impl Deref for FeaturesArg { + type Target = Option; + + fn deref(&self) -> &Self::Target { + &self.features + } +} diff --git a/rewatch/src/config.rs b/rewatch/src/config.rs index 300a4a9ba2..96cce330ae 100644 --- a/rewatch/src/config.rs +++ b/rewatch/src/config.rs @@ -2,6 +2,7 @@ use crate::build::packages; use crate::helpers; use crate::helpers::deserialize::*; use crate::project_context::ProjectContext; +use ahash::AHashSet; use anyhow::{Result, anyhow}; use convert_case::{Case, Casing}; use serde::de::{Error as DeError, Visitor}; @@ -31,6 +32,10 @@ pub struct PackageSource { pub subdirs: Option, #[serde(rename = "type")] pub type_: Option, + /// Optional feature tag. When set, this source directory is only included in the build when + /// the package's active feature set contains this feature (or a feature that transitively + /// implies it through the top-level `features` map). + pub feature: Option, } impl PackageSource { @@ -40,6 +45,15 @@ impl PackageSource { None => false, } } + + /// Returns true when the source directory is part of the active feature set. + /// Untagged directories (no `feature` set) are always included. + pub fn is_feature_enabled(&self, active_features: &AHashSet) -> bool { + match &self.feature { + None => true, + Some(name) => active_features.contains(name), + } + } } impl Eq for PackageSource {} @@ -65,6 +79,7 @@ impl Source { dir: dir.to_string(), subdirs: None, type_: Some(type_), + feature: None, }), (Source::Qualified(package_source), type_) => Source::Qualified(PackageSource { type_, @@ -74,6 +89,34 @@ impl Source { } } + pub fn get_feature(&self) -> Option { + match self { + Source::Shorthand(_) => None, + Source::Qualified(PackageSource { feature, .. }) => feature.clone(), + } + } + + /// Propagate a feature tag down into nested subdirs. A child source's own explicit feature is + /// preserved; only when it has none does it inherit from the parent. This mirrors how `type: + /// "dev"` cascades today. + pub fn set_feature(&self, feature: Option) -> Source { + match (self, feature) { + (Source::Qualified(ps), parent_feature) if ps.feature.is_none() => { + Source::Qualified(PackageSource { + feature: parent_feature, + ..ps.clone() + }) + } + (Source::Shorthand(dir), Some(feature)) => Source::Qualified(PackageSource { + dir: dir.to_string(), + subdirs: None, + type_: None, + feature: Some(feature), + }), + (source, _) => source.clone(), + } + } + /// `to_qualified_without_children` takes a tree like structure of dependencies, coming in from /// `rescript.json`, and turns it into a flat list. The main thing we extract here are the source /// folders, and optional subdirs, where potentially, the subdirs recurse or not. @@ -87,10 +130,12 @@ impl Source { .to_string(), subdirs: None, type_: self.get_type(), + feature: self.get_feature(), }, Source::Qualified(PackageSource { dir, type_, + feature, subdirs: Some(Subdirs::Recurse(should_recurse)), }) => PackageSource { dir: sub_path @@ -100,8 +145,11 @@ impl Source { .to_string(), subdirs: Some(Subdirs::Recurse(*should_recurse)), type_: type_.to_owned(), + feature: feature.to_owned(), }, - Source::Qualified(PackageSource { dir, type_, .. }) => PackageSource { + Source::Qualified(PackageSource { + dir, type_, feature, .. + }) => PackageSource { dir: sub_path .map(|p| p.join(Path::new(dir))) .unwrap_or(Path::new(dir).to_path_buf()) @@ -109,6 +157,7 @@ impl Source { .to_string(), subdirs: None, type_: type_.to_owned(), + feature: feature.to_owned(), }, } } @@ -270,6 +319,42 @@ impl GenTypeModuleResolution { } } +/// A dependency entry in `dependencies` or `dev-dependencies`. Can be either the legacy shorthand +/// form (just the package name), or a qualified object form that can restrict which features of +/// the dependency should be built. +#[derive(Deserialize, Debug, Clone, PartialEq, Eq, Hash)] +#[serde(untagged)] +pub enum Dependency { + Shorthand(String), + Qualified(QualifiedDependency), +} + +#[derive(Deserialize, Debug, Clone, PartialEq, Eq, Hash)] +pub struct QualifiedDependency { + pub name: String, + /// When set, only these features (and their transitive expansion through the dependency's + /// own `features` map) are active when compiling this dependency. When omitted, all of the + /// dependency's features are active. + pub features: Option>, +} + +impl Dependency { + pub fn name(&self) -> &str { + match self { + Dependency::Shorthand(name) => name, + Dependency::Qualified(q) => &q.name, + } + } + + /// `Some(list)` restricts to those features; `None` means "all features" for this dep. + pub fn features(&self) -> Option<&Vec> { + match self { + Dependency::Shorthand(_) => None, + Dependency::Qualified(q) => q.features.as_ref(), + } + } +} + /// Accepts either an object `{ "From": "To", ... }` or (deprecated) an array of /// `"From=To"` strings. #[derive(Debug, Clone, PartialEq, Eq, Default)] @@ -388,9 +473,13 @@ pub struct Config { pub warnings: Option, pub suffix: Option, #[serde(alias = "bs-dependencies")] - pub dependencies: Option>, + pub dependencies: Option>, #[serde(rename = "dev-dependencies", alias = "bs-dev-dependencies")] - pub dev_dependencies: Option>, + pub dev_dependencies: Option>, + /// Optional feature declarations. Each key is a feature name and the value lists other + /// features it implies. Leaf features (no implications) don't need to be declared here — + /// any feature name used as a `feature:` tag on a source is auto-recognized. + pub features: Option>>, #[serde(rename = "ppx-flags")] pub ppx_flags: Option>>, @@ -782,7 +871,7 @@ impl Config { if let Some(deps) = &self.dependencies { for dep in deps { args.push("-bs-gentype-dep".to_string()); - args.push(dep.clone()); + args.push(dep.name().to_string()); } } for dir in source_dirs { @@ -871,6 +960,68 @@ impl Config { .unwrap_or(".js".to_string()) } + /// Returns the names of dependencies as plain strings (without feature restrictions). + /// Use `self.dependencies` directly when you need to know which features each consumer + /// requested for a dependency. + pub fn get_dependency_names(&self) -> Vec { + self.dependencies + .as_ref() + .map(|deps| deps.iter().map(|d| d.name().to_string()).collect()) + .unwrap_or_default() + } + + pub fn get_dev_dependency_names(&self) -> Vec { + self.dev_dependencies + .as_ref() + .map(|deps| deps.iter().map(|d| d.name().to_string()).collect()) + .unwrap_or_default() + } + + /// Collects every feature name that is known to this package: features declared in the + /// `features` map, features declared as keys on implied-lists, and any feature name used as + /// a `feature:` tag on a source directory. Used to compute the "all features active" default + /// when no explicit feature restriction was requested. + pub fn collect_declared_features(&self) -> AHashSet { + let mut set = AHashSet::new(); + + if let Some(map) = &self.features { + for (name, implies) in map { + set.insert(name.clone()); + for implied in implies { + set.insert(implied.clone()); + } + } + } + + fn walk(set: &mut AHashSet, source: &Source) { + match source { + Source::Shorthand(_) => {} + Source::Qualified(PackageSource { feature, subdirs, .. }) => { + if let Some(f) = feature { + set.insert(f.clone()); + } + if let Some(Subdirs::Qualified(children)) = subdirs { + for child in children { + walk(set, child); + } + } + } + } + } + + match &self.sources { + None => {} + Some(OneOrMore::Single(s)) => walk(&mut set, s), + Some(OneOrMore::Multiple(sources)) => { + for s in sources { + walk(&mut set, s); + } + } + } + + set + } + pub fn find_is_type_dev_for_path(&self, relative_path: &Path) -> bool { let relative_parent = match relative_path.parent() { None => return false, @@ -951,6 +1102,58 @@ impl Config { } } +/// Expands `requested` into the transitive closure under `features_map`. Each key in +/// `features_map` is a feature name that implies the feature names in its value list. Unknown +/// feature names (requested but not in the map) are kept in the output — they're treated as leaf +/// features. +/// +/// Errors if a cycle is detected (e.g. `full -> native -> full`). The error message names the +/// cycle participants so the user can fix their config. +pub fn resolve_active_features( + requested: &AHashSet, + features_map: Option<&HashMap>>, +) -> Result> { + let mut closure: AHashSet = AHashSet::new(); + + for feature in requested { + expand_feature(feature, features_map, &mut closure, &mut Vec::new())?; + } + + Ok(closure) +} + +fn expand_feature( + feature: &str, + features_map: Option<&HashMap>>, + closure: &mut AHashSet, + stack: &mut Vec, +) -> Result<()> { + if stack.iter().any(|s| s == feature) { + let mut chain = stack.clone(); + chain.push(feature.to_string()); + return Err(anyhow!( + "Cycle detected in `features` map: {}", + chain.join(" -> ") + )); + } + + if !closure.insert(feature.to_string()) { + return Ok(()); + } + + if let Some(map) = features_map + && let Some(implied) = map.get(feature) + { + stack.push(feature.to_string()); + for child in implied { + expand_feature(child, features_map, closure, stack)?; + } + stack.pop(); + } + + Ok(()) +} + fn validate_package_specs_value(value: &serde_json::Value) -> Result<()> { let specs = match value.get("package-specs") { Some(specs) => specs, @@ -1069,8 +1272,14 @@ pub mod tests { package_specs: None, warnings: None, suffix: None, - dependencies: Some(args.bs_deps), - dev_dependencies: Some(args.build_dev_deps), + dependencies: Some(args.bs_deps.into_iter().map(Dependency::Shorthand).collect()), + dev_dependencies: Some( + args.build_dev_deps + .into_iter() + .map(Dependency::Shorthand) + .collect(), + ), + features: None, ppx_flags: None, compiler_flags: None, namespace: None, @@ -1424,7 +1633,10 @@ pub mod tests { "#; let config = Config::new_from_json_string(json).expect("a valid json string"); - assert_eq!(config.dependencies, Some(vec!["@testrepo/main".to_string()])); + assert_eq!( + config.dependencies, + Some(vec![Dependency::Shorthand("@testrepo/main".to_string())]) + ); assert!(config.get_deprecations().is_empty()); } @@ -1449,7 +1661,10 @@ pub mod tests { "#; let config = Config::new_from_json_string(json).expect("a valid json string"); - assert_eq!(config.dependencies, Some(vec!["@testrepo/main".to_string()])); + assert_eq!( + config.dependencies, + Some(vec![Dependency::Shorthand("@testrepo/main".to_string())]) + ); assert_eq!(config.get_deprecations(), [DeprecationWarning::BsDependencies]); } @@ -1474,7 +1689,10 @@ pub mod tests { "#; let config = Config::new_from_json_string(json).expect("a valid json string"); - assert_eq!(config.dev_dependencies, Some(vec!["@testrepo/main".to_string()])); + assert_eq!( + config.dev_dependencies, + Some(vec![Dependency::Shorthand("@testrepo/main".to_string())]) + ); assert!(config.get_deprecations().is_empty()); } @@ -1499,7 +1717,10 @@ pub mod tests { "#; let config = Config::new_from_json_string(json).expect("a valid json string"); - assert_eq!(config.dev_dependencies, Some(vec!["@testrepo/main".to_string()])); + assert_eq!( + config.dev_dependencies, + Some(vec![Dependency::Shorthand("@testrepo/main".to_string())]) + ); assert_eq!(config.get_deprecations(), [DeprecationWarning::BsDevDependencies]); } @@ -1698,6 +1919,7 @@ pub mod tests { dir: String::from("src"), subdirs: None, type_: Some(String::from("dev")), + feature: None, })), Path::new("src/Foo.res"), true, @@ -1711,6 +1933,7 @@ pub mod tests { dir: String::from("src"), subdirs: None, type_: None, + feature: None, })), Path::new("src/Foo.res"), false, @@ -1724,6 +1947,7 @@ pub mod tests { dir: String::from("src"), subdirs: None, type_: Some(String::from("dev")), + feature: None, })]), Path::new("src/Foo.res"), true, @@ -1746,6 +1970,7 @@ pub mod tests { dir: String::from("src"), subdirs: Some(Subdirs::Recurse(true)), type_: Some(String::from("dev")), + feature: None, })]), Path::new("src/bar/Foo.res"), true, @@ -1761,8 +1986,10 @@ pub mod tests { dir: String::from("bar"), subdirs: None, type_: None, + feature: None, })])), type_: Some(String::from("dev")), + feature: None, })]), Path::new("src/bar/Foo.res"), true, @@ -1776,6 +2003,7 @@ pub mod tests { dir: String::from("src"), subdirs: Some(Subdirs::Qualified(vec![Source::Shorthand(String::from("bar"))])), type_: Some(String::from("dev")), + feature: None, })]), Path::new("src/bar/Foo.res"), true, @@ -1904,4 +2132,217 @@ pub mod tests { .contains(&DeprecationWarning::BsconfigJson) ); } + + #[test] + fn test_source_with_feature_tag_parses() { + let json = r#" + { + "name": "testrepo", + "sources": [ + { "dir": "src" }, + { "dir": "src-native", "feature": "native" } + ] + } + "#; + + let config = Config::new_from_json_string(json).expect("a valid json string"); + if let Some(OneOrMore::Multiple(sources)) = &config.sources { + let native = sources[1].to_qualified_without_children(None); + assert_eq!(native.feature, Some(String::from("native"))); + } else { + unreachable!() + } + } + + #[test] + fn test_features_map_parses() { + let json = r#" + { + "name": "testrepo", + "sources": { "dir": "src" }, + "features": { + "full": ["native", "experimental"], + "native": [], + "experimental": [] + } + } + "#; + + let config = Config::new_from_json_string(json).expect("a valid json string"); + let features = config.features.expect("features map should parse"); + assert_eq!( + features.get("full"), + Some(&vec!["native".to_string(), "experimental".to_string()]) + ); + assert_eq!(features.get("native"), Some(&vec![])); + } + + #[test] + fn test_dependency_qualified_form_parses() { + let json = r#" + { + "name": "testrepo", + "sources": { "dir": "src" }, + "dependencies": [ + "@plain/dep", + { "name": "@tagged/dep", "features": ["native"] } + ] + } + "#; + + let config = Config::new_from_json_string(json).expect("a valid json string"); + let deps = config.dependencies.expect("dependencies should parse"); + assert_eq!(deps.len(), 2); + assert_eq!(deps[0], Dependency::Shorthand("@plain/dep".to_string())); + match &deps[1] { + Dependency::Qualified(q) => { + assert_eq!(q.name, "@tagged/dep"); + assert_eq!(q.features, Some(vec!["native".to_string()])); + } + _ => unreachable!(), + } + } + + #[test] + fn test_is_feature_enabled_untagged_is_always_active() { + let source = PackageSource { + dir: "src".into(), + subdirs: None, + type_: None, + feature: None, + }; + let empty: AHashSet = AHashSet::new(); + assert!(source.is_feature_enabled(&empty)); + } + + #[test] + fn test_is_feature_enabled_tagged_requires_membership() { + let source = PackageSource { + dir: "src-native".into(), + subdirs: None, + type_: None, + feature: Some("native".to_string()), + }; + let mut active: AHashSet = AHashSet::new(); + assert!(!source.is_feature_enabled(&active)); + active.insert("native".to_string()); + assert!(source.is_feature_enabled(&active)); + } + + #[test] + fn test_resolve_active_features_expands_transitive() { + let mut map: HashMap> = HashMap::new(); + map.insert( + "full".to_string(), + vec!["native".to_string(), "experimental".to_string()], + ); + map.insert("native".to_string(), vec![]); + map.insert("experimental".to_string(), vec![]); + + let mut requested: AHashSet = AHashSet::new(); + requested.insert("full".to_string()); + + let closure = resolve_active_features(&requested, Some(&map)).unwrap(); + assert!(closure.contains("full")); + assert!(closure.contains("native")); + assert!(closure.contains("experimental")); + } + + #[test] + fn test_resolve_active_features_no_map_is_identity() { + let mut requested: AHashSet = AHashSet::new(); + requested.insert("native".to_string()); + let closure = resolve_active_features(&requested, None).unwrap(); + assert_eq!(closure.len(), 1); + assert!(closure.contains("native")); + } + + #[test] + fn test_resolve_active_features_detects_cycle() { + let mut map: HashMap> = HashMap::new(); + map.insert("a".to_string(), vec!["b".to_string()]); + map.insert("b".to_string(), vec!["a".to_string()]); + + let mut requested: AHashSet = AHashSet::new(); + requested.insert("a".to_string()); + + let err = resolve_active_features(&requested, Some(&map)) + .unwrap_err() + .to_string(); + assert!(err.contains("Cycle detected"), "unexpected error: {err}"); + } + + #[test] + fn test_collect_declared_features_unions_map_and_tags() { + let json = r#" + { + "name": "testrepo", + "sources": [ + { "dir": "src" }, + { "dir": "src-a", "feature": "a" }, + { "dir": "src-undeclared", "feature": "leaf-only" } + ], + "features": { + "bundle": ["a", "b"] + } + } + "#; + + let config = Config::new_from_json_string(json).expect("a valid json string"); + let declared = config.collect_declared_features(); + assert!(declared.contains("bundle")); + assert!(declared.contains("a")); + assert!(declared.contains("b")); + assert!(declared.contains("leaf-only")); + } + + #[test] + fn test_feature_cascades_to_qualified_subdirs() { + // Parent source is tagged; a nested shorthand subdir should inherit the feature. + let parent = Source::Qualified(PackageSource { + dir: String::from("src"), + subdirs: Some(Subdirs::Qualified(vec![Source::Shorthand(String::from( + "native", + ))])), + type_: None, + feature: Some(String::from("native")), + }); + let child_source = match &parent { + Source::Qualified(ps) => match &ps.subdirs { + Some(Subdirs::Qualified(children)) => children[0].clone(), + _ => unreachable!(), + }, + _ => unreachable!(), + }; + let propagated = child_source + .set_type(parent.get_type()) + .set_feature(parent.get_feature()); + assert_eq!(propagated.get_feature(), Some(String::from("native"))); + } + + #[test] + fn test_feature_cascade_does_not_overwrite_explicit_child() { + let child = Source::Qualified(PackageSource { + dir: String::from("nested"), + subdirs: None, + type_: None, + feature: Some(String::from("experimental")), + }); + let propagated = child.set_feature(Some(String::from("native"))); + assert_eq!(propagated.get_feature(), Some(String::from("experimental"))); + } + + #[test] + fn test_dependency_helpers_return_name_and_features() { + let shorthand = Dependency::Shorthand("@a/b".into()); + assert_eq!(shorthand.name(), "@a/b"); + assert!(shorthand.features().is_none()); + + let qualified = Dependency::Qualified(QualifiedDependency { + name: "@a/b".into(), + features: Some(vec!["x".into()]), + }); + assert_eq!(qualified.name(), "@a/b"); + assert_eq!(qualified.features().cloned(), Some(vec!["x".to_string()])); + } } diff --git a/rewatch/src/format.rs b/rewatch/src/format.rs index c4ea876e45..1397ea4b82 100644 --- a/rewatch/src/format.rs +++ b/rewatch/src/format.rs @@ -36,7 +36,8 @@ fn get_files_in_scope() -> Result> { let current_dir = std::env::current_dir()?; let project_context = project_context::ProjectContext::new(¤t_dir)?; - let packages = packages::make(&None, &project_context, false, false)?; + // Format walks all source files regardless of feature selection. + let packages = packages::make(&None, &project_context, false, false, None)?; let mut files: Vec = Vec::new(); let packages_to_format = project_context.get_scoped_local_packages(); diff --git a/rewatch/src/main.rs b/rewatch/src/main.rs index 8ab4eaa68c..e086206c0c 100644 --- a/rewatch/src/main.rs +++ b/rewatch/src/main.rs @@ -46,6 +46,7 @@ fn main() -> Result<()> { std::process::exit(0); } cli::Command::Build(build_args) => { + let features = build_args.features.parsed(); match build::build( &build_args.filter, Path::new(&build_args.folder as &str), @@ -55,6 +56,7 @@ fn main() -> Result<()> { plain_output, (*build_args.warn_error).clone(), build_args.prod, + features, ) { Err(e) => { eprintln!("{:#}", e); @@ -71,6 +73,7 @@ fn main() -> Result<()> { cli::Command::Watch(watch_args) => { let _lock = get_lock_or_exit(LockKind::Watch, &watch_args.folder); + let features = watch_args.features.parsed(); match watcher::start( &watch_args.filter, show_progress, @@ -81,6 +84,7 @@ fn main() -> Result<()> { (*watch_args.warn_error).clone(), watch_args.clear_screen, watch_args.prod, + features, ) { Err(e) => { eprintln!("{:#}", e); diff --git a/rewatch/src/project_context.rs b/rewatch/src/project_context.rs index afdafa38ab..fc5fae764e 100644 --- a/rewatch/src/project_context.rs +++ b/rewatch/src/project_context.rs @@ -121,13 +121,17 @@ fn read_local_packages( } fn monorepo_or_single_project(path: &Path, current_config: Config) -> Result { - let local_dependencies = match ¤t_config.dependencies { - None => AHashSet::::new(), - Some(deps) => read_local_packages(path, deps)?, + let dep_names = current_config.get_dependency_names(); + let dev_dep_names = current_config.get_dev_dependency_names(); + let local_dependencies = if dep_names.is_empty() { + AHashSet::::new() + } else { + read_local_packages(path, &dep_names)? }; - let local_dev_dependencies = match ¤t_config.dev_dependencies { - None => AHashSet::::new(), - Some(deps) => read_local_packages(path, deps)?, + let local_dev_dependencies = if dev_dep_names.is_empty() { + AHashSet::::new() + } else { + read_local_packages(path, &dev_dep_names)? }; if local_dependencies.is_empty() && local_dev_dependencies.is_empty() { Ok(ProjectContext { @@ -151,15 +155,11 @@ fn monorepo_or_single_project(path: &Path, current_config: Config) -> Result bool { workspace_config - .dependencies - .to_owned() - .unwrap_or_default() + .get_dependency_names() .iter() .any(|dep| dep == ¤t_config.name) || workspace_config - .dev_dependencies - .to_owned() - .unwrap_or_default() + .get_dev_dependency_names() .iter() .any(|dep| dep == ¤t_config.name) } diff --git a/rewatch/src/sourcedirs.rs b/rewatch/src/sourcedirs.rs index 9e18666899..249cb0d0a2 100644 --- a/rewatch/src/sourcedirs.rs +++ b/rewatch/src/sourcedirs.rs @@ -56,13 +56,14 @@ fn package_to_dirs(package: &Package, root_package_path: &Path) -> AHashSet fn deps_to_pkgs<'a>( packages: &'a AHashMap, - dependencies: &'a Option>, + dependencies: &'a Option>, ) -> AHashSet { dependencies .as_ref() .unwrap_or(&vec![]) .iter() - .filter_map(|name| { + .filter_map(|dep| { + let name = dep.name(); packages .get(name) .map(|package| (name.to_owned(), package.path.to_owned())) diff --git a/rewatch/src/watcher.rs b/rewatch/src/watcher.rs index 43fa4ea7f0..89c405cd0a 100644 --- a/rewatch/src/watcher.rs +++ b/rewatch/src/watcher.rs @@ -216,6 +216,7 @@ struct AsyncWatchArgs<'a> { plain_output: bool, clear_screen: bool, prod: bool, + features: Option>, } async fn async_watch( @@ -232,6 +233,7 @@ async fn async_watch( plain_output, clear_screen, prod, + features, }: AsyncWatchArgs<'_>, ) -> Result<()> { let mut build_state = initial_build_state; @@ -468,6 +470,7 @@ async fn async_watch( plain_output, build_state.get_warn_error_override(), prod, + features.clone(), ) .expect("Could not initialize build"); @@ -544,6 +547,7 @@ pub fn start( warn_error: Option, clear_screen: bool, prod: bool, + features: Option>, ) -> Result<()> { futures::executor::block_on(async { let queue = Arc::new(FifoQueue::>::new()); @@ -564,6 +568,7 @@ pub fn start( plain_output, warn_error.clone(), prod, + features.clone(), ) .with_context(|| "Could not initialize build")?; @@ -584,6 +589,7 @@ pub fn start( plain_output, clear_screen, prod, + features, }) .await }) @@ -662,6 +668,7 @@ mod tests { packages, compiler, None, + None, ); build_state.insert_module(module_name, module); build_state diff --git a/rewatch/testrepo/packages/with-features/package.json b/rewatch/testrepo/packages/with-features/package.json new file mode 100644 index 0000000000..ffd600991d --- /dev/null +++ b/rewatch/testrepo/packages/with-features/package.json @@ -0,0 +1,4 @@ +{ + "name": "@testrepo/with-features", + "version": "1.0.0" +} diff --git a/rewatch/testrepo/packages/with-features/rescript.json b/rewatch/testrepo/packages/with-features/rescript.json new file mode 100644 index 0000000000..50a925c517 --- /dev/null +++ b/rewatch/testrepo/packages/with-features/rescript.json @@ -0,0 +1,16 @@ +{ + "name": "@testrepo/with-features", + "sources": [ + { "dir": "src" }, + { "dir": "src-native", "feature": "native" }, + { "dir": "src-experimental", "feature": "experimental" } + ], + "package-specs": { + "module": "esmodule", + "in-source": true + }, + "suffix": ".mjs", + "features": { + "full": ["native", "experimental"] + } +} diff --git a/rewatch/testrepo/packages/with-features/src-experimental/Experimental.res b/rewatch/testrepo/packages/with-features/src-experimental/Experimental.res new file mode 100644 index 0000000000..f4a87bea23 --- /dev/null +++ b/rewatch/testrepo/packages/with-features/src-experimental/Experimental.res @@ -0,0 +1 @@ +let experimental = () => "experimental-only" diff --git a/rewatch/testrepo/packages/with-features/src-native/Native.res b/rewatch/testrepo/packages/with-features/src-native/Native.res new file mode 100644 index 0000000000..0f68f0384e --- /dev/null +++ b/rewatch/testrepo/packages/with-features/src-native/Native.res @@ -0,0 +1 @@ +let native = () => "native-only" diff --git a/rewatch/testrepo/packages/with-features/src/Common.res b/rewatch/testrepo/packages/with-features/src/Common.res new file mode 100644 index 0000000000..05dc63c33d --- /dev/null +++ b/rewatch/testrepo/packages/with-features/src/Common.res @@ -0,0 +1 @@ +let greet = name => "Hello, " ++ name diff --git a/rewatch/tests/features/01-features-default-all-active.sh b/rewatch/tests/features/01-features-default-all-active.sh new file mode 100755 index 0000000000..968e5486a6 --- /dev/null +++ b/rewatch/tests/features/01-features-default-all-active.sh @@ -0,0 +1,40 @@ +#!/bin/bash +cd $(dirname $0) +source "../utils.sh" +cd ../../testrepo + +bold "Test: with no --features flag, all feature-tagged source dirs are compiled" + +pushd ./packages/with-features > /dev/null + +"$REWATCH_EXECUTABLE" clean &> /dev/null + +error_output=$("$REWATCH_EXECUTABLE" build 2>&1) +if [ $? -ne 0 ]; +then + error "Default build should succeed" + printf "%s\n" "$error_output" >&2 + popd > /dev/null + exit 1 +fi + +for dir in src src-native src-experimental; do + if [ -f "$dir/${dir#src-}.mjs" ] || [ -f "$dir/Common.mjs" ] || [ -f "$dir/Native.mjs" ] || [ -f "$dir/Experimental.mjs" ]; then + found=1 + else + found=0 + fi +done + +count=$(find src src-native src-experimental -name "*.mjs" 2>/dev/null | wc -l | tr -d ' ') +if [ "$count" -eq 3 ]; then + success "All 3 source dirs compiled without --features" +else + error "Expected 3 .mjs files (one per source dir), found $count" + find src src-native src-experimental -name "*.mjs" + popd > /dev/null + exit 1 +fi + +"$REWATCH_EXECUTABLE" clean &> /dev/null +popd > /dev/null diff --git a/rewatch/tests/features/02-features-cli-restricts-to-one.sh b/rewatch/tests/features/02-features-cli-restricts-to-one.sh new file mode 100755 index 0000000000..fb2eb13edb --- /dev/null +++ b/rewatch/tests/features/02-features-cli-restricts-to-one.sh @@ -0,0 +1,46 @@ +#!/bin/bash +cd $(dirname $0) +source "../utils.sh" +cd ../../testrepo + +bold "Test: --features=native restricts compilation to the untagged and native source dirs" + +pushd ./packages/with-features > /dev/null + +"$REWATCH_EXECUTABLE" clean &> /dev/null + +error_output=$("$REWATCH_EXECUTABLE" build --features native 2>&1) +if [ $? -ne 0 ]; +then + error "Build with --features native failed" + printf "%s\n" "$error_output" >&2 + popd > /dev/null + exit 1 +fi + +if [ -f "src/Common.mjs" ]; then + success "Untagged source src/Common.res was compiled" +else + error "Expected src/Common.mjs to exist" + popd > /dev/null + exit 1 +fi + +if [ -f "src-native/Native.mjs" ]; then + success "Feature-gated source src-native/Native.res was compiled" +else + error "Expected src-native/Native.mjs to exist" + popd > /dev/null + exit 1 +fi + +if [ -f "src-experimental/Experimental.mjs" ]; then + error "Did not expect src-experimental/Experimental.mjs to be compiled" + popd > /dev/null + exit 1 +else + success "Feature-gated source src-experimental/Experimental.res was skipped" +fi + +"$REWATCH_EXECUTABLE" clean &> /dev/null +popd > /dev/null diff --git a/rewatch/tests/features/03-features-transitive-expansion.sh b/rewatch/tests/features/03-features-transitive-expansion.sh new file mode 100755 index 0000000000..7227e87615 --- /dev/null +++ b/rewatch/tests/features/03-features-transitive-expansion.sh @@ -0,0 +1,32 @@ +#!/bin/bash +cd $(dirname $0) +source "../utils.sh" +cd ../../testrepo + +bold "Test: --features=full transitively expands through the features map" + +pushd ./packages/with-features > /dev/null + +"$REWATCH_EXECUTABLE" clean &> /dev/null + +error_output=$("$REWATCH_EXECUTABLE" build --features full 2>&1) +if [ $? -ne 0 ]; +then + error "Build with --features full failed" + printf "%s\n" "$error_output" >&2 + popd > /dev/null + exit 1 +fi + +count=$(find src src-native src-experimental -name "*.mjs" 2>/dev/null | wc -l | tr -d ' ') +if [ "$count" -eq 3 ]; then + success "--features=full expanded to both native and experimental" +else + error "Expected 3 .mjs files, found $count" + find src src-native src-experimental -name "*.mjs" + popd > /dev/null + exit 1 +fi + +"$REWATCH_EXECUTABLE" clean &> /dev/null +popd > /dev/null diff --git a/rewatch/tests/features/04-features-toggle-cleans-artifacts.sh b/rewatch/tests/features/04-features-toggle-cleans-artifacts.sh new file mode 100755 index 0000000000..a5b432b45e --- /dev/null +++ b/rewatch/tests/features/04-features-toggle-cleans-artifacts.sh @@ -0,0 +1,51 @@ +#!/bin/bash +cd $(dirname $0) +source "../utils.sh" +cd ../../testrepo + +bold "Test: toggling off a feature removes previously-compiled artifacts on next build" + +pushd ./packages/with-features > /dev/null + +"$REWATCH_EXECUTABLE" clean &> /dev/null + +# First build with all features: experimental should be compiled +error_output=$("$REWATCH_EXECUTABLE" build 2>&1) +if [ $? -ne 0 ]; then + error "Initial full build failed" + printf "%s\n" "$error_output" >&2 + popd > /dev/null + exit 1 +fi + +if [ ! -f "src-experimental/Experimental.mjs" ]; then + error "Expected src-experimental/Experimental.mjs from initial build" + popd > /dev/null + exit 1 +fi + +# Rebuild with only 'native' active +error_output=$("$REWATCH_EXECUTABLE" build --features native 2>&1) +if [ $? -ne 0 ]; then + error "Restricted rebuild failed" + printf "%s\n" "$error_output" >&2 + popd > /dev/null + exit 1 +fi + +if [ -f "src-experimental/Experimental.mjs" ]; then + error "Expected src-experimental/Experimental.mjs to be cleaned up when feature disabled" + popd > /dev/null + exit 1 +else + success "Old experimental artifact was removed after feature was disabled" +fi + +if [ ! -f "src-native/Native.mjs" ]; then + error "Expected src-native/Native.mjs to still exist" + popd > /dev/null + exit 1 +fi + +"$REWATCH_EXECUTABLE" clean &> /dev/null +popd > /dev/null diff --git a/rewatch/tests/features/05-features-cycle-errors.sh b/rewatch/tests/features/05-features-cycle-errors.sh new file mode 100755 index 0000000000..17cf411c44 --- /dev/null +++ b/rewatch/tests/features/05-features-cycle-errors.sh @@ -0,0 +1,53 @@ +#!/bin/bash +cd $(dirname $0) +source "../utils.sh" +cd ../../testrepo + +bold "Test: a cycle in the features map produces a clear error" + +pushd ./packages/with-features > /dev/null + +"$REWATCH_EXECUTABLE" clean &> /dev/null + +cp rescript.json rescript.json.bak +cat > rescript.json <<'JSON' +{ + "name": "@testrepo/with-features", + "sources": [ + { "dir": "src" }, + { "dir": "src-native", "feature": "native" }, + { "dir": "src-experimental", "feature": "experimental" } + ], + "package-specs": { + "module": "esmodule", + "in-source": true + }, + "suffix": ".mjs", + "features": { + "a": ["b"], + "b": ["a"] + } +} +JSON + +error_output=$("$REWATCH_EXECUTABLE" build --features a 2>&1) +status=$? + +mv rescript.json.bak rescript.json + +if [ "$status" -eq 0 ]; then + error "Expected build to fail on cycle, but it succeeded" + popd > /dev/null + exit 1 +fi + +if echo "$error_output" | grep -q "Cycle detected"; then + success "Cycle in features map produced a clear error" +else + error "Expected error message to mention cycle, got: $error_output" + popd > /dev/null + exit 1 +fi + +"$REWATCH_EXECUTABLE" clean &> /dev/null +popd > /dev/null diff --git a/rewatch/tests/features/06-features-empty-flag-rejected.sh b/rewatch/tests/features/06-features-empty-flag-rejected.sh new file mode 100755 index 0000000000..afc785a94c --- /dev/null +++ b/rewatch/tests/features/06-features-empty-flag-rejected.sh @@ -0,0 +1,27 @@ +#!/bin/bash +cd $(dirname $0) +source "../utils.sh" +cd ../../testrepo + +bold "Test: --features with empty value is rejected" + +pushd ./packages/with-features > /dev/null + +error_output=$("$REWATCH_EXECUTABLE" build --features "" 2>&1) +status=$? + +if [ "$status" -eq 0 ]; then + error "Expected empty --features to fail, but build succeeded" + popd > /dev/null + exit 1 +fi + +if echo "$error_output" | grep -q "must not be empty"; then + success "Empty --features value is rejected with a helpful message" +else + error "Expected error mentioning 'must not be empty', got: $error_output" + popd > /dev/null + exit 1 +fi + +popd > /dev/null diff --git a/rewatch/tests/suite.sh b/rewatch/tests/suite.sh index 613b291d11..295fdb10c5 100755 --- a/rewatch/tests/suite.sh +++ b/rewatch/tests/suite.sh @@ -119,6 +119,14 @@ fi # Experimental-invalid tests ./experimental-invalid/01-invalid-experimental-key.sh && +# Features tests +./features/01-features-default-all-active.sh && +./features/02-features-cli-restricts-to-one.sh && +./features/03-features-transitive-expansion.sh && +./features/04-features-toggle-cleans-artifacts.sh && +./features/05-features-cycle-errors.sh && +./features/06-features-empty-flag-rejected.sh && + # Compiler-args tests ./compiler-args/01-compiler-args-cwd-invariant.sh && ./compiler-args/02-warnings-in-parser-and-compiler.sh diff --git a/tests/build_tests/cli_help/input.js b/tests/build_tests/cli_help/input.js index 88e2ac8a0a..5f0bb1ea68 100755 --- a/tests/build_tests/cli_help/input.js +++ b/tests/build_tests/cli_help/input.js @@ -44,6 +44,7 @@ const buildHelp = " -a, --after-build Run an additional command after build. E.g., play a sound or run a test suite when done compiling\n" + " -q, --quiet... Decrease logging verbosity\n" + ' --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' + + " --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" + " -n, --no-timing [] Disable output timing [default: false] [possible values: true, false]\n" + ' --prod Skip dev-dependencies and dev sources (type: "dev")\n' + " -h, --help Print help\n";