From 09878365131539c0ae8bd461a15ef87c36ca36f3 Mon Sep 17 00:00:00 2001 From: Lenny Chen Date: Tue, 21 Jul 2026 13:51:33 -0700 Subject: [PATCH] fix(preview-links): honor custom frontmatter id when deriving doc URLs The docs PR preview-link generator built a page's URL purely from its filename when no `slug` frontmatter was present, ignoring a custom `id`. Docusaurus derives a slug-less doc's URL from the directory path plus its `id` (which defaults to the filename but can be overridden), so any page setting a custom `id` without a `slug` got a wrong preview link. Example: docs/production-deployment/self-hosted-guide/temporal-nexus.mdx sets `id: nexus`, so it publishes at .../self-hosted-guide/nexus, but the generator linked to .../self-hosted-guide/temporal-nexus. extractFrontMatter now also reads `id`, and slug derivation substitutes it for the filename segment (falling back to today's filename/index behavior when no id is set). slug frontmatter still takes precedence. Also moves the BASE_SHA guard into main() so the module's pure helpers can be imported by tests without the process exiting. Adds a test suite covering the id/slug/filename cases. --- bin/generate-docs-preview-list.js | 30 +++--- tests/test-docs-preview-list.mjs | 153 ++++++++++++++++++++++++++++++ 2 files changed, 172 insertions(+), 11 deletions(-) create mode 100644 tests/test-docs-preview-list.mjs diff --git a/bin/generate-docs-preview-list.js b/bin/generate-docs-preview-list.js index 2a24d641db..43545fc9cc 100644 --- a/bin/generate-docs-preview-list.js +++ b/bin/generate-docs-preview-list.js @@ -31,11 +31,6 @@ function isExcludedDocPath(filePath) { return false; } -if (!BASE_SHA) { - console.error('BASE_SHA environment variable is required.'); - process.exit(1); -} - function getChangedDocFiles(baseBranch = 'origin/main') { // Find the common ancestor (merge base) const mergeBase = execSync(`git merge-base HEAD ${baseBranch}`, { encoding: 'utf8' }).trim(); @@ -63,7 +58,7 @@ function extractFrontMatter(filePath) { const block = match[1]; const result = {}; - ['slug', 'title', 'sidebar_label'].forEach((key) => { + ['slug', 'id', 'title', 'sidebar_label'].forEach((key) => { const pattern = new RegExp(`^${key}:\\s*(.+)$`, 'm'); const valueMatch = block.match(pattern); if (valueMatch) { @@ -74,25 +69,31 @@ function extractFrontMatter(filePath) { return result; } -function relativeSlugFromPath(filePath) { +function relativeSlugFromPath(filePath, id) { const relativePath = path.relative(DOCS_DIR, filePath); const withoutExtension = relativePath.replace(/\.[^.]+$/, ''); const parts = withoutExtension.split(path.sep); - if (parts[parts.length - 1] === 'index') { + // Mirror Docusaurus: when a doc has no `slug`, its URL is the directory path + // plus its `id`. The `id` defaults to the filename, but a custom `id` in the + // frontmatter overrides that last segment (e.g. temporal-nexus.mdx with + // `id: nexus` publishes at .../nexus, not .../temporal-nexus). + if (typeof id === 'string' && id.trim().length > 0) { + parts[parts.length - 1] = id.trim(); + } else if (parts[parts.length - 1] === 'index') { parts.pop(); } return parts.join('/'); } -function normalizeSlug(slug, filePath) { +function normalizeSlug(slug, filePath, id) { if (typeof slug === 'string' && slug.trim().length > 0) { const trimmed = slug.trim(); return trimmed.startsWith('/') ? trimmed.slice(1) : trimmed; } - return relativeSlugFromPath(filePath); + return relativeSlugFromPath(filePath, id); } function humanizeSegment(segment) { @@ -115,7 +116,7 @@ function buildUrlForSlug(slug) { function collectDocInfo(filePath) { const frontMatter = extractFrontMatter(filePath); - const slug = normalizeSlug(frontMatter.slug, filePath); + const slug = normalizeSlug(frontMatter.slug, filePath, frontMatter.id); const segments = slug.split('/').filter((segment) => segment.length > 0); const label = frontMatter.sidebar_label || @@ -190,6 +191,11 @@ function renderTree(tree) { } function main() { + if (!BASE_SHA) { + console.error('BASE_SHA environment variable is required.'); + process.exit(1); + } + const changedFiles = getChangedDocFiles('origin/main'); if (changedFiles.length === 0) { @@ -216,6 +222,8 @@ if (require.main === module) { isExcludedDocPath, getChangedDocFiles, extractFrontMatter, + relativeSlugFromPath, + normalizeSlug, collectDocInfo, insertIntoTree, renderTree, diff --git a/tests/test-docs-preview-list.mjs b/tests/test-docs-preview-list.mjs new file mode 100644 index 0000000000..603f294a78 --- /dev/null +++ b/tests/test-docs-preview-list.mjs @@ -0,0 +1,153 @@ +/** + * tests/test-docs-preview-list.mjs + * + * Test suite for the docs PR preview-link generator's URL derivation. + * Uses only Node.js built-ins (no test framework). + * + * Run: node tests/test-docs-preview-list.mjs + */ + +import { writeFileSync, mkdtempSync } from "fs"; +import { join, dirname } from "path"; +import { tmpdir } from "os"; +import { fileURLToPath } from "url"; +import { createRequire } from "module"; + +const require = createRequire(import.meta.url); +const { + extractFrontMatter, + relativeSlugFromPath, + normalizeSlug, +} = require("../bin/generate-docs-preview-list.js"); + +const __dirname = dirname(fileURLToPath(import.meta.url)); +const PROJECT_ROOT = join(__dirname, ".."); +// Must match the generator's DOCS_DIR (process.cwd()/docs); the CI/test runner +// invokes tests from the repo root, so PROJECT_ROOT is the working directory. +const DOCS_DIR = join(PROJECT_ROOT, "docs"); + +// --------------------------------------------------------------------------- +// Minimal test harness +// --------------------------------------------------------------------------- +let passed = 0; +let failed = 0; +const failures = []; + +function test(name, fn) { + try { + fn(); + console.log(` āœ… ${name}`); + passed++; + } catch (err) { + console.log(` āŒ ${name}`); + console.log(` ${err.message}`); + failures.push({ name, error: err.message }); + failed++; + } +} + +function assertEqual(actual, expected, message) { + if (actual !== expected) { + throw new Error( + message || `Expected:\n ${JSON.stringify(expected)}\nGot:\n ${JSON.stringify(actual)}` + ); + } +} + +function docPath(...segments) { + return join(DOCS_DIR, ...segments); +} + +// --------------------------------------------------------------------------- +// extractFrontMatter ā€” now reads `id` +// --------------------------------------------------------------------------- +console.log("\nšŸ“¦ extractFrontMatter"); + +test("extracts a custom id", () => { + const dir = mkdtempSync(join(tmpdir(), "preview-fm-")); + const file = join(dir, "temporal-nexus.mdx"); + writeFileSync(file, `---\nid: nexus\ntitle: Self-hosted Temporal Nexus\nsidebar_label: Temporal Nexus\n---\n\nBody\n`); + const fm = extractFrontMatter(file); + assertEqual(fm.id, "nexus"); + assertEqual(fm.sidebar_label, "Temporal Nexus"); +}); + +test("id is undefined when absent", () => { + const dir = mkdtempSync(join(tmpdir(), "preview-fm-")); + const file = join(dir, "page.mdx"); + writeFileSync(file, `---\ntitle: Page\n---\n\nBody\n`); + const fm = extractFrontMatter(file); + assertEqual(fm.id, undefined); +}); + +// --------------------------------------------------------------------------- +// relativeSlugFromPath ā€” honors a custom id +// --------------------------------------------------------------------------- +console.log("\nšŸ“¦ relativeSlugFromPath"); + +test("custom id overrides the filename segment (the reported bug)", () => { + const slug = relativeSlugFromPath( + docPath("production-deployment", "self-hosted-guide", "temporal-nexus.mdx"), + "nexus" + ); + assertEqual(slug, "production-deployment/self-hosted-guide/nexus"); +}); + +test("no id falls back to the filename", () => { + const slug = relativeSlugFromPath( + docPath("production-deployment", "self-hosted-guide", "temporal-nexus.mdx"), + undefined + ); + assertEqual(slug, "production-deployment/self-hosted-guide/temporal-nexus"); +}); + +test("index file with no id drops the index segment", () => { + const slug = relativeSlugFromPath(docPath("cloud", "connectivity", "index.mdx"), undefined); + assertEqual(slug, "cloud/connectivity"); +}); + +test("index file with a custom id uses the id, not the folder", () => { + const slug = relativeSlugFromPath(docPath("cloud", "connectivity", "index.mdx"), "overview"); + assertEqual(slug, "cloud/connectivity/overview"); +}); + +test("empty/whitespace id is treated as absent", () => { + const slug = relativeSlugFromPath(docPath("develop", "go", "set-up.mdx"), " "); + assertEqual(slug, "develop/go/set-up"); +}); + +// --------------------------------------------------------------------------- +// normalizeSlug ā€” slug wins over id; id used only as fallback +// --------------------------------------------------------------------------- +console.log("\nšŸ“¦ normalizeSlug"); + +test("explicit slug takes precedence over id", () => { + const slug = normalizeSlug("/cli/cloud", docPath("cli", "temporal-cloud.mdx"), "cloud"); + assertEqual(slug, "cli/cloud"); +}); + +test("falls back to id-aware path when slug is absent", () => { + const slug = normalizeSlug(undefined, docPath("develop", "go", "set-up.mdx"), "set-up-your-local-go"); + assertEqual(slug, "develop/go/set-up-your-local-go"); +}); + +test("falls back to filename when neither slug nor id present", () => { + const slug = normalizeSlug(undefined, docPath("develop", "go", "set-up.mdx"), undefined); + assertEqual(slug, "develop/go/set-up"); +}); + +// --------------------------------------------------------------------------- +// Summary +// --------------------------------------------------------------------------- +console.log(`\n${"ā”€".repeat(50)}`); +console.log(`Results: ${passed} passed, ${failed} failed`); + +if (failures.length > 0) { + console.log("\nFailed tests:"); + for (const f of failures) { + console.log(` āŒ ${f.name}`); + } + process.exit(1); +} else { + console.log("\nšŸŽ‰ All tests passed!"); +}