diff --git a/src/runtime/bake/dev_server/incremental_graph.rs b/src/runtime/bake/dev_server/incremental_graph.rs index 3522f6447791..b00e1c1623e6 100644 --- a/src/runtime/bake/dev_server/incremental_graph.rs +++ b/src/runtime/bake/dev_server/incremental_graph.rs @@ -516,6 +516,25 @@ impl IncrementalGraph { 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. diff --git a/test/bake/dev/bundle.test.ts b/test/bake/dev/bundle.test.ts index 6c6d3657029c..3bdf472eeaa7 100644 --- a/test/bake/dev/bundle.test.ts +++ b/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[] = []; + 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 => '').join(""); + return new Response("" + meta.pageModule.default() + scripts + "", { + 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("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"] }),