fix: deleting an island wedges the dev server

packages/plugin-vite/src/plugins/server_snapshot.ts

@@ -50,12 +50,41 @@ export function serverSnapshot(options: ResolvedFreshViteConfig): Plugin[] {
   const islands = new Map<string, { name: string; chunk: string | null }>();
   const islandsByFile = new Set<string>();
   const islandSpecByName = new Map<string, string>();
+  const islandNameBySpec = new Map<string, string>();
   const routeNamer = new UniqueNamer();
   const routeFileToName = new Map<string, string>();
 
   // deno-lint-ignore no-explicit-any
   const routes: Map<string, FsRouteFileNoMod<any>> = 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();
+
+    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);
+      }
+
+      add(spec, name);
+      islandsByFile.add(spec);
+    }
+  }
+
   return [
     {
       name: "fresh:server-snapshot",
@@ -78,11 +107,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 +137,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 +179,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];
@@ -196,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;
               }

packages/plugin-vite/tests/dev_server_test.ts

@@ -110,6 +110,49 @@ integrationTest(
   },
 );
 
+// 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 island recovers and keeps siblings interactive",
+  async () => {
+    const fixture = path.join(FIXTURE_DIR, "delete_island");
+    await withDevServer(fixture, async (address, dir) => {
+      await withBrowser(async (page) => {
+        // Wire the surviving island into the client graph and hydrate it.
+        await page.goto(`${address}/`, { waitUntil: "networkidle2" });
+        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 server didn't wedge on the dead import.
+        await waitFor(async () => {
+          for (let i = 0; i < 5; i++) {
+            const res = await fetch(`${address}/`);
+            const html = await res.text();
+            expect(res.status).toEqual(200);
+            // 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");
+      });
+    });
+  },
+);
+
 integrationTest("vite dev - starts without routes/ dir", async () => {
   const fixture = path.join(FIXTURE_DIR, "no_routes");
   await withDevServer(fixture, async (address) => {

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 (
+    <button type="button" class="extra" onClick={() => s.value++}>
+      extra {s.value}
+    </button>
+  );
+}

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 (
+    <button type="button" class="keep" onClick={() => s.value++}>
+      keep {s.value}
+    </button>
+  );
+}

packages/plugin-vite/tests/fixtures/delete_island/main.ts

@@ -0,0 +1,3 @@
+import { App } from "@fresh/core";
+
+export const app = new App().fsRoutes();

packages/plugin-vite/tests/fixtures/delete_island/routes/index.tsx

@@ -0,0 +1,10 @@
+import Keep from "../islands/Keep.tsx";
+
+export default function Hello() {
+  return (
+    <div>
+      <h1>ok</h1>
+      <Keep />
+    </div>
+  );
+}