diff --git a/.changeset/remove-route-by-source.md b/.changeset/remove-route-by-source.md new file mode 100644 index 0000000..cf74284 --- /dev/null +++ b/.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. diff --git a/src/router.ts b/src/router.ts index f416e0f..d58d868 100644 --- a/src/router.ts +++ b/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(); + 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) { diff --git a/test/convention.spec.ts b/test/convention.spec.ts index 4cdebba..7eadee4 100644 --- a/test/convention.spec.ts +++ b/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) { 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 () =>

About

;", + "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([]); + }); +});