fix(hot): publish sync instead of built for unchanged bundles (#2357)

* fix(hot): publish sync instead of built for unchanged bundles

With a MultiCompiler, rebuilding one child re-emits done for every
child. Clients of the unchanged bundles then try to fetch a hot-update
manifest that was never emitted (404 / "Cannot find update", or an
unwanted full reload with reload=true).

Compare each bundle's hash against the previous build and announce
unchanged bundles as `sync` instead of `built`. The first build still
publishes `built`, and new clients are still caught up with `sync` —
but no longer while a rebuild is in progress.

Ref webpack/webpack-hot-middleware#312

* fixup!

Author: bjohansebas

src/hot.js

@@ -165,13 +165,14 @@ function formatErrors(errors) {
 /**
  * @param {Stats} stats stats
  * @param {StatsOptions} statsOptions stats options
- * @returns {StatsCompilation} json stats with compilation reference attached
+ * @returns {StatsCompilation} json stats with the compilation name resolved
  */
 function normalizeStats(stats, statsOptions) {
   const statsJson = stats.toJson(statsOptions);
 
-  if (stats.compilation) {
-    statsJson.compilation = stats.compilation;
+  // Resolved here so stored bundles do not retain Compilation objects.
+  if (!statsJson.name && stats.compilation) {
+    statsJson.name = stats.compilation.name || "";
   }
 
   return statsJson;
@@ -194,12 +195,11 @@ function extractBundles(stats) {
 }
 
 /**
- * @param {string} action action
  * @param {Stats | MultiStats} statsResult stats result
- * @param {EventStream} eventStream event stream
  * @param {StatsOptions | undefined} statsOptions stats options
+ * @returns {StatsCompilation[]} normalized per-bundle stats
  */
-function publishStats(action, statsResult, eventStream, statsOptions) {
+function toBundles(statsResult, statsOptions) {
   const resultStatsOptions = {
     all: false,
     hash: true,
@@ -209,29 +209,36 @@ function publishStats(action, statsResult, eventStream, statsOptions) {
     ...(statsOptions && typeof statsOptions === "object" ? statsOptions : {}),
   };
 
-  /** @type {StatsCompilation[]} */
-  let bundles;
-
   // Multi-compiler stats have stats for each child compiler.
   if ("stats" in statsResult) {
-    bundles = statsResult.stats.flatMap((stats) =>
+    return statsResult.stats.flatMap((stats) =>
       extractBundles(normalizeStats(stats, resultStatsOptions)),
     );
-  } else {
-    bundles = extractBundles(normalizeStats(statsResult, resultStatsOptions));
   }
 
-  for (const stats of bundles) {
-    let name = stats.name || "";
+  return extractBundles(normalizeStats(statsResult, resultStatsOptions));
+}
 
-    // Fallback to compilation name when there is a single bundle.
-    if (!name && stats.compilation) {
-      name = stats.compilation.name || "";
-    }
+/**
+ * Publish one event per bundle. Bundles whose hash did not change are
+ * published as `sync`, so their clients do not fetch a hot-update manifest
+ * that was never emitted.
+ * @param {StatsCompilation[]} bundles bundles from the current build
+ * @param {StatsCompilation[] | null} previousBundles bundles from the previous build (null on the first build, which publishes everything as `built`)
+ * @param {EventStream} eventStream event stream
+ */
+function publishBundles(bundles, previousBundles, eventStream) {
+  for (const [index, stats] of bundles.entries()) {
+    const name = stats.name || "";
+
+    const changed =
+      previousBundles === null ||
+      !previousBundles[index] ||
+      previousBundles[index].hash !== stats.hash;
 
     eventStream.publish({
       name,
-      action,
+      action: changed ? "built" : "sync",
       time: stats.time,
       hash: stats.hash,
       warnings: formatErrors(stats.warnings || []),
@@ -263,8 +270,10 @@ function createHot(compiler, userOptions) {
   let eventStream = createEventStream(heartbeat, logger);
   logger.log(`Hot module replacement enabled, serving events at "${path}"`);
 
-  /** @type {Stats | MultiStats | null} */
-  let latestStats = null;
+  // `latestBundles` survives rebuilds so hashes can be compared per build.
+  /** @type {StatsCompilation[] | null} */
+  let latestBundles = null;
+  let valid = false;
   let closed = false;
   let lastProgressPercent = -1;
 
@@ -293,7 +302,7 @@ function createHot(compiler, userOptions) {
   const onInvalid = (fileName) => {
     if (closed) return;
 
-    latestStats = null;
+    valid = false;
     lastProgressPercent = -1;
 
     /** @type {{ action: string, file?: string }} */
@@ -312,8 +321,11 @@ function createHot(compiler, userOptions) {
   const onDone = (statsResult) => {
     if (closed) return;
 
-    latestStats = statsResult;
-    publishStats("built", latestStats, eventStream, statsOptions);
+    const bundles = toBundles(statsResult, statsOptions);
+
+    publishBundles(bundles, latestBundles, eventStream);
+    latestBundles = bundles;
+    valid = true;
   };
 
   compiler.hooks.invalid.tap(PLUGIN_NAME, onInvalid);
@@ -326,8 +338,9 @@ function createHot(compiler, userOptions) {
 
       eventStream.handler(req, res);
 
-      if (latestStats) {
-        publishStats("sync", latestStats, eventStream, statsOptions);
+      // Catch the new client up; self-comparison publishes everything as `sync`.
+      if (valid && latestBundles) {
+        publishBundles(latestBundles, latestBundles, eventStream);
       }
     },
     publish(payload) {
@@ -354,4 +367,5 @@ module.exports.createEventStream = createEventStream;
 module.exports.createHot = createHot;
 module.exports.formatErrors = formatErrors;
 module.exports.pathMatch = pathMatch;
-module.exports.publishStats = publishStats;
+module.exports.publishBundles = publishBundles;
+module.exports.toBundles = toBundles;

test/hot.test.js

@@ -37,7 +37,7 @@ function makeFakeCompiler(logger = noopLogger) {
 }
 
 /**
- * Build a minimal Stats-like object that satisfies `publishStats`.
+ * Build a minimal Stats-like object that satisfies `toBundles`.
  * @param {EXPECTED_OBJECT=} overrides field overrides applied on top of the defaults
  * @returns {EXPECTED_OBJECT} fake stats
  */
@@ -347,6 +347,100 @@ describe("createHot", () => {
     hot.close();
   });
 
+  it("publishes sync instead of built when a bundle's hash did not change", () => {
+    const compiler = makeFakeCompiler();
+    const hot = createHot(compiler, {});
+    const { writes } = attachClient({ handler: hot.handle });
+
+    compiler.emitDone(makeFakeStats({ hash: "same" }));
+    writes.length = 0;
+
+    // A rebuild that produced the same hash (e.g. another bundle of a
+    // multi-compiler changed) must not be announced as `built`.
+    compiler.emitInvalid();
+    compiler.emitDone(makeFakeStats({ hash: "same" }));
+
+    expect(writes.some((w) => w.includes('"action":"sync"'))).toBe(true);
+    expect(writes.some((w) => w.includes('"action":"built"'))).toBe(false);
+
+    hot.close();
+  });
+
+  it("publishes built when the bundle's hash changed", () => {
+    const compiler = makeFakeCompiler();
+    const hot = createHot(compiler, {});
+    const { writes } = attachClient({ handler: hot.handle });
+
+    compiler.emitDone(makeFakeStats({ hash: "one" }));
+    writes.length = 0;
+
+    compiler.emitInvalid();
+    compiler.emitDone(makeFakeStats({ hash: "two" }));
+
+    expect(
+      writes.some(
+        (w) => w.includes('"action":"built"') && w.includes('"hash":"two"'),
+      ),
+    ).toBe(true);
+
+    hot.close();
+  });
+
+  it("publishes built only for the changed bundles of a multi-compiler", () => {
+    const compiler = makeFakeCompiler();
+    const hot = createHot(compiler, {});
+    const { writes } = attachClient({ handler: hot.handle });
+
+    compiler.emitDone({
+      stats: [
+        makeFakeStats({ name: "app", hash: "app-1" }),
+        makeFakeStats({ name: "admin", hash: "admin-1" }),
+      ],
+    });
+    writes.length = 0;
+
+    // Only "admin" rebuilt.
+    compiler.emitInvalid();
+    compiler.emitDone({
+      stats: [
+        makeFakeStats({ name: "app", hash: "app-1" }),
+        makeFakeStats({ name: "admin", hash: "admin-2" }),
+      ],
+    });
+
+    expect(
+      writes.some(
+        (w) => w.includes('"name":"app"') && w.includes('"action":"sync"'),
+      ),
+    ).toBe(true);
+    expect(
+      writes.some(
+        (w) => w.includes('"name":"admin"') && w.includes('"action":"built"'),
+      ),
+    ).toBe(true);
+    expect(
+      writes.some(
+        (w) => w.includes('"name":"app"') && w.includes('"action":"built"'),
+      ),
+    ).toBe(false);
+
+    hot.close();
+  });
+
+  it("does not sync new clients while a rebuild is in progress", () => {
+    const compiler = makeFakeCompiler();
+    const hot = createHot(compiler, {});
+
+    compiler.emitDone(makeFakeStats());
+    compiler.emitInvalid();
+
+    const { writes } = attachClient({ handler: hot.handle });
+
+    expect(writes.some((w) => w.includes('"action":"sync"'))).toBe(false);
+
+    hot.close();
+  });
+
   it("sends a sync payload to a client that connects after a build", () => {
     const compiler = makeFakeCompiler();
     const hot = createHot(compiler, {});

test/middleware.test.js

@@ -1,4 +1,5 @@
 import fs from "node:fs";
+import os from "node:os";
 import path from "node:path";
 
 import Hapi from "@hapi/hapi";
@@ -7117,7 +7118,85 @@ describe.each([
         }
       });
 
-      it("streams building and built events on recompilation", async () => {
+      it("publishes built only for the changed bundle of a MultiCompiler", async () => {
+        // Real watch scenario: two compilers, only one source file changes.
+
+        const srcDir = fs.mkdtempSync(
+          path.join(fs.realpathSync.native(os.tmpdir()), "wdm-hot-multi-"),
+        );
+        const appEntry = path.join(srcDir, "app.js");
+        const adminEntry = path.join(srcDir, "admin.js");
+
+        fs.writeFileSync(appEntry, "console.log('app v1');");
+        fs.writeFileSync(adminEntry, "console.log('admin v1');");
+
+        const compiler = getCompiler([
+          {
+            mode: "development",
+            name: "app",
+            entry: appEntry,
+            output: {
+              filename: "bundle.js",
+              path: path.resolve(__dirname, "./outputs/hot-multi-app"),
+            },
+          },
+          {
+            mode: "development",
+            name: "admin",
+            entry: adminEntry,
+            output: {
+              filename: "bundle.js",
+              path: path.resolve(__dirname, "./outputs/hot-multi-admin"),
+            },
+          },
+        ]);
+
+        [server, req, instance] = await frameworkFactory(
+          name,
+          framework,
+          compiler,
+          { hot: true },
+        );
+
+        try {
+          await waitUntilValid(instance);
+
+          const pending = readSseEvents(server, "/__webpack_hmr", 3000);
+          // Change only the "admin" source once the client is listening.
+          setTimeout(() => {
+            fs.writeFileSync(adminEntry, "console.log('admin v2');");
+          }, 300);
+          const events = await pending;
+
+          const buildingIndex = events.findIndex(
+            (event) => event.action === "building",
+          );
+
+          expect(buildingIndex).toBeGreaterThan(-1);
+
+          const afterRebuild = events.slice(buildingIndex + 1);
+
+          expect(
+            afterRebuild.some(
+              (event) => event.name === "admin" && event.action === "built",
+            ),
+          ).toBe(true);
+          expect(
+            afterRebuild.some(
+              (event) => event.name === "app" && event.action === "sync",
+            ),
+          ).toBe(true);
+          expect(
+            afterRebuild.some(
+              (event) => event.name === "app" && event.action === "built",
+            ),
+          ).toBe(false);
+        } finally {
+          fs.rmSync(srcDir, { recursive: true, force: true });
+        }
+      });
+
+      it("streams building and sync events when recompilation does not change the hash", async () => {
         const compiler = getCompiler({ ...webpackConfig, watch: true });
         [server, req, instance] = await frameworkFactory(
           name,
@@ -7135,7 +7214,10 @@ describe.each([
         const actions = events.map((event) => event.action);
 
         expect(actions).toContain("building");
-        expect(actions).toContain("built");
+        expect(
+          actions.filter((action) => action === "sync").length,
+        ).toBeGreaterThanOrEqual(2);
+        expect(actions).not.toContain("built");
       });
 
       it("broadcasts to every connected client", async () => {

types/hot.d.ts

@@ -23,7 +23,8 @@ declare namespace createHot {
     createHot,
     formatErrors,
     pathMatch,
-    publishStats,
+    publishBundles,
+    toBundles,
     HotInstance,
     Compiler,
     MultiCompiler,
@@ -98,17 +99,27 @@ declare function formatErrors(errors: (string | StatsError)[]): string[];
  */
 declare function pathMatch(url: string | undefined, expected: string): boolean;
 /**
- * @param {string} action action
- * @param {Stats | MultiStats} statsResult stats result
+ * Publish one event per bundle. Bundles whose hash did not change are
+ * published as `sync`, so their clients do not fetch a hot-update manifest
+ * that was never emitted.
+ * @param {StatsCompilation[]} bundles bundles from the current build
+ * @param {StatsCompilation[] | null} previousBundles bundles from the previous build (null on the first build, which publishes everything as `built`)
  * @param {EventStream} eventStream event stream
+ */
+declare function publishBundles(
+  bundles: StatsCompilation[],
+  previousBundles: StatsCompilation[] | null,
+  eventStream: EventStream,
+): void;
+/**
+ * @param {Stats | MultiStats} statsResult stats result
  * @param {StatsOptions | undefined} statsOptions stats options
+ * @returns {StatsCompilation[]} normalized per-bundle stats
  */
-declare function publishStats(
-  action: string,
+declare function toBundles(
   statsResult: Stats | MultiStats,
-  eventStream: EventStream,
   statsOptions: StatsOptions | undefined,
-): void;
+): StatsCompilation[];
 type HotInstance = {
   /**
    * path the SSE endpoint is served at