bake: retract a client component's bundling failure when it is demoted

src/runtime/bake/dev_server/incremental_graph.rs

@@ -516,6 +516,25 @@ impl<const SIDE: bake::Side> IncrementalGraph<SIDE> {
         directory_watchers
             .remove_dependencies_for_file(&self.bundled_files.keys()[file_index.get() as usize]);
 
+        // Nothing receives this slot again, so its failure is retracted here.
+        let mut file =
+            core::mem::take(&mut self.bundled_files.values_mut()[file_index.get() as usize]);
+        let key = bun_ptr::RawSlice::new(&*self.bundled_files.keys()[file_index.get() as usize]);
+        self.free_file_content(key.slice(), &mut file, FreeCssMode::UnrefCss);
+        file.kind = FileKind::Unknown;
+        if file.failed {
+            file.failed = false;
+            let owner = serialized_failure::OwnerPacked::new(SIDE, file_index.get());
+            let kv = self.dev_bundling_failures().fetch_swap_remove(&owner);
+            let kv = kv.unwrap_or_else(|| {
+                bun_core::Output::panic(format_args!(
+                    "Missing SerializedFailure in IncrementalGraph",
+                ))
+            });
+            self.dev_incremental_result().failures_removed.push(kv.1);
+        }
+        self.bundled_files.values_mut()[file_index.get() as usize] = file;
+
         // Free the key string and tombstone the slot. Cannot swap-remove since
         // FrameworkRouter / SerializedFailure hold FileIndices into this graph.
         // Tombstoned slots are not reused; a free-list is a pending idea.

test/bake/dev/bundle.test.ts

@@ -1,6 +1,7 @@
 // Bundle tests are tests concerning bundling bugs that only occur in DevServer.
+import type { Bake } from "bun";
 import { expect } from "bun:test";
-import { devTest, emptyHtmlFile, minimalFramework } from "../bake-harness";
+import { devTest, emptyHtmlFile, minimalFramework, type Dev } from "../bake-harness";
 
 devTest("import identifier doesnt get renamed", {
   framework: minimalFramework,
@@ -358,6 +359,242 @@ devTest("removing 'use client' from a component with a pending resolution failur
     expect(res).toBeInstanceOf(Response);
   },
 });
+/**
+ * Decodes concatenated `SerializedFailure`s (see
+ * src/runtime/bake/dev_server/serialized_failure.rs). This is the layout of
+ * both the "added" tail of a `MessageId.errors` packet and the payload embedded
+ * in the "Build Failed" page.
+ */
+function decodeSerializedFailures(bytes: Uint8Array) {
+  const view = new DataView(bytes.buffer, bytes.byteOffset, bytes.byteLength);
+  let pos = 0;
+  const u32 = () => {
+    const value = view.getUint32(pos, true);
+    pos += 4;
+    return value;
+  };
+  const string32 = () => {
+    const length = u32();
+    const text = new TextDecoder().decode(bytes.subarray(pos, pos + length));
+    pos += length;
+    return text;
+  };
+  const logData = () => {
+    const text = string32();
+    // A zero line means there is no location.
+    if (u32() !== 0) {
+      u32(); // column
+      u32(); // length
+      string32(); // line text
+    }
+    return text;
+  };
+  const failures: { owner: number; file: string; messages: string[] }[] = [];
+  while (pos < bytes.byteLength) {
+    const owner = u32();
+    const file = string32();
+    const messages: string[] = [];
+    for (let messageCount = u32(); messageCount > 0; messageCount--) {
+      pos += 1; // ErrorKind
+      messages.push(logData());
+      for (let noteCount = u32(); noteCount > 0; noteCount--) {
+        logData();
+      }
+    }
+    failures.push({ owner, file, messages });
+  }
+  return failures;
+}
+/** Decodes a `MessageId.errors` packet from the HMR socket. */
+function decodeErrorsPacket(data: Uint8Array) {
+  const view = new DataView(data.buffer, data.byteOffset, data.byteLength);
+  const removedCount = view.getUint32(1, true);
+  const removed: number[] = [];
+  for (let i = 0; i < removedCount; i++) {
+    removed.push(view.getUint32(5 + i * 4, true));
+  }
+  return { removed, added: decodeSerializedFailures(data.subarray(5 + removedCount * 4)) };
+}
+/**
+ * Subscribes the harness socket to the errors topic and records every errors
+ * packet. Each `dev.write` resolves only after a later message on this socket,
+ * so by then the packets of that rebuild have been recorded.
+ */
+function recordErrorsPackets(dev: Dev) {
+  const packets: ReturnType<typeof decodeErrorsPacket>[] = [];
+  dev.on("hmr", (data: Uint8Array) => {
+    if (data[0] === "e".charCodeAt(0)) packets.push(decodeErrorsPacket(data));
+  });
+  dev.socket!.send("sre");
+  return packets;
+}
+// separateSSRGraph makes a "use client" file's own bundling failures belong to
+// the client graph's node, which is the node deleted when the file is demoted.
+const separateSSRGraphFramework: Bake.Framework = {
+  ...minimalFramework,
+  serverComponents: {
+    ...minimalFramework.serverComponents!,
+    separateSSRGraph: true,
+  },
+};
+// The route sees a client reference object while Comp.ts is a client
+// component boundary and the exported string once it is demoted.
+const demotionFiles = {
+  "routes/index.ts": `
+    import * as Comp from '../components/Comp';
+    export default function (req, meta) {
+      return new Response('marker: ' + typeof Comp.marker);
+    }
+  `,
+  "components/Comp.ts": `
+    "use client";
+    export const marker = "initial";
+  `,
+};
+devTest("removing 'use client' from a working component", {
+  framework: separateSSRGraphFramework,
+  files: demotionFiles,
+  async test(dev) {
+    const errorPackets = recordErrorsPackets(dev);
+    await dev.fetch("/").equals("marker: object");
+    await dev.write("components/Comp.ts", `export const marker = "plain";`);
+    await dev.fetch("/").equals("marker: string");
+    expect(errorPackets).toEqual([]);
+  },
+});
+// When a client component boundary is demoted, the server graph deletes the
+// client graph's node for it (disconnectAndDeleteFile). If that node was
+// failing, its failure used to stay behind in dev.bundling_failures: error
+// overlays were never told to drop it and every later "Build Failed" page
+// listed it again.
+devTest("removing 'use client' from a failing component retracts its bundling failure", {
+  framework: separateSSRGraphFramework,
+  files: demotionFiles,
+  async test(dev) {
+    const errorPackets = recordErrorsPackets(dev);
+    await dev.fetch("/").equals("marker: object");
+    expect(errorPackets).toEqual([]);
+
+    // Break the component while it is still a boundary. The unresolvable
+    // import is attributed to the client graph's node.
+    await dev.write(
+      "components/Comp.ts",
+      `
+        "use client";
+        import './missing';
+        export const marker = "initial";
+      `,
+      { errors: null },
+    );
+    expect(errorPackets).toEqual([
+      {
+        removed: [],
+        added: [
+          {
+            owner: expect.any(Number),
+            file: "components/Comp.ts",
+            messages: ['Could not resolve: "./missing"'],
+          },
+        ],
+      },
+    ]);
+    const compOwner = errorPackets[0].added[0].owner;
+    errorPackets.length = 0;
+
+    // Demote the component and fix it in the same edit. Only the server
+    // re-bundles the file; the client graph's node is deleted along with
+    // the failure it owned, which must be announced as removed.
+    await dev.write("components/Comp.ts", `export const marker = "plain";`);
+    expect(errorPackets).toEqual([{ removed: [compOwner], added: [] }]);
+    await dev.fetch("/").equals("marker: string");
+
+    // An unrelated failure later on renders every failure the dev server
+    // still tracks. The retracted one must not be among them.
+    await dev.write(
+      "routes/index.ts",
+      `
+        import * as Comp from '../components/Comp';
+        import './does-not-exist';
+        export default function (req, meta) {
+          return new Response('marker: ' + typeof Comp.marker);
+        }
+      `,
+      { errors: null },
+    );
+    const response = await dev.fetch("/");
+    expect(response.status).toBe(500);
+    const [, encodedFailures] = (await response.text()).match(/atob\("([^"]*)"\)/)!;
+    expect(decodeSerializedFailures(Buffer.from(encodedFailures, "base64"))).toEqual([
+      {
+        owner: expect.any(Number),
+        file: "routes/index.ts",
+        messages: ['Could not resolve: "./does-not-exist"'],
+      },
+    ]);
+  },
+});
+// Same demotion as above, observed from a browser sitting on the "Build
+// Failed" page: the retraction is what lets it drop the error and reload.
+devTest("removing 'use client' from a failing component clears the error overlay", {
+  framework: {
+    fileSystemRouterTypes: [
+      {
+        root: "routes",
+        style: "nextjs-pages",
+        serverEntryPoint: "./framework/server.ts",
+        clientEntryPoint: "./framework/client.ts",
+      },
+    ],
+    serverComponents: {
+      separateSSRGraph: true,
+      serverRuntimeImportSource: "./framework/server.ts",
+      serverRegisterClientReferenceExport: "registerClientReference",
+    },
+  },
+  files: {
+    "framework/server.ts": `
+      export function render(req, meta) {
+        const scripts = meta.modules.map(src => '<script type="module" src="' + src + '"></script>').join("");
+        return new Response("<!DOCTYPE html><html><body>" + meta.pageModule.default() + scripts + "</body></html>", {
+          headers: { "Content-Type": "text/html" },
+        });
+      }
+      export function registerClientReference(value, file, uid) {
+        return { value, file, uid };
+      }
+    `,
+    "framework/client.ts": `
+      console.log("marker: " + document.body.textContent);
+    `,
+    "routes/index.ts": `
+      import * as Comp from '../components/Comp';
+      export default () => typeof Comp.marker;
+    `,
+    "components/Comp.ts": `
+      "use client";
+      export const marker = "initial";
+    `,
+  },
+  async test(dev) {
+    await dev.fetch("/").expect.toInclude("<body>object<");
+    await dev.write(
+      "components/Comp.ts",
+      `
+        "use client";
+        import './missing';
+        export const marker = "initial";
+      `,
+      { errors: null },
+    );
+    await using c = await dev.client("/", {
+      errors: ['components/Comp.ts:2:8: error: Could not resolve: "./missing"'],
+    });
+    await c.expectReload(async () => {
+      await dev.write("components/Comp.ts", `export const marker = "plain";`);
+    });
+    await c.expectMessage("marker: string");
+  },
+});
 devTest("deinit with a free-list slot in DirectoryWatchStore.dependencies", {
   files: {
     "index.html": emptyHtmlFile({ scripts: ["index.ts"] }),