Remove watched routes by the file that added them

.changeset/remove-route-by-source.md

@@ -0,0 +1,5 @@
+---
+"filesystem-routing": patch
+---
+
+Remove a watched route by the file that added it. `removeRoute` looked the entry up with `toPath`, so saving or deleting a module that isn't a route removed a page with the same path (for example `about.ts` next to `about.tsx`), and the entry from a custom `toRoute` stayed in the manifest after its file was deleted. `BaseFileSystemRouter` configured with only `toRoute` threw `Not implemented` when a file was deleted. An edit that changes the path returned by `toRoute` now replaces the old entry instead of adding a second one.

src/router.ts

@@ -54,6 +54,12 @@ export class BaseFileSystemRouter extends EventTarget {
 
   config: FileSystemRouterConfig;
 
+  /**
+   * Route path each source file produced. A removed file can no longer be
+   * passed to `toRoute`, and `toPath` may give another file's route.
+   */
+  private routePaths = new Map<string, string>();
+
   constructor(config: FileSystemRouterConfig) {
     super();
     this.routes = [];
@@ -106,7 +112,9 @@ export class BaseFileSystemRouter extends EventTarget {
       try {
         const route = this.toRoute(src);
         if (route) {
+          if (this.routePaths.get(src) !== route.path) this.removeRoute(src);
           this._addRoute(route);
+          this.routePaths.set(src, route.path);
           this.reload(route.path, "add");
         }
       } catch (e) {
@@ -132,7 +140,9 @@ export class BaseFileSystemRouter extends EventTarget {
       try {
         const route = this.toRoute(src);
         if (route) {
+          if (this.routePaths.get(src) !== route.path) this.removeRoute(src_);
           const updated = this._addRoute(route);
+          this.routePaths.set(src, route.path);
           this.reload(route.path, updated ? "update" : "add");
         } else {
           this.removeRoute(src_);
@@ -145,18 +155,15 @@ export class BaseFileSystemRouter extends EventTarget {
 
   removeRoute(src: string) {
     src = normalizePath(src);
-    if (this.isRoute(src)) {
-      const path = this.toPath(src);
-      if (path === undefined) {
-        return;
-      }
+    const path = this.routePaths.get(src);
+    if (path === undefined) return;
+    this.routePaths.delete(src);
 
-      const idx = this.routes.findIndex(r => r.path === path);
-      if (idx === -1) return;
+    const idx = this.routes.findIndex(r => r.path === path);
+    if (idx === -1) return;
 
-      this.routes.splice(idx, 1);
-      this.reload(path, "remove");
-    }
+    this.routes.splice(idx, 1);
+    this.reload(path, "remove");
   }
 
   on(type: string, cb: (evt: RouterEvent) => void) {

test/convention.spec.ts

@@ -3,7 +3,7 @@ import os from "node:os";
 import path from "node:path";
 import { afterEach, describe, expect, it } from "vitest";
 
-import { PageFileSystemRouter, routePathFromFile } from "../src/index.ts";
+import { BaseFileSystemRouter, PageFileSystemRouter, routePathFromFile } from "../src/index.ts";
 
 describe("routePathFromFile", () => {
   it("maps index files to their directory path", () => {
@@ -53,6 +53,12 @@ function createRouteTree(files: Record<string, string>) {
   return directory;
 }
 
+// A module convention that takes the route path from a `// path:` comment.
+function routeFromComment(src: string) {
+  const match = /^\/\/ path: (\S+)/.exec(fs.readFileSync(src, "utf8"));
+  return match ? { path: match[1]!, page: true } : undefined;
+}
+
 afterEach(() => {
   for (const directory of temporaryDirectories.splice(0)) {
     fs.rmSync(directory, { recursive: true });
@@ -134,6 +140,64 @@ describe("PageFileSystemRouter", () => {
     expect(events).toEqual(["add:/contact", "remove:/contact"]);
   });
 
+  it("keeps a page when a non-route file with the same path changes or is removed", async () => {
+    const dir = createRouteTree({
+      "about.tsx": "export default () => <h1>About</h1>;",
+      "about.ts": "export const data = {};"
+    });
+    const router = new PageFileSystemRouter({ dir, extensions: ["ts", "tsx"] });
+    await router.getRoutes();
+
+    const helper = path.join(dir, "about.ts");
+    await router.updateRoute(helper);
+    fs.rmSync(helper);
+    router.removeRoute(helper);
+
+    expect(router.routes.map(route => route.path)).toEqual(["/about"]);
+  });
+
+  it("removes the entry a custom toRoute produced", async () => {
+    const dir = createRouteTree({ "about.tsx": "// path: /a" });
+    const router = new PageFileSystemRouter({
+      dir,
+      extensions: ["tsx"],
+      toRoute: routeFromComment
+    });
+    await router.getRoutes();
+
+    const events: string[] = [];
+    router.on("reload", evt => events.push(`${evt.detail.type}:${evt.detail.route}`));
+
+    const about = path.join(dir, "about.tsx");
+    fs.writeFileSync(about, "// path: /b");
+    await router.updateRoute(about);
+    expect(router.routes.map(route => route.path)).toEqual(["/b"]);
+
+    fs.writeFileSync(about, "export default () => null;");
+    await router.updateRoute(about);
+    expect(router.routes).toEqual([]);
+
+    fs.writeFileSync(about, "// path: /c");
+    await router.addRoute(about);
+    fs.writeFileSync(about, "// path: /d");
+    await router.addRoute(about);
+    expect(router.routes.map(route => route.path)).toEqual(["/d"]);
+
+    fs.rmSync(about);
+    router.removeRoute(about);
+    expect(router.routes).toEqual([]);
+
+    expect(events).toEqual([
+      "remove:/a",
+      "add:/b",
+      "remove:/b",
+      "add:/c",
+      "remove:/c",
+      "add:/d",
+      "remove:/d"
+    ]);
+  });
+
   it("omits component refs when components are off", async () => {
     const dir = createRouteTree({
       "index.tsx": `
@@ -311,3 +375,21 @@ describe("PageFileSystemRouter", () => {
     ]);
   });
 });
+
+describe("BaseFileSystemRouter", () => {
+  it("removes routes when only toRoute is configured", async () => {
+    const dir = createRouteTree({ "about.tsx": "// path: /a" });
+    const router = new BaseFileSystemRouter({
+      dir,
+      extensions: ["tsx"],
+      toRoute: routeFromComment
+    });
+    await router.getRoutes();
+
+    const about = path.join(dir, "about.tsx");
+    fs.rmSync(about);
+    router.removeRoute(about);
+
+    expect(router.routes).toEqual([]);
+  });
+});