From b25c0458853fed0e8b420a883bd79659b97b3b31 Mon Sep 17 00:00:00 2001 From: Paul Gottschling Date: Tue, 8 Oct 2024 09:22:53 -0400 Subject: [PATCH] Catch Docusaurus build issues Break the docs build on states that are acceptable for the current NextJS-based docs engine but cause Docusaurus builds to fail: - **Unresolved partial parameters:** Edit the `remark-includes` linter to ensure that all parameters declared within a partial (using `{{ param }}` syntax) are either (a) assigned by the user or (b) given a default value. Otherwise, when we move to the new docs engine, builds will fail on unresolved parameters. - **Add more config checks:** Throw an exception for duplicate redirects and redirects where the `source` points to an existing file. With the current logic, this check takes place with every page build. While this is not ideal, and leads to noisy error output, we should be migrating soon and will not need to deal with this for long. --- server/config-docs.ts | 59 +++++++++++- .../includes-vars-erroneous-include.mdx | 5 + server/remark-includes.ts | 15 +++ uvu-tests/config-docs.test.ts | 93 ++++++++++++++++++- uvu-tests/remark-includes.test.ts | 21 +++++ 5 files changed, 191 insertions(+), 2 deletions(-) create mode 100644 server/fixtures/includes-vars-erroneous-include.mdx diff --git a/server/config-docs.ts b/server/config-docs.ts index 83aea430bd..5c917c7422 100644 --- a/server/config-docs.ts +++ b/server/config-docs.ts @@ -211,8 +211,41 @@ export const checkURLsForCorrespondingFiles = ( }, []); }; +// checkForRedirectsFromExistingFiles returns an array of redirects in which the +// source corresponds to a file at a path rooted at dirRoot. +export const checkForRedirectsFromExistingFiles = ( + dirRoot: string, + redirects: Redirect[] +): Redirect[] => { + let result: Redirect[] = []; + + redirects.forEach((r) => { + if (correspondingFileExistsForURL(dirRoot, r.source)) { + result.push(r); + } + }); + return result; +}; + +// checkDuplicateRedirects checks the provided redirects for duplicates and +// returns an array of Redirect objects. Duplicate checks are based on the +// source of each redirect. +export const checkDuplicateRedirects = (redirects: Redirect[]): Redirect[] => { + const result: Redirect[] = []; + const uniques = new Set(); + redirects.forEach((r) => { + if (uniques.has(r.source)) { + result.push(r); + return; + } + uniques.add(r.source); + }); + return result; +}; + // checkURLForCorrespondingFile determines whether a file exists in the content -// directory rooted at dirRoot for the file corresponding to the provided URL path.// If a file does not exist, it returns false. +// directory rooted at dirRoot for the file corresponding to the provided URL path. +// If a file does not exist, it returns false. const correspondingFileExistsForURL = ( dirRoot: string, urlpath: string @@ -326,6 +359,30 @@ export const loadConfig = (version: string) => { ); } + const redirsFrom = checkForRedirectsFromExistingFiles( + join("content", version, "docs", "pages"), + config.redirects + ); + + if (redirsFrom.length > 0) { + throw new Error( + "Error parsing docs config file " + + join("content", version, "docs", "config.json") + + ': Each of the following redirects includes a "source" that corresponds to an existing file: ' + + JSON.stringify(redirsFrom, null, 2) + ); + } + + const duplicateRedirects = checkDuplicateRedirects(config.redirects); + if (duplicateRedirects.length > 0) { + throw new Error( + "Error parsing docs config file " + + join("content", version, "docs", "config.json") + + ": Found redirects with duplicate sources: " + + JSON.stringify(duplicateRedirects, null, 2) + ); + } + validateConfig(validator, config); config.navigation.forEach((item, i) => { diff --git a/server/fixtures/includes-vars-erroneous-include.mdx b/server/fixtures/includes-vars-erroneous-include.mdx new file mode 100644 index 0000000000..9c9807da53 --- /dev/null +++ b/server/fixtures/includes-vars-erroneous-include.mdx @@ -0,0 +1,5 @@ +## Installation + +Here is a test for including variables in an MDX file. + +(!install-version.mdx version="10" unsupport="9" !) diff --git a/server/remark-includes.ts b/server/remark-includes.ts index 35321a8605..0259bcc487 100644 --- a/server/remark-includes.ts +++ b/server/remark-includes.ts @@ -244,6 +244,21 @@ const resolveIncludes = ({ content = content.replace(varRegexp, finalVal); } + // Catch unresolved parameters, which can break docs builds + const paramRE = new RegExp(`{{ ?\\w+ ?}}`, "g"); + const unresolvedParams = Array.from(content.matchAll(paramRE)); + if (unresolvedParams.length > 0) { + const errs = unresolvedParams + .map((el) => { + return el[0]; + }) + .join(","); + + error = + `${includePath}: the following partial parameters were not assigned and have no default value: ` + + errs; + } + return content; } else { error = `Wrong import path ${includePath} in file ${filePath}.`; diff --git a/uvu-tests/config-docs.test.ts b/uvu-tests/config-docs.test.ts index 54524d8b5b..e913f06f6e 100644 --- a/uvu-tests/config-docs.test.ts +++ b/uvu-tests/config-docs.test.ts @@ -1,11 +1,17 @@ import { Redirect } from "next/dist/lib/load-custom-routes"; import { suite } from "uvu"; import * as assert from "uvu/assert"; -import { Config, checkURLsForCorrespondingFiles } from "../server/config-docs"; +import { + Config, + checkURLsForCorrespondingFiles, + checkForRedirectsFromExistingFiles, + checkDuplicateRedirects, +} from "../server/config-docs"; import { generateNavPaths } from "../server/pages-helpers"; import { randomUUID } from "crypto"; import { join } from "path"; import { Volume, createFsFromVolume } from "memfs"; +import type { Redirect } from "next/dist/lib/load-custom-routes"; const Suite = suite("server/config-docs"); @@ -459,4 +465,89 @@ title: Deploying the Database Service on Kubernetes ); }); +Suite("Checks for duplicate redirects", () => { + const redirects: Array = [ + { + source: "/getting-started/", + destination: "/get-started/", + permanent: true, + }, + { + source: "/getting-started/", + destination: "/get-started/", + permanent: true, + }, + { + source: "/application-access/", + destination: "/connecting-apps/", + permanent: true, + }, + { + source: "/application-access/", + destination: "/connecting-apps/", + permanent: true, + }, + { + source: "/database-access/", + destination: "/connecting-databases/", + permanent: true, + }, + ]; + + const expected: Array = [ + { + source: "/getting-started/", + destination: "/get-started/", + permanent: true, + }, + { + source: "/application-access/", + destination: "/connecting-apps/", + permanent: true, + }, + ]; + + const actual = checkDuplicateRedirects(redirects); + assert.equal(actual, expected); +}); + +Suite("Checks for redirects from existing paths", () => { + const redirects: Array = [ + { + source: "/contact/offices/", + destination: "/get-in-touch/offices/", + permanent: true, + }, + { + source: "/locations/", + destination: "/contact/offices/", + permanent: true, + }, + { + source: "/about/projects/project1/", + destination: "/project1/", + permanent: true, + }, + ]; + + const expected: Array = [ + { + source: "/contact/offices/", + destination: "/get-in-touch/offices/", + permanent: true, + }, + { + source: "/about/projects/project1/", + destination: "/project1/", + permanent: true, + }, + ]; + + const actual = checkForRedirectsFromExistingFiles( + join("server", "fixtures", "fake-content"), + redirects + ); + assert.equal(actual, expected); +}); + Suite.run(); diff --git a/uvu-tests/remark-includes.test.ts b/uvu-tests/remark-includes.test.ts index ea3f651df5..2970b7d017 100644 --- a/uvu-tests/remark-includes.test.ts +++ b/uvu-tests/remark-includes.test.ts @@ -483,6 +483,27 @@ Suite("Resolves template variables in includes", () => { assert.equal(result, expected); }); +Suite("Throws an error if a variable is unresolved and has no default", () => { + const value = readFileSync( + resolve("server/fixtures/includes-vars-erroneous-include.mdx"), + "utf-8" + ); + + const out = transformer( + { + value, + path: "/content/4.0/docs/pages/filename.mdx", + }, + { lint: true } + ); + + assert.equal(out.messages.length, 1); + assert.equal( + out.messages[0].reason, + "The following partial parameters were not assigned and have no default value: {{ unsupported }}" + ); +}); + Suite( "Resolves relative links in partials based on the path of the partial", () => {