From 1f023f00cf53317cef941792b1d89998cc54ce29 Mon Sep 17 00:00:00 2001 From: aiibe Date: Fri, 26 Jun 2026 11:46:23 +0200 Subject: [PATCH 1/2] fix: deleting an island wedges the dev server --- .../src/plugins/server_snapshot.ts | 65 +++++++++++-------- packages/plugin-vite/tests/dev_server_test.ts | 35 ++++++++++ .../fixtures/delete_island/islands/Extra.tsx | 10 +++ .../tests/fixtures/delete_island/main.ts | 3 + .../fixtures/delete_island/routes/index.tsx | 3 + 5 files changed, 89 insertions(+), 27 deletions(-) create mode 100644 packages/plugin-vite/tests/fixtures/delete_island/islands/Extra.tsx create mode 100644 packages/plugin-vite/tests/fixtures/delete_island/main.ts create mode 100644 packages/plugin-vite/tests/fixtures/delete_island/routes/index.tsx diff --git a/packages/plugin-vite/src/plugins/server_snapshot.ts b/packages/plugin-vite/src/plugins/server_snapshot.ts index ae1487652ff..c6bd4496974 100644 --- a/packages/plugin-vite/src/plugins/server_snapshot.ts +++ b/packages/plugin-vite/src/plugins/server_snapshot.ts @@ -50,12 +50,39 @@ export function serverSnapshot(options: ResolvedFreshViteConfig): Plugin[] { const islands = new Map(); const islandsByFile = new Set(); const islandSpecByName = new Map(); + const islandNameBySpec = new Map(); const routeNamer = new UniqueNamer(); const routeFileToName = new Map(); // deno-lint-ignore no-explicit-any const routes: Map> = new Map(); + // Rebuild the module-scoped island maps from scratch; a deleted island would + // otherwise linger and get emitted as a stale `import` in the snapshot. + function rebuildIslands(fileSpecs: string[]): void { + islands.clear(); + islandsByFile.clear(); + islandSpecByName.clear(); + + // Remote islands are re-seeded first. + options.islandSpecifiers.forEach((name, spec) => { + islands.set(spec, { name, chunk: null }); + islandSpecByName.set(name, spec); + }); + + for (const spec of fileSpecs) { + let name = islandNameBySpec.get(spec); + if (name === undefined) { + name = options.namer.getUniqueName(specToName(spec)); + islandNameBySpec.set(spec, name); + } + + islands.set(spec, { name, chunk: null }); + islandSpecByName.set(name, spec); + islandsByFile.add(spec); + } + } + return [ { name: "fresh:server-snapshot", @@ -78,11 +105,7 @@ export function serverSnapshot(options: ResolvedFreshViteConfig): Plugin[] { config.root, ); - options.islandSpecifiers.forEach((name, spec) => { - islands.set(spec, { name, chunk: null }); - islandSpecByName.set(name, spec); - // islandsByFile.add(spec); - }); + rebuildIslands([]); }, configureServer(viteServer) { server = viteServer; @@ -112,21 +135,17 @@ export function serverSnapshot(options: ResolvedFreshViteConfig): Plugin[] { } } - // Check for route files. We need to invalidate the snapshot if - // they are removed or added. - if ( - (ev === "add" || ev === "unlink") && - !/[\\/]+\(_[^)]+\)[\\/]+/.test(filePath) - ) { - const relRoutes = path.relative(options.routeDir, filePath); - if (!relRoutes.startsWith("..")) { + // Check for route and island files. We need to invalidate the + // snapshot if they are removed or added. + if (ev === "add" || ev === "unlink") { + const inRoutes = !/[\\/]+\(_[^)]+\)[\\/]+/.test(filePath) && + !path.relative(options.routeDir, filePath).startsWith(".."); + const inIslands = !path.relative(options.islandsDir, filePath) + .startsWith(".."); + + if (inRoutes || inIslands) { const mod = ssr.moduleGraph.getModuleById(`\0${modName}`); if (mod !== undefined) { - // Clear state - islands.clear(); - islandsByFile.clear(); - islandSpecByName.clear(); - ssr.moduleGraph.invalidateModule(mod); } } @@ -158,15 +177,7 @@ export function serverSnapshot(options: ResolvedFreshViteConfig): Plugin[] { ignore: options.ignore, }); - for (let i = 0; i < result.islands.length; i++) { - const spec = result.islands[i]; - const specName = specToName(spec); - const name = options.namer.getUniqueName(specName); - - islands.set(spec, { name, chunk: null }); - islandSpecByName.set(name, spec); - islandsByFile.add(spec); - } + rebuildIslands(result.islands); for (let i = 0; i < result.routes.length; i++) { const route = result.routes[i]; diff --git a/packages/plugin-vite/tests/dev_server_test.ts b/packages/plugin-vite/tests/dev_server_test.ts index 1572154f96a..1f8e0bca09f 100644 --- a/packages/plugin-vite/tests/dev_server_test.ts +++ b/packages/plugin-vite/tests/dev_server_test.ts @@ -110,6 +110,41 @@ integrationTest( }, ); +// Deleting an auto-discovered island used to wedge the dev server with a +// permanent 500: it lingered in the snapshot's island map as a dead import. +integrationTest( + "vite dev - deleting an unreferenced island does not wedge the server", + async () => { + const fixture = path.join(FIXTURE_DIR, "delete_island"); + await withDevServer(fixture, async (address, dir) => { + // Nothing references the island, but Fresh auto-discovers it. + await withBrowser(async (page) => { + await page.goto(`${address}/`, { waitUntil: "networkidle2" }); + await waitForText(page, "h1", "ok"); + + // Evaluate the client snapshot so the island is wired into the SSR graph. + const snapshot = await fetch(`${address}/@id/fresh:client-snapshot`); + expect(snapshot.status).toEqual(200); + await snapshot.body?.cancel(); + + await Deno.remove(path.join(dir, "islands", "Extra.tsx")); + + // The delete is async, so require several consecutive 200s to be sure. + await waitFor(async () => { + for (let i = 0; i < 5; i++) { + const res = await fetch(`${address}/`); + const text = await res.text(); + expect(res.status).toEqual(200); + expect(text).toContain("ok"); + await new Promise((r) => setTimeout(r, 150)); + } + return true; + }); + }); + }); + }, +); + integrationTest("vite dev - starts without routes/ dir", async () => { const fixture = path.join(FIXTURE_DIR, "no_routes"); await withDevServer(fixture, async (address) => { diff --git a/packages/plugin-vite/tests/fixtures/delete_island/islands/Extra.tsx b/packages/plugin-vite/tests/fixtures/delete_island/islands/Extra.tsx new file mode 100644 index 00000000000..856e7eb0608 --- /dev/null +++ b/packages/plugin-vite/tests/fixtures/delete_island/islands/Extra.tsx @@ -0,0 +1,10 @@ +import { useSignal } from "@preact/signals"; + +export default function Extra() { + const s = useSignal(0); + return ( + + ); +} diff --git a/packages/plugin-vite/tests/fixtures/delete_island/main.ts b/packages/plugin-vite/tests/fixtures/delete_island/main.ts new file mode 100644 index 00000000000..d98e91b183d --- /dev/null +++ b/packages/plugin-vite/tests/fixtures/delete_island/main.ts @@ -0,0 +1,3 @@ +import { App } from "@fresh/core"; + +export const app = new App().fsRoutes(); diff --git a/packages/plugin-vite/tests/fixtures/delete_island/routes/index.tsx b/packages/plugin-vite/tests/fixtures/delete_island/routes/index.tsx new file mode 100644 index 00000000000..00fdc5813f9 --- /dev/null +++ b/packages/plugin-vite/tests/fixtures/delete_island/routes/index.tsx @@ -0,0 +1,3 @@ +export default function Hello() { + return

ok

; +} From 4e6db5d6de3af23dd8ed4a4cea53bf328c92f596 Mon Sep 17 00:00:00 2001 From: aiibe Date: Tue, 30 Jun 2026 23:30:54 +0200 Subject: [PATCH 2/2] fix: deleting an island poisons sibling island's browser chunk --- .../src/plugins/server_snapshot.ts | 17 ++++++---- packages/plugin-vite/tests/dev_server_test.ts | 34 ++++++++++++------- .../fixtures/delete_island/islands/Keep.tsx | 10 ++++++ .../fixtures/delete_island/routes/index.tsx | 9 ++++- 4 files changed, 50 insertions(+), 20 deletions(-) create mode 100644 packages/plugin-vite/tests/fixtures/delete_island/islands/Keep.tsx diff --git a/packages/plugin-vite/src/plugins/server_snapshot.ts b/packages/plugin-vite/src/plugins/server_snapshot.ts index c6bd4496974..6751b427d1c 100644 --- a/packages/plugin-vite/src/plugins/server_snapshot.ts +++ b/packages/plugin-vite/src/plugins/server_snapshot.ts @@ -64,21 +64,23 @@ export function serverSnapshot(options: ResolvedFreshViteConfig): Plugin[] { islandsByFile.clear(); islandSpecByName.clear(); - // Remote islands are re-seeded first. - options.islandSpecifiers.forEach((name, spec) => { + const add = (spec: string, name: string) => { islands.set(spec, { name, chunk: null }); islandSpecByName.set(name, spec); - }); + }; + + // Remote islands are re-seeded first. + options.islandSpecifiers.forEach((name, spec) => add(spec, name)); for (const spec of fileSpecs) { + // Reuse the cached name so the namer doesn't suffix `_1` on each rebuild. let name = islandNameBySpec.get(spec); if (name === undefined) { name = options.namer.getUniqueName(specToName(spec)); islandNameBySpec.set(spec, name); } - islands.set(spec, { name, chunk: null }); - islandSpecByName.set(name, spec); + add(spec, name); islandsByFile.add(spec); } } @@ -207,7 +209,10 @@ export function serverSnapshot(options: ResolvedFreshViteConfig): Plugin[] { const mod = server.environments.client.moduleGraph.getModuleById( id, ); - if (mod !== undefined) { + // Skip a `fresh-island::Name` virtual `mod.url` (from the fallback + // resolver when the client graph was cold): emitting it as the chunk + // leaves a bare import the browser can't resolve. Keep the `/@id/` URL. + if (mod !== undefined && !mod.url.startsWith("fresh-island::")) { const def = islands.get(id); if (def !== undefined) def.chunk = mod.url; } diff --git a/packages/plugin-vite/tests/dev_server_test.ts b/packages/plugin-vite/tests/dev_server_test.ts index 1f8e0bca09f..e9abb6a376c 100644 --- a/packages/plugin-vite/tests/dev_server_test.ts +++ b/packages/plugin-vite/tests/dev_server_test.ts @@ -110,36 +110,44 @@ integrationTest( }, ); -// Deleting an auto-discovered island used to wedge the dev server with a -// permanent 500: it lingered in the snapshot's island map as a dead import. +// Deleting an auto-discovered island used to break the dev server two ways: the +// stale island lingered in the snapshot map as a dead import (permanent 500), +// and the surviving sibling's boot import got rewritten to the unresolvable +// `fresh-island::Name` specifier, killing its hydration after a reload. integrationTest( - "vite dev - deleting an unreferenced island does not wedge the server", + "vite dev - deleting an island recovers and keeps siblings interactive", async () => { const fixture = path.join(FIXTURE_DIR, "delete_island"); await withDevServer(fixture, async (address, dir) => { - // Nothing references the island, but Fresh auto-discovers it. await withBrowser(async (page) => { + // Wire the surviving island into the client graph and hydrate it. await page.goto(`${address}/`, { waitUntil: "networkidle2" }); - await waitForText(page, "h1", "ok"); - - // Evaluate the client snapshot so the island is wired into the SSR graph. - const snapshot = await fetch(`${address}/@id/fresh:client-snapshot`); - expect(snapshot.status).toEqual(200); - await snapshot.body?.cancel(); + await waitForText(page, ".keep", "keep 0"); + await page.locator(".keep").click(); + await waitForText(page, ".keep", "keep 1"); + // Delete the sibling: the watcher rebuilds the island map and reloads. await Deno.remove(path.join(dir, "islands", "Extra.tsx")); - // The delete is async, so require several consecutive 200s to be sure. + // The delete is async, so require several consecutive 200s to be sure + // the server didn't wedge on the dead import. await waitFor(async () => { for (let i = 0; i < 5; i++) { const res = await fetch(`${address}/`); - const text = await res.text(); + const html = await res.text(); expect(res.status).toEqual(200); - expect(text).toContain("ok"); + // Boot import must stay resolvable, no virtual specifier leaked. + expect(/from\s+"fresh-island::/.test(html)).toBe(false); await new Promise((r) => setTimeout(r, 150)); } return true; }); + + // The survivor still hydrates and stays interactive after the reload. + await page.goto(`${address}/`, { waitUntil: "networkidle2" }); + await waitForText(page, ".keep", "keep 0"); + await page.locator(".keep").click(); + await waitForText(page, ".keep", "keep 1"); }); }); }, diff --git a/packages/plugin-vite/tests/fixtures/delete_island/islands/Keep.tsx b/packages/plugin-vite/tests/fixtures/delete_island/islands/Keep.tsx new file mode 100644 index 00000000000..2c8f659cfdc --- /dev/null +++ b/packages/plugin-vite/tests/fixtures/delete_island/islands/Keep.tsx @@ -0,0 +1,10 @@ +import { useSignal } from "@preact/signals"; + +export default function Keep() { + const s = useSignal(0); + return ( + + ); +} diff --git a/packages/plugin-vite/tests/fixtures/delete_island/routes/index.tsx b/packages/plugin-vite/tests/fixtures/delete_island/routes/index.tsx index 00fdc5813f9..12bf2c33617 100644 --- a/packages/plugin-vite/tests/fixtures/delete_island/routes/index.tsx +++ b/packages/plugin-vite/tests/fixtures/delete_island/routes/index.tsx @@ -1,3 +1,10 @@ +import Keep from "../islands/Keep.tsx"; + export default function Hello() { - return

ok

; + return ( +
+

ok

+ +
+ ); }