Skip to content

Commit d0d1a8f

Browse files
jfrolichclaude
andauthored
Rewatch: restore backward compatibility for bsconfig.json (#8368)
* Rewatch: restore backward compatibility for bsconfig.json Accept the legacy config file name and the old field aliases (bs-dependencies, bs-dev-dependencies, bsc-flags) so existing packages continue to build. Emit a deprecation warning on the legacy field names pointing at the modern replacements. Co-Authored-By: Claude <noreply@anthropic.com> * Format Co-Authored-By: Claude <noreply@anthropic.com> * Rewatch: deprecation warning when bsconfig.json filename is used Co-Authored-By: Claude <noreply@anthropic.com> * Rewatch: fall back to bsconfig.json for package name lookup; update legacy-config test read_package_name now also looks inside bsconfig.json if neither package.json nor rescript.json carry the package name. The warn_legacy_config build test is updated to assert the new behavior (build succeeds + deprecation warning is printed). Co-Authored-By: Claude <noreply@anthropic.com> * Rewatch: surface bsconfig/bs-* deprecations for external packages too External packages can't fix these themselves, so the warning now fires for any package (local or external). For external packages we also look up an issue tracker URL from bugs/repository in package.json and include it so users can report the issue upstream. Co-Authored-By: Claude <noreply@anthropic.com> * Rewatch: sort packages alphabetically when logging config warnings; refresh snapshots Sorting the package iteration makes the warning output stable across runs (AHashMap iteration order is otherwise nondeterministic). Snapshot files are updated to reflect both the new deprecation messages (bs-dependencies/bs-dev-dependencies/bsc-flags for @testrepo/deprecated-config, and for the external rescript-nodejs + sury packages plus the bsconfig.json filename deprecation on rescript-nodejs) and the new sorted order. Co-Authored-By: Claude <noreply@anthropic.com> * Rewatch: trigger full rebuild on bsconfig.json changes in watch mode Previously watch mode only upgraded rescript.json changes to a full rebuild. In a bsconfig-only project, editing config fields did not trigger recompilation, so the running watcher kept stale build state until restarted. Co-Authored-By: Claude <noreply@anthropic.com> * Rewatch: consolidate per-package deprecation warnings into one block Previously each deprecated field/filename printed its own paragraph, which was noisy (especially for packages with multiple legacy fields, like rescript-nodejs). The issue-tracker URL was also repeated per warning. Collapsed into a single block per package: Package 'foo' uses deprecated config (support will be removed in a future version): - field 'bs-dependencies' — use 'dependencies' instead - filename 'bsconfig.json' — rename to 'rescript.json' Please report this to the package maintainer: https://…/issues Snapshots and the warn_legacy_config build test are updated to match. Co-Authored-By: Claude <noreply@anthropic.com> * Format warn_legacy_config/input.js per biome Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent c6bf334 commit d0d1a8f

16 files changed

Lines changed: 480 additions & 72 deletions

rewatch/src/build.rs

Lines changed: 53 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -483,8 +483,21 @@ pub fn incremental_build(
483483
}
484484

485485
fn log_config_warnings(build_state: &BuildCommandState) {
486-
build_state.packages.iter().for_each(|(_, package)| {
487-
// Only warn for local dependencies, not external packages
486+
let mut packages: Vec<_> = build_state.packages.values().collect();
487+
packages.sort_by(|a, b| a.name.cmp(&b.name));
488+
packages.iter().for_each(|package| {
489+
let deprecations = package.config.get_deprecations();
490+
if !deprecations.is_empty() {
491+
// External consumers can't fix deprecations themselves, so we
492+
// point them at the package's issue tracker when we can find one.
493+
let issue_tracker_url = if package.is_local_dep {
494+
None
495+
} else {
496+
crate::build::packages::read_issue_tracker_url(&package.path)
497+
};
498+
log_deprecations(&package.name, deprecations, issue_tracker_url.as_deref());
499+
}
500+
488501
if package.is_local_dep {
489502
package
490503
.config
@@ -501,6 +514,44 @@ fn log_config_warnings(build_state: &BuildCommandState) {
501514
});
502515
}
503516

517+
fn log_deprecations(
518+
package_name: &str,
519+
deprecations: &[crate::config::DeprecationWarning],
520+
issue_tracker_url: Option<&str>,
521+
) {
522+
let mut message = format!(
523+
"Package '{package_name}' uses deprecated config (support will be removed in a future version):"
524+
);
525+
for deprecation in deprecations {
526+
let line = match deprecation {
527+
crate::config::DeprecationWarning::BsconfigJson => {
528+
" - filename 'bsconfig.json' — rename to 'rescript.json'"
529+
}
530+
crate::config::DeprecationWarning::BsDependencies => {
531+
" - field 'bs-dependencies' — use 'dependencies' instead"
532+
}
533+
crate::config::DeprecationWarning::BsDevDependencies => {
534+
" - field 'bs-dev-dependencies' — use 'dev-dependencies' instead"
535+
}
536+
crate::config::DeprecationWarning::BscFlags => {
537+
" - field 'bsc-flags' — use 'compiler-flags' instead"
538+
}
539+
crate::config::DeprecationWarning::CjsModule => {
540+
" - module 'cjs' in package-specs — use 'commonjs' instead"
541+
}
542+
crate::config::DeprecationWarning::Es6Module => {
543+
" - module 'es6' in package-specs — use 'esmodule' instead"
544+
}
545+
};
546+
message.push('\n');
547+
message.push_str(line);
548+
}
549+
if let Some(url) = issue_tracker_url {
550+
message.push_str(&format!("\nPlease report this to the package maintainer: {url}"));
551+
}
552+
eprintln!("\n{}", style(message).yellow());
553+
}
554+
504555
fn log_unsupported_config_field(package_name: &str, field_name: &str) {
505556
let warning = format!(
506557
"The field '{field_name}' found in the package config of '{package_name}' is not supported by ReScript 12's new build system."

rewatch/src/build/packages.rs

Lines changed: 121 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -251,7 +251,13 @@ fn get_source_dirs(source: config::Source, sub_path: Option<PathBuf>) -> AHashSe
251251

252252
pub fn read_config(package_dir: &Path) -> Result<Config> {
253253
let rescript_json_path = package_dir.join("rescript.json");
254-
Config::new(&rescript_json_path)
254+
let bsconfig_json_path = package_dir.join("bsconfig.json");
255+
256+
if rescript_json_path.exists() {
257+
Config::new(&rescript_json_path)
258+
} else {
259+
Config::new(&bsconfig_json_path)
260+
}
255261
}
256262

257263
pub fn read_dependency(
@@ -451,12 +457,52 @@ pub fn read_package_name(package_dir: &Path) -> Result<String> {
451457
return Ok(name);
452458
}
453459

460+
if let Some(name) = read_name("bsconfig.json")? {
461+
return Ok(name);
462+
}
463+
454464
Err(anyhow!(
455465
"No name field found in package.json or rescript.json in {}",
456466
package_dir.to_string_lossy()
457467
))
458468
}
459469

470+
/// Looks up the best-effort issue tracker URL for a package by reading its
471+
/// package.json, preferring `bugs.url`, then deriving from `repository`.
472+
/// Returns None if no tracker could be inferred.
473+
pub fn read_issue_tracker_url(package_dir: &Path) -> Option<String> {
474+
let contents = fs::read_to_string(package_dir.join("package.json")).ok()?;
475+
let json: serde_json::Value = serde_json::from_str(&contents).ok()?;
476+
477+
let extract_url = |v: &serde_json::Value| -> Option<String> {
478+
match v {
479+
serde_json::Value::String(s) => Some(s.to_owned()),
480+
serde_json::Value::Object(o) => o.get("url").and_then(|u| u.as_str()).map(String::from),
481+
_ => None,
482+
}
483+
};
484+
485+
if let Some(bugs_url) = json.get("bugs").and_then(extract_url) {
486+
return Some(bugs_url);
487+
}
488+
489+
json.get("repository")
490+
.and_then(extract_url)
491+
.map(|repo| issues_url_from_repository(&repo))
492+
}
493+
494+
fn issues_url_from_repository(repo: &str) -> String {
495+
let cleaned = repo.trim_start_matches("git+").trim_end_matches(".git");
496+
497+
// npm shorthand (no scheme, no user@): treat as github-style "owner/repo"
498+
if !cleaned.contains("://") && !cleaned.contains('@') {
499+
let path = cleaned.trim_start_matches("github:");
500+
return format!("https://github.com/{path}/issues");
501+
}
502+
503+
format!("{cleaned}/issues")
504+
}
505+
460506
fn make_package(
461507
config: config::Config,
462508
package_path: &Path,
@@ -1100,7 +1146,7 @@ pub fn validate_packages_dependencies(packages: &AHashMap<String, Package>) -> b
11001146
mod test {
11011147
use crate::config;
11021148

1103-
use super::{Namespace, Package, read_package_name};
1149+
use super::{Namespace, Package, read_issue_tracker_url, read_package_name};
11041150
use ahash::{AHashMap, AHashSet};
11051151
use std::fs;
11061152
use std::path::PathBuf;
@@ -1235,4 +1281,77 @@ mod test {
12351281
)
12361282
);
12371283
}
1284+
1285+
fn write_pkg_json(dir: &std::path::Path, body: &str) {
1286+
fs::write(dir.join("package.json"), body).expect("package.json should be written");
1287+
}
1288+
1289+
#[test]
1290+
fn issue_tracker_url_prefers_bugs_url_string() {
1291+
let temp_dir = TempDir::new().unwrap();
1292+
write_pkg_json(
1293+
temp_dir.path(),
1294+
r#"{"name":"x","bugs":"https://example.com/issues"}"#,
1295+
);
1296+
assert_eq!(
1297+
read_issue_tracker_url(temp_dir.path()),
1298+
Some("https://example.com/issues".to_string())
1299+
);
1300+
}
1301+
1302+
#[test]
1303+
fn issue_tracker_url_prefers_bugs_object_url() {
1304+
let temp_dir = TempDir::new().unwrap();
1305+
write_pkg_json(
1306+
temp_dir.path(),
1307+
r#"{"name":"x","bugs":{"url":"https://example.com/report"}}"#,
1308+
);
1309+
assert_eq!(
1310+
read_issue_tracker_url(temp_dir.path()),
1311+
Some("https://example.com/report".to_string())
1312+
);
1313+
}
1314+
1315+
#[test]
1316+
fn issue_tracker_url_derives_from_repository_git_url() {
1317+
let temp_dir = TempDir::new().unwrap();
1318+
write_pkg_json(
1319+
temp_dir.path(),
1320+
r#"{"name":"x","repository":"git+https://github.com/owner/repo.git"}"#,
1321+
);
1322+
assert_eq!(
1323+
read_issue_tracker_url(temp_dir.path()),
1324+
Some("https://github.com/owner/repo/issues".to_string())
1325+
);
1326+
}
1327+
1328+
#[test]
1329+
fn issue_tracker_url_derives_from_repository_object() {
1330+
let temp_dir = TempDir::new().unwrap();
1331+
write_pkg_json(
1332+
temp_dir.path(),
1333+
r#"{"name":"x","repository":{"type":"git","url":"https://github.com/owner/repo.git"}}"#,
1334+
);
1335+
assert_eq!(
1336+
read_issue_tracker_url(temp_dir.path()),
1337+
Some("https://github.com/owner/repo/issues".to_string())
1338+
);
1339+
}
1340+
1341+
#[test]
1342+
fn issue_tracker_url_handles_shorthand() {
1343+
let temp_dir = TempDir::new().unwrap();
1344+
write_pkg_json(temp_dir.path(), r#"{"name":"x","repository":"owner/repo"}"#);
1345+
assert_eq!(
1346+
read_issue_tracker_url(temp_dir.path()),
1347+
Some("https://github.com/owner/repo/issues".to_string())
1348+
);
1349+
}
1350+
1351+
#[test]
1352+
fn issue_tracker_url_returns_none_without_hints() {
1353+
let temp_dir = TempDir::new().unwrap();
1354+
write_pkg_json(temp_dir.path(), r#"{"name":"x"}"#);
1355+
assert_eq!(read_issue_tracker_url(temp_dir.path()), None);
1356+
}
12381357
}

0 commit comments

Comments
 (0)