From 66ec1ca2b1e15707f0ec2240dd2ec79087fa6cb6 Mon Sep 17 00:00:00 2001 From: Tomi Virtanen Date: Thu, 3 Sep 2026 16:25:07 +0300 Subject: [PATCH] fix(devloop): recompile the app when a reactor sibling leaves the loop The module set is derived from the app's resolved classpath, so dropping a sibling dependency removes both the sibling's target/classes and the sibling itself from the loop. That module-set change rebuilt Compile, whose constructor re-seeded compiledAgainst from the project as it now stood - overwriting the app's classpath baseline with the already-moved classpath. classpathForced then saw no movement and forced no recompile, so the apply fell through to the drift restart and reported Stable while the page broke with ClassNotFoundException. An external jar was unaffected: it changes no module, so the baseline survived and the forced recompile fired. Compile now takes the outgoing baseline and carries compiledAgainst over for every module still in the loop. The app module is recompiled whole, and javac's diagnostic ends the apply as Failed before the restart leg runs. Only the classpath baseline is carried; per-file stamps still start afresh, as they describe a different build. Co-Authored-By: Claude Opus 5 (1M context) --- .../vaadin/flow/devloop/daemon/Compile.java | 32 +++++++++++++- .../devloop/daemon/TransactionEngine.java | 6 ++- .../flow/devloop/daemon/CompileTest.java | 44 +++++++++++++++++++ .../devloop/test/it/DevLoopMultiModuleIT.java | 24 ++++++++++ 4 files changed, 103 insertions(+), 3 deletions(-) diff --git a/flow-devloop-daemon/src/main/java/com/vaadin/flow/devloop/daemon/Compile.java b/flow-devloop-daemon/src/main/java/com/vaadin/flow/devloop/daemon/Compile.java index 3b9379a3ad3..03ad009c7b3 100644 --- a/flow-devloop-daemon/src/main/java/com/vaadin/flow/devloop/daemon/Compile.java +++ b/flow-devloop-daemon/src/main/java/com/vaadin/flow/devloop/daemon/Compile.java @@ -246,11 +246,39 @@ private interface Visitor { private final Map compiledAgainst = new java.util.concurrent.ConcurrentHashMap<>(); Compile(Launch.Project project) { + this(project, null); + } + + /** + * Carries what each surviving module was compiled against over from the + * baseline it replaces. + *

+ * The event that rebuilds this instance - a changed module set - is itself + * a classpath change: a reactor sibling is in the loop only while the + * application depends on it, so dropping that dependency drops the module + * too. Seeding the new baseline from the project as it now stands would + * declare that move already compiled, and the apply would restart the + * application against a classpath its own sources no longer compile against + * - the removed type surfacing as a {@code + * ClassNotFoundException} on the next page load rather than as a diagnostic + * from the apply that caused it. Only the classpath baseline is carried; + * the per-file stamps deliberately start afresh, as they describe a + * different build. + * + * @param project + * the resolved build the new baseline describes + * @param previous + * the baseline this instance replaces, or {@code null} for a + * project's first one + */ + Compile(Launch.Project project, Compile previous) { this.modules = List.copyOf(project.modules()); this.frontend = Frontend.of(project.app()); for (Reactor.Module module : modules) { - compiledAgainst.put(module.artifactId(), - Launch.membership(project.compileClasspath(module))); + String carried = previous == null ? null + : previous.compiledAgainst.get(module.artifactId()); + compiledAgainst.put(module.artifactId(), carried != null ? carried + : Launch.membership(project.compileClasspath(module))); } } diff --git a/flow-devloop-daemon/src/main/java/com/vaadin/flow/devloop/daemon/TransactionEngine.java b/flow-devloop-daemon/src/main/java/com/vaadin/flow/devloop/daemon/TransactionEngine.java index 624eabfa255..a117f86270a 100644 --- a/flow-devloop-daemon/src/main/java/com/vaadin/flow/devloop/daemon/TransactionEngine.java +++ b/flow-devloop-daemon/src/main/java/com/vaadin/flow/devloop/daemon/TransactionEngine.java @@ -249,7 +249,11 @@ private Compile compileFor(Launch.Project project, Launch.Log log) { if (current != null) { log.line("module set changed; re-seeding the change baseline"); } - Compile fresh = new Compile(project); + // The outgoing baseline is handed over so the classpath each module + // was compiled against survives the hand-off: a module set changes + // only because the application gained or lost a reactor dependency, + // and that is the very move the compile leg has to see. + Compile fresh = new Compile(project, current); // A baseline built while the app is already running must not swallow a // frontend edit made since it started - that is exactly the "start, // edit, first apply" sequence, and answering "no changes" to it is the diff --git a/flow-devloop-daemon/src/test/java/com/vaadin/flow/devloop/daemon/CompileTest.java b/flow-devloop-daemon/src/test/java/com/vaadin/flow/devloop/daemon/CompileTest.java index 068062a374e..38462deb170 100644 --- a/flow-devloop-daemon/src/test/java/com/vaadin/flow/devloop/daemon/CompileTest.java +++ b/flow-devloop-daemon/src/test/java/com/vaadin/flow/devloop/daemon/CompileTest.java @@ -333,6 +333,50 @@ public class Main { } assertEquals(List.of("app"), compile.classpathChangedModules(after)); } + @Test + void classpathForced_recompilesWhenAReactorSiblingLeavesTheLoop() + throws IOException { + // Dropping a sibling dependency also drops the sibling from the loop, + // because the module set is read off the application's resolved + // classpath. The baseline is rebuilt for that new module set, and a + // baseline seeded from the project as it now stands would call the + // move already compiled - the apply would then restart the app into a + // ClassNotFoundException instead of failing with a diagnostic. + Reactor.Module app = module("app", "Main", """ + package app; + public class Main { + public static String label() { + return shared.Formatter.label(); + } + } + """); + Reactor.Module shared = module("shared", "Formatter", """ + package shared; + public class Formatter { + public static String label() { return "shared"; } + } + """); + Launch.Project before = reactor(List.of(app, shared), + Map.of("app", List.of(shared))); + Compile previous = new Compile(before); + previous.compile( + List.of(source(app, "Main"), source(shared, "Formatter")), + before); + Launch.Project after = reactor(List.of(app), Map.of()); + + Compile compile = new Compile(after, previous); + + assertEquals(List.of("app"), compile.classpathChangedModules(after)); + List forced = compile.classpathForced(after); + assertEquals(List.of(source(app, "Main")), forced); + Compile.Result result = compile.compile(forced, after); + assertFalse(result.success()); + assertTrue( + result.errors().stream() + .anyMatch(error -> error.text().contains("shared")), + () -> "errors: " + result.errors()); + } + @Test void relative_namesASiblingAsTheDeveloperWouldTypeIt() throws IOException { Reactor.Module app = module("app", "Main", """ diff --git a/flow-tests/test-devloop/devloop-app/src/test/java/com/vaadin/flow/devloop/test/it/DevLoopMultiModuleIT.java b/flow-tests/test-devloop/devloop-app/src/test/java/com/vaadin/flow/devloop/test/it/DevLoopMultiModuleIT.java index e289e6469e8..e917b8d1212 100644 --- a/flow-tests/test-devloop/devloop-app/src/test/java/com/vaadin/flow/devloop/test/it/DevLoopMultiModuleIT.java +++ b/flow-tests/test-devloop/devloop-app/src/test/java/com/vaadin/flow/devloop/test/it/DevLoopMultiModuleIT.java @@ -74,6 +74,30 @@ void siblingModuleClassesAreCompiledIntoItsOwnOutput() throws IOException { // DevLoopBrowserIT asserts that half. } + @Test + void droppingTheSiblingDependency_failsBeforeAnythingRestarts() { + // The module set is read off the application's resolved classpath, so + // dropping the dependency drops the sibling from the loop too - and + // rebuilding the baseline for that new module set is where the + // classpath move used to be lost. The apply then restarted the + // application into a ClassNotFoundException that only surfaced on the + // next page load, rather than failing here with the diagnostic javac + // already had. Swapped for another jar rather than deleted so the edit + // stays a single line: a multi-line literal would not match a pom + // checked out with CRLF. + patch.replace(APP.resolve("pom.xml"), + "flow-test-devloop-shared", + "flow-webpush"); + + VaadinDevCli.Outcome outcome = cli.run("apply").assertExitCode(1); + + // No source file changed, so javac only ever sees TaskListView - which + // imports the sibling's formatter - because the whole module whose + // classpath moved is recompiled. + outcome.assertOutputContains("TaskListView.java"); + outcome.assertOutputDoesNotContain("restarting"); + } + @Test void compileErrorInTheSibling_isNamedByItsModule() { patch.replace(FORMATTER, "return dueDate == null",