From afa63033cce295f496362e468afa212d378340b2 Mon Sep 17 00:00:00 2001 From: saberzero1 Date: Fri, 10 Apr 2026 21:14:23 +0200 Subject: [PATCH] fix: make fileTrie last-write-wins; warn on slug collisions --- quartz/build.ts | 19 ++-- quartz/util/fileTrie.test.ts | 58 ++++++++++++ quartz/util/fileTrie.ts | 7 +- quartz/util/slugCollisions.test.ts | 145 +++++++++++++++++++++++++++++ quartz/util/slugCollisions.ts | 84 +++++++++++++++++ 5 files changed, 306 insertions(+), 7 deletions(-) create mode 100644 quartz/util/slugCollisions.test.ts create mode 100644 quartz/util/slugCollisions.ts diff --git a/quartz/build.ts b/quartz/build.ts index bc0340e..b585ff2 100644 --- a/quartz/build.ts +++ b/quartz/build.ts @@ -10,6 +10,7 @@ import { filterContent } from "./processors/filter" import { emitContent } from "./processors/emit" import cfg from "../quartz" import { FilePath, FullSlug, joinSegments, slugifyFilePath } from "./util/path" +import { detectSlugCollisions, formatCollisionWarning } from "./util/slugCollisions" import chokidar from "chokidar" import { ProcessedContent } from "./plugins/vfile" import { Argv, BuildCtx } from "./util/ctx" @@ -22,6 +23,12 @@ import { randomIdNonSecure } from "./util/random" import { ChangeEvent, QuartzPageTypePluginInstance } from "./plugins/types" import { minimatch } from "minimatch" +function reportSlugCollisions(content: ProcessedContent[]): void { + const collisions = detectSlugCollisions(content) + if (collisions.length === 0) return + console.warn(styleText("yellow", formatCollisionWarning(collisions))) +} + function getPageTypeExtensions(ctx: BuildCtx): Set { const extensions = new Set() const pageTypes = (ctx.cfg.plugins.pageTypes ?? []) as unknown as QuartzPageTypePluginInstance[] @@ -123,6 +130,7 @@ async function buildQuartz(argv: Argv, mut: Mutex, clientRefresh: () => void) { } const parsedFiles = await parseMarkdown(ctx, filePaths) + reportSlugCollisions(parsedFiles) const filteredContent = filterContent(ctx, parsedFiles) await emitContent(ctx, filteredContent) @@ -303,12 +311,11 @@ async function rebuild(changes: ChangeEvent[], clientRefresh: () => void, buildD const aliases = addVirtualPageSlugAliases(ctx.allSlugs, ptExtensions) ctx.allSlugs.push(...aliases) } - let processedFiles = filterContent( - ctx, - Array.from(contentMap.values()) - .filter((file) => file.type === "markdown") - .map((file) => file.content), - ) + const markdownContent = Array.from(contentMap.values()) + .filter((file) => file.type === "markdown") + .map((file) => file.content) + reportSlugCollisions(markdownContent) + let processedFiles = filterContent(ctx, markdownContent) let emittedFiles = 0 diff --git a/quartz/util/fileTrie.test.ts b/quartz/util/fileTrie.test.ts index c747f34..adb93bf 100644 --- a/quartz/util/fileTrie.test.ts +++ b/quartz/util/fileTrie.test.ts @@ -107,6 +107,64 @@ describe("FileTrie", () => { assert.strictEqual(trie.children[1].children[0].children[0].data, data2) assert.strictEqual(trie.children[1].children[0].children[0].children.length, 0) }) + + test("last-insert-wins on folder-note collision (matches emitter semantics)", () => { + const first = { + title: "First Folder Note", + slug: "foo/index", + filePath: "foo/foo.md", + } + const second = { + title: "Second Folder Note", + slug: "foo/index", + filePath: "foo/index.md", + } + + trie.add(first) + trie.add(second) + + assert.strictEqual(trie.children.length, 1) + assert.strictEqual(trie.children[0].slug, "foo/index") + assert.strictEqual(trie.children[0].data, second) + }) + + test("last-insert-wins on root-level index collision", () => { + const first = { title: "First", slug: "index", filePath: "a.md" } + const second = { title: "Second", slug: "index", filePath: "b.md" } + + trie.add(first) + trie.add(second) + + assert.strictEqual(trie.data, second) + }) + + test("collision does not affect sibling files in the same folder", () => { + const folderNoteA = { + title: "Folder Note A", + slug: "foo/index", + filePath: "foo/foo.md", + } + const folderNoteB = { + title: "Folder Note B", + slug: "foo/index", + filePath: "foo/index.md", + } + const sibling = { + title: "Sibling", + slug: "foo/alice", + filePath: "foo/alice.md", + } + + trie.add(folderNoteA) + trie.add(sibling) + trie.add(folderNoteB) + + assert.strictEqual(trie.children.length, 1) + assert.strictEqual(trie.children[0].slug, "foo/index") + assert.strictEqual(trie.children[0].data, folderNoteB) + assert.strictEqual(trie.children[0].children.length, 1) + assert.strictEqual(trie.children[0].children[0].data, sibling) + }) }) describe("filter", () => { diff --git a/quartz/util/fileTrie.ts b/quartz/util/fileTrie.ts index f4a4e78..31fa499 100644 --- a/quartz/util/fileTrie.ts +++ b/quartz/util/fileTrie.ts @@ -69,7 +69,12 @@ export class FileTrieNode { if (path.length === 1) { // base case, we are at the end of the path if (segment === "index") { - this.data ??= file + // Last-insert-wins on collision. Matches the emitter's last-write-wins + // semantics at plugins/emitters/helpers.ts so the trie's data and the + // file on disk agree on which source file "owns" a colliding slug. + // Collision detection happens upstream in build.ts; this assignment is + // the fallback for any duplicates that still reach the trie. + this.data = file } else { this.makeChild(path, file) } diff --git a/quartz/util/slugCollisions.test.ts b/quartz/util/slugCollisions.test.ts new file mode 100644 index 0000000..4d06693 --- /dev/null +++ b/quartz/util/slugCollisions.test.ts @@ -0,0 +1,145 @@ +import test, { describe } from "node:test" +import assert from "node:assert" +import { detectSlugCollisions, formatCollisionWarning } from "./slugCollisions" +import { ProcessedContent } from "../plugins/vfile" +import { FilePath, FullSlug } from "./path" + +function makeContent( + entries: Array<{ slug: string; relativePath?: string; filePath?: string }>, +): ProcessedContent[] { + return entries.map((e) => { + const vfile = { + data: { + slug: e.slug as FullSlug, + relativePath: (e.relativePath ?? `${e.slug}.md`) as FilePath, + filePath: (e.filePath ?? `/vault/${e.relativePath ?? `${e.slug}.md`}`) as FilePath, + }, + } + return [{ type: "root", children: [] }, vfile] as unknown as ProcessedContent + }) +} + +describe("detectSlugCollisions", () => { + test("returns empty array when there are no collisions", () => { + const content = makeContent([{ slug: "alice" }, { slug: "bob" }, { slug: "characters/index" }]) + assert.deepStrictEqual(detectSlugCollisions(content), []) + }) + + test("returns empty array for empty input", () => { + assert.deepStrictEqual(detectSlugCollisions([]), []) + }) + + test("detects a two-file collision with winner = last file", () => { + const content = makeContent([ + { slug: "foo/index", relativePath: "foo/foo.md" }, + { slug: "foo/index", relativePath: "foo/index.md" }, + ]) + const collisions = detectSlugCollisions(content) + assert.strictEqual(collisions.length, 1) + assert.strictEqual(collisions[0]!.slug, "foo/index") + assert.strictEqual(collisions[0]!.files.length, 2) + assert.strictEqual(collisions[0]!.winner.relativePath, "foo/index.md") + assert.strictEqual(collisions[0]!.files[0]!.relativePath, "foo/foo.md") + assert.strictEqual(collisions[0]!.files[1]!.relativePath, "foo/index.md") + }) + + test("detects a three-file collision with all files listed, winner = last", () => { + const content = makeContent([ + { slug: "bar/index", relativePath: "bar/_index.md" }, + { slug: "bar/index", relativePath: "bar/bar.md" }, + { slug: "bar/index", relativePath: "bar/index.md" }, + ]) + const collisions = detectSlugCollisions(content) + assert.strictEqual(collisions.length, 1) + assert.strictEqual(collisions[0]!.files.length, 3) + assert.strictEqual(collisions[0]!.winner.relativePath, "bar/index.md") + }) + + test("detects multiple separate collisions", () => { + const content = makeContent([ + { slug: "a/index", relativePath: "a/a.md" }, + { slug: "a/index", relativePath: "a/index.md" }, + { slug: "b/index", relativePath: "b/b.md" }, + { slug: "b/index", relativePath: "b/index.md" }, + { slug: "unique" }, + ]) + const collisions = detectSlugCollisions(content) + assert.strictEqual(collisions.length, 2) + const slugs = collisions.map((c) => c.slug).sort() + assert.deepStrictEqual(slugs, ["a/index", "b/index"]) + }) + + test("ignores entries without a slug", () => { + const content: ProcessedContent[] = [ + ...makeContent([{ slug: "alice" }]), + [ + { type: "root", children: [] }, + { data: { slug: undefined, relativePath: "broken.md" } }, + ] as unknown as ProcessedContent, + ] + assert.deepStrictEqual(detectSlugCollisions(content), []) + }) + + test("winner annotation matches fileTrie last-insert-wins semantics", () => { + // Glob order is alphabetical: foo/foo.md sorts before foo/index.md. + // Both the fileTrie and this detector must agree that the second file wins. + const content = makeContent([ + { slug: "foo/index", relativePath: "foo/foo.md" }, + { slug: "foo/index", relativePath: "foo/index.md" }, + ]) + const collisions = detectSlugCollisions(content) + assert.strictEqual(collisions[0]!.winner.relativePath, "foo/index.md") + }) +}) + +describe("formatCollisionWarning", () => { + test("returns empty string for empty input", () => { + assert.strictEqual(formatCollisionWarning([]), "") + }) + + test("formats single collision with winner and shadowed markers", () => { + const content = makeContent([ + { slug: "foo/index", relativePath: "foo/foo.md" }, + { slug: "foo/index", relativePath: "foo/index.md" }, + ]) + const collisions = detectSlugCollisions(content) + const output = formatCollisionWarning(collisions) + assert.match(output, /1 slug collision detected/) + assert.match(output, /foo\/index/) + assert.match(output, /foo\/foo\.md .*\(shadowed\)/) + assert.match(output, /foo\/index\.md .*\(used for this URL\)/) + }) + + test("formats multiple collisions with count in header", () => { + const content = makeContent([ + { slug: "a/index", relativePath: "a/a.md" }, + { slug: "a/index", relativePath: "a/index.md" }, + { slug: "b/index", relativePath: "b/b.md" }, + { slug: "b/index", relativePath: "b/index.md" }, + ]) + const collisions = detectSlugCollisions(content) + const output = formatCollisionWarning(collisions) + assert.match(output, /2 slug collisions detected/) + assert.match(output, /a\/index/) + assert.match(output, /b\/index/) + }) + + test("output mentions Folder Notes convention as a common cause", () => { + const content = makeContent([ + { slug: "foo/index", relativePath: "foo/foo.md" }, + { slug: "foo/index", relativePath: "foo/index.md" }, + ]) + const output = formatCollisionWarning(detectSlugCollisions(content)) + assert.match(output, /Folder Notes/) + }) + + test("falls back to filePath when relativePath is missing", () => { + const content = makeContent([ + { slug: "x/index", relativePath: "", filePath: "/vault/x/x.md" }, + { slug: "x/index", relativePath: "", filePath: "/vault/x/index.md" }, + ]) + const output = formatCollisionWarning(detectSlugCollisions(content)) + assert.match(output, /\/vault\/x\/x\.md/) + assert.match(output, /\/vault\/x\/index\.md/) + }) +}) diff --git a/quartz/util/slugCollisions.ts b/quartz/util/slugCollisions.ts new file mode 100644 index 0000000..af99543 --- /dev/null +++ b/quartz/util/slugCollisions.ts @@ -0,0 +1,84 @@ +import { ProcessedContent } from "../plugins/vfile" +import { FullSlug } from "./path" + +/** + * A slug collision: two or more source files that produce the same FullSlug + * after slugifyFilePath. The `winner` is the file whose HTML output and trie + * data represent this slug in the final build (see fileTrie.ts and the + * emitter's last-write-wins semantics in plugins/emitters/helpers.ts). + * + * `files` is in the order the files appear in the parsed content array + * (glob order), which is deterministic. `winner` is always the last entry. + */ +export interface SlugCollision { + slug: FullSlug + files: Array<{ relativePath: string; filePath: string }> + winner: { relativePath: string; filePath: string } +} + +export function detectSlugCollisions(content: ProcessedContent[]): SlugCollision[] { + const bySlug = new Map>() + + for (const [, file] of content) { + const slug = file.data.slug + if (!slug) continue + const entry = { + relativePath: (file.data.relativePath ?? "") as string, + filePath: (file.data.filePath ?? "") as string, + } + const existing = bySlug.get(slug) + if (existing) { + existing.push(entry) + } else { + bySlug.set(slug, [entry]) + } + } + + const collisions: SlugCollision[] = [] + for (const [slug, files] of bySlug) { + if (files.length < 2) continue + collisions.push({ + slug, + files, + winner: files[files.length - 1]!, + }) + } + + return collisions +} + +/** + * Format a list of collisions as a single human-readable warning block. + * Returns an empty string when there are no collisions so callers can + * unconditionally log the result. + */ +export function formatCollisionWarning(collisions: SlugCollision[]): string { + if (collisions.length === 0) return "" + + const lines: string[] = [] + const header = + collisions.length === 1 + ? `Warning: 1 slug collision detected.` + : `Warning: ${collisions.length} slug collisions detected.` + lines.push(header) + lines.push( + `Multiple source files produced the same URL slug. The last-processed file wins; the others are shadowed and their content will not appear in the output.`, + ) + lines.push("") + + for (const collision of collisions) { + lines.push(` slug \`${collision.slug}\``) + for (const file of collision.files) { + const marker = file === collision.winner ? "(used for this URL)" : "(shadowed)" + const path = file.relativePath || file.filePath || "(unknown source)" + lines.push(` - ${path} ${marker}`) + } + lines.push("") + } + + lines.push( + `To resolve, rename or delete all but one file per collided slug. This may include files using the Obsidian "Folder Notes" convention (\`folder/folder.md\`) that collide with an existing \`folder/index.md\`.`, + ) + + return lines.join("\n") +}