Skip to content

Deprecate bs-dependencies and bs-dev-dependencies #7658

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+379 −191
Open

Deprecate bs-dependencies and bs-dev-dependencies #7658

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 35 additions & 2 deletions rewatch/src/build.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use self::parse::parser_args;
use crate::build::compile::{mark_modules_with_deleted_deps_dirty, mark_modules_with_expired_deps_dirty};
use crate::helpers::emojis::*;
use crate::helpers::{self, get_workspace_root};
use crate::sourcedirs;
use crate::{config, sourcedirs};
use anyhow::{Result, anyhow};
use build_types::*;
use console::style;
Expand Down Expand Up @@ -280,7 +280,7 @@ impl fmt::Display for IncrementalBuildError {
pub fn incremental_build(
build_state: &mut BuildState,
default_timing: Option<Duration>,
_initial_build: bool,
initial_build: bool,
show_progress: bool,
only_incremental: bool,
create_sourcedirs: bool,
Expand Down Expand Up @@ -426,6 +426,9 @@ pub fn incremental_build(
if helpers::contains_ascii_characters(&compile_warnings) {
println!("{}", &compile_warnings);
}
if initial_build {
log_deprecations(build_state);
}
if helpers::contains_ascii_characters(&compile_errors) {
println!("{}", &compile_errors);
}
Expand All @@ -452,10 +455,40 @@ pub fn incremental_build(
if helpers::contains_ascii_characters(&compile_warnings) {
println!("{}", &compile_warnings);
}
if initial_build {
log_deprecations(build_state);
}

Ok(())
}
}

fn log_deprecations(build_state: &BuildState) {
build_state.packages.iter().for_each(|(_, package)| {
package
.config
.get_deprecations()
.iter()
.for_each(|deprecation_warning| match deprecation_warning {
config::DeprecationWarning::BsDependencies => {
log_deprecated_config_field(&package.name, "bs-dependencies", "dependencies");
}
config::DeprecationWarning::BsDevDependencies => {
log_deprecated_config_field(&package.name, "bs-dev-dependencies", "dev-dependencies");
}
});
});
}

fn log_deprecated_config_field(package_name: &str, field_name: &str, new_field_name: &str) {
let warning = format!(
"Deprecated use of field '{}' found in the config of package '{}'.\n\
Use '{}' instead. This field will be removed in a future version.",
field_name, package_name, new_field_name
);
println!("\n{}", style(warning).yellow());
}

// write build.ninja files in the packages after a non-incremental build
// this is necessary to bust the editor tooling cache. The editor tooling
// is watching this file.
Expand Down
4 changes: 2 additions & 2 deletions rewatch/src/build/compile.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -504,7 +504,7 @@ fn get_dependency_paths(
is_file_type_dev: bool,
) -> Vec<Vec<String>> {
let normal_deps = config
.bs_dependencies
.dependencies
.clone()
.unwrap_or_default()
.into_iter()
Expand All @@ -514,7 +514,7 @@ fn get_dependency_paths(
// We can only access dev dependencies for source_files that are marked as "type":"dev"
let dev_deps = if is_file_type_dev {
config
.bs_dev_dependencies
.dev_dependencies
.clone()
.unwrap_or_default()
.into_iter()
Expand Down
11 changes: 2 additions & 9 deletions rewatch/src/build/deps.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,18 +37,11 @@ fn get_dep_modules(
// Get the list of allowed dependency packages for this package
let allowed_dependencies: AHashSet<String> = package
.config
.bs_dependencies
.dependencies
.as_ref()
.unwrap_or(&vec![])
.iter()
.chain(
package
.config
.bs_dev_dependencies
.as_ref()
.unwrap_or(&vec![])
.iter(),
)
.chain(package.config.dev_dependencies.as_ref().unwrap_or(&vec![]).iter())
.cloned()
.collect();

Expand Down
161 changes: 76 additions & 85 deletions rewatch/src/build/packages.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -243,9 +243,9 @@ pub fn read_config(package_dir: &Path) -> Result<config::Config> {
let bsconfig_json_path = package_dir.join("bsconfig.json");

if Path::new(&rescript_json_path).exists() {
config::read(&rescript_json_path)
config::Config::new(&rescript_json_path)
} else {
config::read(&bsconfig_json_path)
config::Config::new(&bsconfig_json_path)
}
}

Expand Down Expand Up @@ -310,10 +310,10 @@ fn read_dependencies(
show_progress: bool,
build_dev_deps: bool,
) -> Vec<Dependency> {
let mut dependencies = parent_config.bs_dependencies.to_owned().unwrap_or_default();
let mut dependencies = parent_config.dependencies.to_owned().unwrap_or_default();

// Concatenate dev dependencies if build_dev_deps is true
if build_dev_deps && let Some(dev_deps) = parent_config.bs_dev_dependencies.to_owned() {
if build_dev_deps && let Some(dev_deps) = parent_config.dev_dependencies.to_owned() {
dependencies.extend(dev_deps);
}

Expand Down Expand Up @@ -916,9 +916,9 @@ pub fn validate_packages_dependencies(packages: &AHashMap<String, Package>) -> b
let mut detected_unallowed_dependencies: AHashMap<String, UnallowedDependency> = AHashMap::new();

for (package_name, package) in packages {
let bs_dependencies = &package.config.bs_dependencies.to_owned().unwrap_or(vec![]);
let bs_dependencies = &package.config.dependencies.to_owned().unwrap_or(vec![]);
let pinned_dependencies = &package.config.pinned_dependencies.to_owned().unwrap_or(vec![]);
let dev_dependencies = &package.config.bs_dev_dependencies.to_owned().unwrap_or(vec![]);
let dev_dependencies = &package.config.dev_dependencies.to_owned().unwrap_or(vec![]);

[
("bs-dependencies", bs_dependencies),
Expand Down Expand Up @@ -984,39 +984,30 @@ pub fn validate_packages_dependencies(packages: &AHashMap<String, Package>) -> b

#[cfg(test)]
mod test {
use crate::config;

use super::{Namespace, Package};
use crate::config::Source;
use ahash::{AHashMap, AHashSet};
use std::path::PathBuf;

fn create_package(
pub struct CreatePackageArgs {
name: String,
bs_deps: Vec<String>,
pinned_deps: Vec<String>,
build_dev_deps: Vec<String>,
allowed_dependents: Option<Vec<String>>,
) -> Package {
}

fn create_package(args: CreatePackageArgs) -> Package {
Package {
name: name.clone(),
config: crate::config::Config {
name: name.clone(),
sources: Some(crate::config::OneOrMore::Single(Source::Shorthand(String::from(
"Source",
)))),
package_specs: None,
warnings: None,
suffix: None,
pinned_dependencies: Some(pinned_deps),
bs_dependencies: Some(bs_deps),
bs_dev_dependencies: Some(build_dev_deps),
ppx_flags: None,
bsc_flags: None,
namespace: None,
jsx: None,
gentype_config: None,
namespace_entry: None,
allowed_dependents,
},
name: args.name.clone(),
config: config::tests::create_config(config::tests::CreateConfigArgs {
name: args.name,
bs_deps: args.bs_deps,
pinned_deps: args.pinned_deps,
build_dev_deps: args.build_dev_deps,
allowed_dependents: args.allowed_dependents,
}),
source_folders: AHashSet::new(),
source_files: None,
namespace: Namespace::Namespace(String::from("Package1")),
Expand All @@ -1033,23 +1024,23 @@ mod test {
let mut packages: AHashMap<String, Package> = AHashMap::new();
packages.insert(
String::from("Package1"),
create_package(
String::from("Package1"),
vec![String::from("Package2")],
vec![],
vec![],
None,
),
create_package(CreatePackageArgs {
name: String::from("Package1"),
bs_deps: vec![String::from("Package2")],
pinned_deps: vec![],
build_dev_deps: vec![],
allowed_dependents: None,
}),
);
packages.insert(
String::from("Package2"),
create_package(
String::from("Package2"),
vec![],
vec![],
vec![],
Some(vec![String::from("Package3")]),
),
create_package(CreatePackageArgs {
name: String::from("Package2"),
bs_deps: vec![],
pinned_deps: vec![],
build_dev_deps: vec![],
allowed_dependents: Some(vec![String::from("Package3")]),
}),
);

let is_valid = super::validate_packages_dependencies(&packages);
Expand All @@ -1061,23 +1052,23 @@ mod test {
let mut packages: AHashMap<String, Package> = AHashMap::new();
packages.insert(
String::from("Package1"),
create_package(
String::from("Package1"),
vec![],
vec![String::from("Package2")],
vec![],
None,
),
create_package(CreatePackageArgs {
name: String::from("Package1"),
bs_deps: vec![],
pinned_deps: vec![String::from("Package2")],
build_dev_deps: vec![],
allowed_dependents: None,
}),
);
packages.insert(
String::from("Package2"),
create_package(
String::from("Package2"),
vec![],
vec![],
vec![],
Some(vec![String::from("Package3")]),
),
create_package(CreatePackageArgs {
name: String::from("Package2"),
bs_deps: vec![],
pinned_deps: vec![],
build_dev_deps: vec![],
allowed_dependents: Some(vec![String::from("Package3")]),
}),
);

let is_valid = super::validate_packages_dependencies(&packages);
Expand All @@ -1089,23 +1080,23 @@ mod test {
let mut packages: AHashMap<String, Package> = AHashMap::new();
packages.insert(
String::from("Package1"),
create_package(
String::from("Package1"),
vec![],
vec![],
vec![String::from("Package2")],
None,
),
create_package(CreatePackageArgs {
name: String::from("Package1"),
bs_deps: vec![],
pinned_deps: vec![],
build_dev_deps: vec![String::from("Package2")],
allowed_dependents: None,
}),
);
packages.insert(
String::from("Package2"),
create_package(
String::from("Package2"),
vec![],
vec![],
vec![],
Some(vec![String::from("Package3")]),
),
create_package(CreatePackageArgs {
name: String::from("Package2"),
bs_deps: vec![],
pinned_deps: vec![],
build_dev_deps: vec![],
allowed_dependents: Some(vec![String::from("Package3")]),
}),
);

let is_valid = super::validate_packages_dependencies(&packages);
Expand All @@ -1117,23 +1108,23 @@ mod test {
let mut packages: AHashMap<String, Package> = AHashMap::new();
packages.insert(
String::from("Package1"),
create_package(
String::from("Package1"),
vec![String::from("Package2")],
vec![],
vec![],
None,
),
create_package(CreatePackageArgs {
name: String::from("Package1"),
bs_deps: vec![String::from("Package2")],
pinned_deps: vec![],
build_dev_deps: vec![],
allowed_dependents: None,
}),
);
packages.insert(
String::from("Package2"),
create_package(
String::from("Package2"),
vec![],
vec![],
vec![],
Some(vec![String::from("Package1")]),
),
create_package(CreatePackageArgs {
name: String::from("Package2"),
bs_deps: vec![],
pinned_deps: vec![],
build_dev_deps: vec![],
allowed_dependents: Some(vec![String::from("Package1")]),
}),
);

let is_valid = super::validate_packages_dependencies(&packages);
Expand Down
Loading