Skip to content

Commit 3fe9059

Browse files
authored
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
1 parent 4d182e7 commit 3fe9059

26 files changed

Lines changed: 326 additions & 35 deletions

CHANGELOG.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
- Remove the deprecated module system names `es6` and `es6-global` (superseded by `esmodule`). https://github.com/rescript-lang/rescript/pull/8205
2121
- Default to module system `esmodule`. https://github.com/rescript-lang/rescript/pull/8213
2222
- Remove `external-stdlib` configuration option from `rescript.json`. This option was rarely used and is no longer supported.
23-
- Remove the deprecated uncurried `(. args) => ...` function syntax. https://github.com/rescript-lang/rescript/pull/8211
23+
- Remove the deprecated uncurried `(. args) => ...` function syntax. https://github.com/rescript-lang/rescript/pull/8211
2424
- `js-post-build` now passes the correct output file path based on `in-source` configuration: when `in-source: true`, the path next to the source file is passed; when `in-source: false`, the path in the `lib/<module>/` directory is passed. Additionally, stdout and stderr from the post-build command are now logged. https://github.com/rescript-lang/rescript/pull/8190
2525
- `js-post-build` command now runs in the directory containing the `rescript.json` where it is defined, instead of the unpredictable build invocation directory. This provides consistent behavior in monorepos. https://github.com/rescript-lang/rescript/pull/8195
2626
- Remove support for deprecated `bs-dependencies`, `bs-dev-dependencies`, and `bsc-flags` configuration options. Use `dependencies`, `dev-dependencies`, and `compiler-flags` instead. https://github.com/rescript-lang/rescript/pull/8196
@@ -40,6 +40,7 @@
4040
- Add duplicate package detection to rewatch. https://github.com/rescript-lang/rescript/pull/8180
4141
- Rewatch: do not warn about "reanalyze" config field. https://github.com/rescript-lang/rescript/pull/8181
4242
- Fix error when importing CommonJS runtime modules with `require()`. https://github.com/rescript-lang/rescript/pull/8194
43+
- Rewatch: fix warnings from non-recompiled modules being lost during incremental builds in watch mode. https://github.com/rescript-lang/rescript/pull/8216
4344

4445
#### :memo: Documentation
4546

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
@@ -800,6 +800,7 @@ pub fn parse_packages(build_state: &mut BuildState) -> Result<()> {
800800
compile_state: CompileState::Pending,
801801
last_modified: metadata.modified,
802802
parse_dirty: true,
803+
compile_warnings: None,
803804
},
804805
interface: None,
805806
}),
@@ -871,6 +872,7 @@ pub fn parse_packages(build_state: &mut BuildState) -> Result<()> {
871872
compile_state: CompileState::Pending,
872873
last_modified: metadata.modified,
873874
parse_dirty: true,
875+
compile_warnings: None,
874876
});
875877
}
876878
})
@@ -884,13 +886,15 @@ pub fn parse_packages(build_state: &mut BuildState) -> Result<()> {
884886
compile_state: CompileState::Pending,
885887
last_modified: metadata.modified,
886888
parse_dirty: true,
889+
compile_warnings: None,
887890
},
888891
interface: Some(Interface {
889892
path: file.to_owned(),
890893
parse_state: ParseState::Pending,
891894
compile_state: CompileState::Pending,
892895
last_modified: metadata.modified,
893896
parse_dirty: true,
897+
compile_warnings: None,
894898
}),
895899
}),
896900
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"
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+
}
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+
}
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 */
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
let world = () => Console.log("world")
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)