Skip to content

Commit d8b903a

Browse files
nojafcknitt
nojaf
authored and
cknitt
committed
Fix warning loss during incremental builds in watch mode (#8216)
* Fix warning loss during incremental builds in watch mode * Add changelog * Update lock file * Remove sleep * Update existing snapshots with added warning * Rename field # Conflicts: # CHANGELOG.md # rewatch/tests/02-watch-warnings-persist.sh # rewatch/tests/snapshots/bs-dev-dependency-used-by-non-dev-source.txt # rewatch/tests/snapshots/clean-rebuild.txt # rewatch/tests/snapshots/dependency-cycle.txt # rewatch/tests/snapshots/remove-file.txt # rewatch/tests/snapshots/rename-file-internal-dep-namespace.txt # rewatch/tests/snapshots/rename-file-internal-dep.txt # rewatch/tests/snapshots/rename-file-with-interface.txt # rewatch/tests/snapshots/rename-file.txt # rewatch/tests/snapshots/rename-interface-file.txt # rewatch/tests/suffix.sh # rewatch/tests/suite.sh
1 parent 4a79ce9 commit d8b903a

24 files changed

Lines changed: 230 additions & 34 deletions

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
- Add duplicate package detection to rewatch. https://github.com/rescript-lang/rescript/pull/8180
3030
- Rewatch: do not warn about "reanalyze" config field. https://github.com/rescript-lang/rescript/pull/8181
3131
- Fix error when importing CommonJS runtime modules with `require()`. https://github.com/rescript-lang/rescript/pull/8194
32+
- Rewatch: fix warnings from non-recompiled modules being lost during incremental builds in watch mode. https://github.com/rescript-lang/rescript/pull/8216
3233

3334
#### :memo: Documentation
3435

‎rewatch/src/build/build_types.rs‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,10 @@ pub struct Interface {
2727
pub compile_state: CompileState,
2828
pub last_modified: SystemTime,
2929
pub parse_dirty: bool,
30+
/// Compiler warning output (from bsc stderr) stored for re-emission
31+
/// during incremental builds when this module is not recompiled.
32+
/// Written to `.compiler.log` on each build cycle.
33+
pub compile_warnings: Option<String>,
3034
}
3135

3236
#[derive(Debug, Clone, PartialEq)]
@@ -36,6 +40,10 @@ pub struct Implementation {
3640
pub compile_state: CompileState,
3741
pub last_modified: SystemTime,
3842
pub parse_dirty: bool,
43+
/// Compiler warning output (from bsc stderr) stored for re-emission
44+
/// during incremental builds when this module is not recompiled.
45+
/// Written to `.compiler.log` on each build cycle.
46+
pub compile_warnings: Option<String>,
3947
}
4048

4149
#[derive(Debug, Clone, PartialEq)]

‎rewatch/src/build/compile.rs‎

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -327,15 +327,19 @@ pub fn compile(
327327
}
328328
SourceType::SourceFile(ref mut source_file) => match result {
329329
Ok(Some(err)) => {
330+
let warning_text = err.to_string();
330331
source_file.implementation.compile_state = CompileState::Warning;
331-
(Some(err.to_string()), None)
332+
source_file.implementation.compile_warnings = Some(warning_text.clone());
333+
(Some(warning_text), None)
332334
}
333335
Ok(None) => {
334336
source_file.implementation.compile_state = CompileState::Success;
337+
source_file.implementation.compile_warnings = None;
335338
(None, None)
336339
}
337340
Err(err) => {
338341
source_file.implementation.compile_state = CompileState::Error;
342+
source_file.implementation.compile_warnings = None;
339343
(None, Some(err.to_string()))
340344
}
341345
},
@@ -345,17 +349,22 @@ pub fn compile(
345349
if let SourceType::SourceFile(ref mut source_file) = module.source_type {
346350
match interface_result {
347351
Some(Ok(Some(err))) => {
352+
let warning_text = err.to_string();
348353
source_file.interface.as_mut().unwrap().compile_state = CompileState::Warning;
349-
(Some(err.to_string()), None)
354+
source_file.interface.as_mut().unwrap().compile_warnings =
355+
Some(warning_text.clone());
356+
(Some(warning_text), None)
350357
}
351358
Some(Ok(None)) => {
352359
if let Some(interface) = source_file.interface.as_mut() {
353360
interface.compile_state = CompileState::Success;
361+
interface.compile_warnings = None;
354362
}
355363
(None, None)
356364
}
357365
Some(Err(err)) => {
358366
source_file.interface.as_mut().unwrap().compile_state = CompileState::Error;
367+
source_file.interface.as_mut().unwrap().compile_warnings = None;
359368
(None, Some(err.to_string()))
360369
}
361370
_ => (None, None),
@@ -434,6 +443,32 @@ pub fn compile(
434443
};
435444
}
436445

446+
// Collect warnings from modules that were not recompiled in this build
447+
// but still have stored warnings from a previous compilation.
448+
// This ensures warnings are not lost during incremental builds in watch mode.
449+
for (module_name, module) in build_state.modules.iter() {
450+
if compile_universe.contains(module_name) {
451+
continue;
452+
}
453+
if let SourceType::SourceFile(ref source_file) = module.source_type {
454+
let package = build_state.get_package(&module.package_name);
455+
if let Some(ref warning) = source_file.implementation.compile_warnings {
456+
if let Some(package) = package {
457+
logs::append(package, warning);
458+
}
459+
compile_warnings.push_str(warning);
460+
}
461+
if let Some(ref interface) = source_file.interface
462+
&& let Some(ref warning) = interface.compile_warnings
463+
{
464+
if let Some(package) = package {
465+
logs::append(package, warning);
466+
}
467+
compile_warnings.push_str(warning);
468+
}
469+
}
470+
}
471+
437472
Ok((compile_errors, compile_warnings, num_compiled_modules))
438473
}
439474

‎rewatch/src/build/packages.rs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -806,6 +806,7 @@ pub fn parse_packages(build_state: &mut BuildState) -> Result<()> {
806806
compile_state: CompileState::Pending,
807807
last_modified: metadata.modified,
808808
parse_dirty: true,
809+
compile_warnings: None,
809810
},
810811
interface: None,
811812
}),
@@ -877,6 +878,7 @@ pub fn parse_packages(build_state: &mut BuildState) -> Result<()> {
877878
compile_state: CompileState::Pending,
878879
last_modified: metadata.modified,
879880
parse_dirty: true,
881+
compile_warnings: None,
880882
});
881883
}
882884
})
@@ -890,13 +892,15 @@ pub fn parse_packages(build_state: &mut BuildState) -> Result<()> {
890892
compile_state: CompileState::Pending,
891893
last_modified: metadata.modified,
892894
parse_dirty: true,
895+
compile_warnings: None,
893896
},
894897
interface: Some(Interface {
895898
path: file.to_owned(),
896899
parse_state: ParseState::Pending,
897900
compile_state: CompileState::Pending,
898901
last_modified: metadata.modified,
899902
parse_dirty: true,
903+
compile_warnings: None,
900904
}),
901905
}),
902906
deps: AHashSet::new(),

