find non-parameterized page loaders, too (#1626)

src/config.ts

@@ -7,14 +7,14 @@ import {cwd} from "node:process";
 import {pathToFileURL} from "node:url";
 import type MarkdownIt from "markdown-it";
 import wrapAnsi from "wrap-ansi";
-import {visitMarkdownFiles} from "./files.js";
+import {visitFiles} from "./files.js";
 import {formatIsoDate, formatLocaleDate} from "./format.js";
 import type {FrontMatter} from "./frontMatter.js";
 import {LoaderResolver} from "./loader.js";
 import {createMarkdownIt, parseMarkdownMetadata} from "./markdown.js";
 import {getPagePaths} from "./pager.js";
 import {isAssetPath, parseRelativeUrl, resolvePath} from "./path.js";
-import {isParameterizedPath} from "./route.js";
+import {isParameterized} from "./route.js";
 import {resolveTheme} from "./theme.js";
 import {bold, yellow} from "./tty.js";
 
@@ -183,8 +183,8 @@ let cachedPages: {key: string; pages: Page[]} | null = null;
 function readPages(root: string, md: MarkdownIt): Page[] {
   const files: {file: string; source: string}[] = [];
   const hash = createHash("sha256");
-  for (const file of visitMarkdownFiles(root)) {
-    if (isParameterizedPath(file) || file === "index.md" || file === "404.md") continue;
+  for (const file of visitFiles(root, (name) => !isParameterized(name))) {
+    if (extname(file) !== ".md" || file === "index.md" || file === "404.md") continue;
     const source = readFileSync(join(root, file), "utf8");
     files.push({file, source});
     hash.update(file).update(source);
@@ -281,7 +281,7 @@ export function normalizeConfig(spec: ConfigSpec = {}, defaultRoot?: string, wat
           yield path;
         }
       }
-      for (const path of getDefaultPaths(root)) {
+      for (const path of this.loaders.findPagePaths()) {
         yield* visit(path);
       }
       for (const path of getPagePaths(this)) {
@@ -310,12 +310,6 @@ export function normalizeConfig(spec: ConfigSpec = {}, defaultRoot?: string, wat
   return config;
 }
 
-function getDefaultPaths(root: string): string[] {
-  return Array.from(visitMarkdownFiles(root))
-    .filter((path) => !isParameterizedPath(path))
-    .map((path) => join("/", dirname(path), basename(path, ".md")));
-}
-
 function normalizeDynamicPaths(spec: unknown): Config["paths"] {
   if (typeof spec === "function") return spec as () => AsyncIterable<string>;
   const paths = Array.from((spec ?? []) as ArrayLike<string>, String);

src/files.ts

@@ -2,7 +2,7 @@ import type {Stats} from "node:fs";
 import {existsSync, readdirSync, statSync} from "node:fs";
 import {mkdir, stat} from "node:fs/promises";
 import op from "node:path";
-import {extname, join, normalize, relative, sep} from "node:path/posix";
+import {join, normalize, relative, sep} from "node:path/posix";
 import {cwd} from "node:process";
 import {fileURLToPath} from "node:url";
 import {isEnoent} from "./error.js";
@@ -40,16 +40,13 @@ export function getStylePath(entry: string): string {
   return fromOsPath(op.relative(cwd(), op.join(fileURLToPath(import.meta.url), "..", "style", entry)));
 }
 
-/** Yields every Markdown (.md) file within the given root, recursively. */
-export function* visitMarkdownFiles(root: string): Generator<string> {
-  for (const file of visitFiles(root)) {
-    if (extname(file) !== ".md") continue;
-    yield file;
-  }
-}
-
-/** Yields every file within the given root, recursively, ignoring .observablehq. */
-export function* visitFiles(root: string): Generator<string> {
+/**
+ * Yields every file within the given root, recursively, ignoring .observablehq.
+ * If a test function is specified, any directories or files whose names don’t
+ * pass the specified test will be skipped (in addition to .observablehq). This
+ * is typically used to skip parameterized paths.
+ */
+export function* visitFiles(root: string, test?: (name: string) => boolean): Generator<string> {
   const visited = new Set<number>();
   const queue: string[] = [(root = normalize(root))];
   for (const path of queue) {
@@ -59,6 +56,7 @@ export function* visitFiles(root: string): Generator<string> {
       visited.add(status.ino);
       for (const entry of readdirSync(path)) {
         if (entry === ".observablehq") continue; // ignore the .observablehq directory
+        if (test !== undefined && !test(entry)) continue;
         queue.push(join(path, entry));
       }
     } else {

src/loader.ts

@@ -8,7 +8,7 @@ import {spawn} from "cross-spawn";
 import JSZip from "jszip";
 import {extract} from "tar-stream";
 import {enoent} from "./error.js";
-import {maybeStat, prepareOutput} from "./files.js";
+import {maybeStat, prepareOutput, visitFiles} from "./files.js";
 import {FileWatchers} from "./fileWatchers.js";
 import {formatByteSize} from "./format.js";
 import type {FileInfo} from "./javascript/module.js";
@@ -17,7 +17,7 @@ import type {Logger, Writer} from "./logger.js";
 import type {MarkdownPage, ParseOptions} from "./markdown.js";
 import {parseMarkdown} from "./markdown.js";
 import type {Params} from "./route.js";
-import {route} from "./route.js";
+import {isParameterized, requote, route} from "./route.js";
 import {cyan, faint, green, red, yellow} from "./tty.js";
 
 const runningCommands = new Map<string, Promise<string>>();
@@ -102,6 +102,17 @@ export class LoaderResolver {
     return watch(join(this.root, loader.path), listener);
   }
 
+  /**
+   * Finds the paths of all non-parameterized pages within the source root.
+   */
+  *findPagePaths(): Generator<string> {
+    const ext = new RegExp(`\\.md(${["", ...this.interpreters.keys()].map(requote).join("|")})$`);
+    for (const path of visitFiles(this.root, (name) => !isParameterized(name))) {
+      if (!ext.test(path)) continue;
+      yield `/${path.slice(0, path.lastIndexOf(".md"))}`;
+    }
+  }
+
   /**
    * Finds the page loader for the specified target path, relative to the source
    * root, if the loader exists. If there is no such loader, returns undefined.

src/route.ts

@@ -6,8 +6,8 @@ export type Params = {[name: string]: string};
 
 export type RouteResult = {path: string; ext: string; params?: Params};
 
-export function isParameterizedPath(path: string): boolean {
-  return path.split("/").some((name) => /\[.+\]/.test(name));
+export function isParameterized(name: string): boolean {
+  return /\[([a-z_]\w*)\]/i.test(name);
 }
 
 /**
@@ -110,6 +110,6 @@ function compilePattern(file: string): RegExp {
   return new RegExp(pattern, "i");
 }
 
-function requote(text: string): string {
+export function requote(text: string): string {
   return text.replace(/[\\^$*+?|[\]().{}]/g, "\\$&");
 }

test/build-test.ts

@@ -4,7 +4,7 @@ import {mkdir, mkdtemp, open, readFile, rename, rm, unlink, writeFile} from "nod
 import os from "node:os";
 import {join, normalize, relative} from "node:path/posix";
 import {PassThrough} from "node:stream";
-import {difference} from "d3-array";
+import {ascending, difference} from "d3-array";
 import type {BuildManifest} from "../src/build.js";
 import {FileBuildEffects, build} from "../src/build.js";
 import {normalizeConfig, readConfig, setCurrentDate} from "../src/config.js";
@@ -124,12 +124,13 @@ describe("build", () => {
     const config = normalizeConfig({root: inputDir, output: outputDir}, inputDir);
     const effects = new LoggingBuildEffects(outputDir, cacheDir);
     await build({config}, effects);
+    effects.buildManifest!.pages.sort((a, b) => ascending(a.path, b.path));
     assert.deepEqual(effects.buildManifest, {
       pages: [
         {path: "/", title: "Hello, world!"},
-        {path: "/weather", title: "It's going to be !"},
         {path: "/cities/", title: "Cities"},
-        {path: "/cities/portland", title: "Portland"}
+        {path: "/cities/portland", title: "Portland"},
+        {path: "/weather", title: "It's going to be !"}
       ]
     });
 

test/files-test.ts

@@ -1,7 +1,9 @@
 import assert from "node:assert";
 import {stat} from "node:fs/promises";
 import os from "node:os";
-import {getClientPath, getStylePath, maybeStat, prepareOutput, visitFiles, visitMarkdownFiles} from "../src/files.js";
+import {extname} from "node:path/posix";
+import {getClientPath, getStylePath, maybeStat, prepareOutput, visitFiles} from "../src/files.js";
+import {isParameterized} from "../src/route.js";
 
 describe("getClientPath(entry)", () => {
   it("returns the relative path to the specified source", () => {
@@ -67,11 +69,14 @@ describe("visitFiles(root)", () => {
   });
 });
 
-describe("visitMarkdownFiles(root)", () => {
-  it("visits all Markdown files in a directory, return the relative path from the root", () => {
-    assert.deepStrictEqual(collect(visitMarkdownFiles("test/input/build/files")), [
-      "files.md",
-      "subsection/subfiles.md"
+describe("visitFiles(root, test)", () => {
+  it("skips directories and files that don’t pass the specified test", () => {
+    assert.deepStrictEqual(
+      collect(visitFiles("test/input/build/params", (name) => isParameterized(name) || extname(name) !== "")),
+      ["observablehq.config.js", "[dir]/index.md", "[dir]/loaded.md.js"]
+    );
+    assert.deepStrictEqual(collect(visitFiles("test/input/build/params", (name) => !isParameterized(name))), [
+      "observablehq.config.js"
     ]);
   });
 });

test/input/build/page-loaders/hello-js.md.js

@@ -0,0 +1 @@
+process.stdout.write("# Hello JavaScript\n");

test/input/build/page-loaders/hello-ts.md.ts

@@ -0,0 +1 @@
+process.stdout.write("# Hello TypeScript\n");

test/input/build/page-loaders/index.md.sh

@@ -0,0 +1 @@
+echo '# Hello shell'

test/loader-test.ts

@@ -2,6 +2,7 @@ import assert from "node:assert";
 import {mkdir, readFile, rm, stat, unlink, utimes, writeFile} from "node:fs/promises";
 import os from "node:os";
 import {join} from "node:path/posix";
+import {sort} from "d3-array";
 import {clearFileInfo} from "../src/javascript/module.js";
 import type {LoadEffects} from "../src/loader.js";
 import {LoaderResolver} from "../src/loader.js";
@@ -11,6 +12,21 @@ const noopEffects: LoadEffects = {
   output: {write() {}}
 };
 
+describe("LoaderResolver.findPagePaths()", () => {
+  it("finds static Markdown pages", () => {
+    const loaders = new LoaderResolver({root: "test/input/build/simple"});
+    assert.deepStrictEqual(sort(loaders.findPagePaths()), ["/simple"]);
+  });
+  it("finds non-parameterized Markdown page loaders", () => {
+    const loaders = new LoaderResolver({root: "test/input/build/page-loaders"});
+    assert.deepStrictEqual(sort(loaders.findPagePaths()), ["/hello-js", "/hello-ts", "/index"]);
+  });
+  it("ignores parameterized pages", () => {
+    const loaders = new LoaderResolver({root: "test/input/build/params"});
+    assert.deepStrictEqual(sort(loaders.findPagePaths()), []);
+  });
+});
+
 describe("LoaderResolver.find(path)", () => {
   const loaders = new LoaderResolver({root: "test"});
   it("a .js data loader is called with node", async () => {

test/output/build/page-loaders/hello-js.html

@@ -0,0 +1,30 @@
+<!DOCTYPE html>
+<meta charset="utf-8">
+<meta name="viewport" content="width=device-width, initial-scale=1, maximum-scale=1">
+<meta name="generator" content="Observable Framework v1.0.0-test">
+<title>Hello JavaScript</title>
+<link rel="preconnect" href="https://fonts.gstatic.com" crossorigin>
+<link rel="preload" as="style" href="https://fonts.googleapis.com/css2?family=Source+Serif+4:ital,opsz,wght@0,8..60,200..900;1,8..60,200..900&amp;display=swap" crossorigin>
+<link rel="preload" as="style" href="./_observablehq/theme-air,near-midnight.00000004.css">
+<link rel="stylesheet" type="text/css" href="https://fonts.googleapis.com/css2?family=Source+Serif+4:ital,opsz,wght@0,8..60,200..900;1,8..60,200..900&amp;display=swap" crossorigin>
+<link rel="stylesheet" type="text/css" href="./_observablehq/theme-air,near-midnight.00000004.css">
+<link rel="modulepreload" href="./_observablehq/client.00000001.js">
+<link rel="modulepreload" href="./_observablehq/runtime.00000002.js">
+<link rel="modulepreload" href="./_observablehq/stdlib.00000003.js">
+<script type="module">
+
+import "./_observablehq/client.00000001.js";
+
+</script>
+<aside id="observablehq-toc" data-selector="h1:not(:first-of-type)[id], h2:first-child[id], :not(h1) + h2[id]">
+<nav>
+</nav>
+</aside>
+<div id="observablehq-center">
+<main id="observablehq-main" class="observablehq">
+<h1 id="hello-java-script" tabindex="-1"><a class="observablehq-header-anchor" href="#hello-java-script">Hello JavaScript</a></h1>
+</main>
+<footer id="observablehq-footer">
+<div>Built with <a href="https://observablehq.com/" target="_blank" rel="noopener noreferrer">Observable</a> on <a title="2024-01-10T16:00:00">Jan 10, 2024</a>.</div>
+</footer>
+</div>

test/output/build/page-loaders/hello-ts.html

@@ -0,0 +1,30 @@
+<!DOCTYPE html>
+<meta charset="utf-8">
+<meta name="viewport" content="width=device-width, initial-scale=1, maximum-scale=1">
+<meta name="generator" content="Observable Framework v1.0.0-test">
+<title>Hello TypeScript</title>
+<link rel="preconnect" href="https://fonts.gstatic.com" crossorigin>
+<link rel="preload" as="style" href="https://fonts.googleapis.com/css2?family=Source+Serif+4:ital,opsz,wght@0,8..60,200..900;1,8..60,200..900&amp;display=swap" crossorigin>
+<link rel="preload" as="style" href="./_observablehq/theme-air,near-midnight.00000004.css">
+<link rel="stylesheet" type="text/css" href="https://fonts.googleapis.com/css2?family=Source+Serif+4:ital,opsz,wght@0,8..60,200..900;1,8..60,200..900&amp;display=swap" crossorigin>
+<link rel="stylesheet" type="text/css" href="./_observablehq/theme-air,near-midnight.00000004.css">
+<link rel="modulepreload" href="./_observablehq/client.00000001.js">
+<link rel="modulepreload" href="./_observablehq/runtime.00000002.js">
+<link rel="modulepreload" href="./_observablehq/stdlib.00000003.js">
+<script type="module">
+
+import "./_observablehq/client.00000001.js";
+
+</script>
+<aside id="observablehq-toc" data-selector="h1:not(:first-of-type)[id], h2:first-child[id], :not(h1) + h2[id]">
+<nav>
+</nav>
+</aside>
+<div id="observablehq-center">
+<main id="observablehq-main" class="observablehq">
+<h1 id="hello-type-script" tabindex="-1"><a class="observablehq-header-anchor" href="#hello-type-script">Hello TypeScript</a></h1>
+</main>
+<footer id="observablehq-footer">
+<div>Built with <a href="https://observablehq.com/" target="_blank" rel="noopener noreferrer">Observable</a> on <a title="2024-01-10T16:00:00">Jan 10, 2024</a>.</div>
+</footer>
+</div>

test/output/build/page-loaders/index.html

@@ -0,0 +1,30 @@
+<!DOCTYPE html>
+<meta charset="utf-8">
+<meta name="viewport" content="width=device-width, initial-scale=1, maximum-scale=1">
+<meta name="generator" content="Observable Framework v1.0.0-test">
+<title>Hello shell</title>
+<link rel="preconnect" href="https://fonts.gstatic.com" crossorigin>
+<link rel="preload" as="style" href="https://fonts.googleapis.com/css2?family=Source+Serif+4:ital,opsz,wght@0,8..60,200..900;1,8..60,200..900&amp;display=swap" crossorigin>
+<link rel="preload" as="style" href="./_observablehq/theme-air,near-midnight.00000004.css">
+<link rel="stylesheet" type="text/css" href="https://fonts.googleapis.com/css2?family=Source+Serif+4:ital,opsz,wght@0,8..60,200..900;1,8..60,200..900&amp;display=swap" crossorigin>
+<link rel="stylesheet" type="text/css" href="./_observablehq/theme-air,near-midnight.00000004.css">
+<link rel="modulepreload" href="./_observablehq/client.00000001.js">
+<link rel="modulepreload" href="./_observablehq/runtime.00000002.js">
+<link rel="modulepreload" href="./_observablehq/stdlib.00000003.js">
+<script type="module">
+
+import "./_observablehq/client.00000001.js";
+
+</script>
+<aside id="observablehq-toc" data-selector="h1:not(:first-of-type)[id], h2:first-child[id], :not(h1) + h2[id]">
+<nav>
+</nav>
+</aside>
+<div id="observablehq-center">
+<main id="observablehq-main" class="observablehq">
+<h1 id="hello-shell" tabindex="-1"><a class="observablehq-header-anchor" href="#hello-shell">Hello shell</a></h1>
+</main>
+<footer id="observablehq-footer">
+<div>Built with <a href="https://observablehq.com/" target="_blank" rel="noopener noreferrer">Observable</a> on <a title="2024-01-10T16:00:00">Jan 10, 2024</a>.</div>
+</footer>
+</div>

test/route-test.ts

@@ -1,31 +1,26 @@
 import assert from "node:assert";
-import {isParameterizedPath, route} from "../src/route.js";
+import {isParameterized, route} from "../src/route.js";
 
-describe("isParameterizedPath(path)", () => {
+describe("isParameterizedName(path)", () => {
   it("returns true for a parameterized file name", () => {
-    assert.strictEqual(isParameterizedPath("/[file].md"), true);
-    assert.strictEqual(isParameterizedPath("/prefix-[file].md"), true);
-    assert.strictEqual(isParameterizedPath("/[file]-suffix.md"), true);
-    assert.strictEqual(isParameterizedPath("/[file]-[number].md"), true);
-    assert.strictEqual(isParameterizedPath("/path/[file].md"), true);
-    assert.strictEqual(isParameterizedPath("/path/to/[file].md"), true);
-    assert.strictEqual(isParameterizedPath("/path/[dir]/[file].md"), true);
+    assert.strictEqual(isParameterized("[file].md"), true);
+    assert.strictEqual(isParameterized("prefix-[file].md"), true);
+    assert.strictEqual(isParameterized("[file]-suffix.md"), true);
+    assert.strictEqual(isParameterized("[file]-[number].md"), true);
   });
   it("returns true for a parameterized directory name", () => {
-    assert.strictEqual(isParameterizedPath("/[dir]/file.md"), true);
-    assert.strictEqual(isParameterizedPath("/prefix-[dir]/file.md"), true);
-    assert.strictEqual(isParameterizedPath("/[dir]-suffix/file.md"), true);
-    assert.strictEqual(isParameterizedPath("/[dir]-[number]/file.md"), true);
-    assert.strictEqual(isParameterizedPath("/path/[dir]/file.md"), true);
-    assert.strictEqual(isParameterizedPath("/[dir1]/[dir2]/file.md"), true);
+    assert.strictEqual(isParameterized("[dir]"), true);
+    assert.strictEqual(isParameterized("prefix-[dir]"), true);
+    assert.strictEqual(isParameterized("[dir]-suffix"), true);
+    assert.strictEqual(isParameterized("[dir]-[number]"), true);
   });
   it("doesn’t consider an empty parameter to be valid", () => {
-    assert.strictEqual(isParameterizedPath("/[]/file.md"), false);
-    assert.strictEqual(isParameterizedPath("/path/to/[].md"), false);
+    assert.strictEqual(isParameterized("[]"), false);
+    assert.strictEqual(isParameterized("[].md"), false);
   });
   it("returns false for a non-parameterized path", () => {
-    assert.strictEqual(isParameterizedPath("/file.md"), false);
-    assert.strictEqual(isParameterizedPath("/path/to/file.md"), false);
+    assert.strictEqual(isParameterized("file.md"), false);
+    assert.strictEqual(isParameterized("dir"), false);
   });
 });