‎rewatch/testrepo/package.json‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,8 @@
1616
"packages/pure-dev",
1717
"packages/with-ppx",
1818
"packages/nohoist",
19-
"packages/standalone"
19+
"packages/standalone",
20+
"packages/watch-warnings"
2021
],
2122
"nohoist": [
2223
"rescript-bun"

‎rewatch/testrepo/packages/watch-warnings/package.json‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
{
2+
"name": "@testrepo/watch-warnings",
3+
"version": "0.0.1"
4+
}

‎rewatch/testrepo/packages/watch-warnings/rescript.json‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
{
2+
"name": "@testrepo/watch-warnings",
3+
"sources": {
4+
"dir": "src",
5+
"subdirs": true
6+
},
7+
"package-specs": {
8+
"module": "commonjs",
9+
"in-source": true
10+
},
11+
"suffix": ".bs.js"
12+
}

‎rewatch/testrepo/packages/watch-warnings/src/B.mjs‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
// Generated by ReScript, PLEASE EDIT WITH CARE
2+
3+
4+
function world() {
5+
console.log("world");
6+
}
7+
8+
export {
9+
world,
10+
}
11+
/* No side effect */

‎rewatch/testrepo/packages/watch-warnings/src/B.res‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
let world = () => Console.log("world")

‎rewatch/testrepo/packages/watch-warnings/src/ModuleA.mjs‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
// Generated by ReScript, PLEASE EDIT WITH CARE
2+
3+
4+
function hello() {
5+
console.log("hello");
6+
}
7+
8+
export {
9+
hello,
10+
}
11+
/* No side effect */

0 commit comments

Comments
 (0)