From 71b5054be6dcba2cb0428f737189972dfdc048be Mon Sep 17 00:00:00 2001 From: Joshua Chen Date: Fri, 17 Jun 2022 17:45:54 +0800 Subject: [PATCH] refactor: remove "error" reporting level, move reportMessage to logger --- packages/docusaurus-logger/README.md | 1 + .../src/__tests__/index.test.ts | 30 +++++++++++++ packages/docusaurus-logger/src/index.ts | 45 +++++++++++++++++++ .../src/index.ts | 7 ++- .../src/index.ts | 9 +--- packages/docusaurus-types/src/index.d.ts | 2 +- .../src/__tests__/jsUtils.test.ts | 38 ---------------- packages/docusaurus-utils/src/index.ts | 1 - packages/docusaurus-utils/src/jsUtils.ts | 43 ------------------ .../configValidation.test.ts.snap | 5 +++ .../server/__tests__/configValidation.test.ts | 14 ++++++ packages/docusaurus/src/server/brokenLinks.ts | 9 +--- .../docusaurus/src/server/configValidation.ts | 6 +-- packages/docusaurus/src/server/routes.ts | 16 +++---- website/docs/api/docusaurus.config.js.md | 6 +-- website/docs/api/misc/logger/logger.md | 1 + 16 files changed, 117 insertions(+), 116 deletions(-) diff --git a/packages/docusaurus-logger/README.md b/packages/docusaurus-logger/README.md index 9b4540ce4c50..d81ff3c0b9f1 100644 --- a/packages/docusaurus-logger/README.md +++ b/packages/docusaurus-logger/README.md @@ -25,6 +25,7 @@ It exports a single object as default export: `logger`. `logger` has the followi - `warn`: prints a warning that should be payed attention to. - `error`: prints an error (not necessarily halting the program) that signals significant problems. - `success`: prints a success message. +- The `report` function. It takes a `ReportingSeverity` value (`ignore`, `log`, `warn`, `throw`) and reports a message according to the severity. ### A word on the `error` formatter diff --git a/packages/docusaurus-logger/src/__tests__/index.test.ts b/packages/docusaurus-logger/src/__tests__/index.test.ts index 59a8f9c0d1d9..cea662c1ca6f 100644 --- a/packages/docusaurus-logger/src/__tests__/index.test.ts +++ b/packages/docusaurus-logger/src/__tests__/index.test.ts @@ -124,3 +124,33 @@ describe('success', () => { expect(consoleMock.mock.calls).toMatchSnapshot(); }); }); + +describe('report', () => { + beforeAll(() => jest.clearAllMocks()); + it('works with all severities', () => { + const consoleLog = jest.spyOn(console, 'info').mockImplementation(() => {}); + const consoleWarn = jest + .spyOn(console, 'warn') + .mockImplementation(() => {}); + logger.report('ignore')('hey'); + logger.report('log')('hey'); + logger.report('warn')('hey'); + expect(() => + logger.report('throw')('hey'), + ).toThrowErrorMatchingInlineSnapshot(`"hey"`); + expect(() => + // @ts-expect-error: for test + logger.report('foo')('hey'), + ).toThrowErrorMatchingInlineSnapshot( + `"Unexpected "reportingSeverity" value: foo."`, + ); + expect(consoleLog).toBeCalledTimes(1); + expect(consoleLog).toBeCalledWith( + expect.stringMatching(/.*\[INFO\].* hey/), + ); + expect(consoleWarn).toBeCalledTimes(1); + expect(consoleWarn).toBeCalledWith( + expect.stringMatching(/.*\[WARNING\].* hey/), + ); + }); +}); diff --git a/packages/docusaurus-logger/src/index.ts b/packages/docusaurus-logger/src/index.ts index f430555b0e26..3de5f81c0346 100644 --- a/packages/docusaurus-logger/src/index.ts +++ b/packages/docusaurus-logger/src/index.ts @@ -6,6 +6,7 @@ */ import chalk from 'chalk'; +import type {ReportingSeverity} from '@docusaurus/types'; type InterpolatableValue = string | number | (string | number)[]; @@ -122,11 +123,54 @@ function success(msg: unknown, ...values: InterpolatableValue[]): void { }`, ); } +function throwError(msg: unknown): void; +function throwError( + msg: TemplateStringsArray, + ...values: [InterpolatableValue, ...InterpolatableValue[]] +): void; +function throwError(msg: unknown, ...values: InterpolatableValue[]): void { + throw new Error( + values.length === 0 + ? stringify(msg) + : interpolate(msg as TemplateStringsArray, ...values), + ); +} function newLine(): void { console.log(); } +/** + * Takes a message and reports it according to the severity that the user wants. + * + * - `ignore`: completely no-op + * - `log`: uses the `INFO` log level + * - `warn`: uses the `WARN` log level + * - `throw`: aborts the process, throws the error. + * + * Since the logger doesn't have logging level filters yet, these severities + * mostly just differ by their colors. + * + * @throws In addition to throwing when `reportingSeverity === "throw"`, this + * function also throws if `reportingSeverity` is not one of the above. + */ +function report(reportingSeverity: ReportingSeverity): typeof success { + const reportingMethods = { + ignore: () => {}, + log: info, + warn, + throw: throwError, + }; + if ( + !Object.prototype.hasOwnProperty.call(reportingMethods, reportingSeverity) + ) { + throw new Error( + `Unexpected "reportingSeverity" value: ${reportingSeverity}.`, + ); + } + return reportingMethods[reportingSeverity]; +} + const logger = { red: (msg: string | number): string => chalk.red(msg), yellow: (msg: string | number): string => chalk.yellow(msg), @@ -144,6 +188,7 @@ const logger = { warn, error, success, + report, newLine, }; diff --git a/packages/docusaurus-plugin-content-blog/src/index.ts b/packages/docusaurus-plugin-content-blog/src/index.ts index c4073e49274d..c96ea7a84f34 100644 --- a/packages/docusaurus-plugin-content-blog/src/index.ts +++ b/packages/docusaurus-plugin-content-blog/src/index.ts @@ -6,12 +6,12 @@ */ import path from 'path'; +import logger from '@docusaurus/logger'; import { normalizeUrl, docuHash, aliasedSitePath, getPluginI18nPath, - reportMessage, posixPath, addTrailingPathSeparator, createAbsoluteFilePathMatcher, @@ -391,10 +391,9 @@ export default async function pluginContentBlog( if (onBrokenMarkdownLinks === 'ignore') { return; } - reportMessage( - `Blog markdown link couldn't be resolved: (${brokenMarkdownLink.link}) in ${brokenMarkdownLink.filePath}`, + logger.report( onBrokenMarkdownLinks, - ); + )`Blog markdown link couldn't be resolved: (url=${brokenMarkdownLink.link}) in path=${brokenMarkdownLink.filePath}`; }, }; diff --git a/packages/docusaurus-plugin-content-docs/src/index.ts b/packages/docusaurus-plugin-content-docs/src/index.ts index c9ce6d8decd0..0b03d48a0542 100644 --- a/packages/docusaurus-plugin-content-docs/src/index.ts +++ b/packages/docusaurus-plugin-content-docs/src/index.ts @@ -13,7 +13,6 @@ import { docuHash, aliasedSitePath, getContentPathList, - reportMessage, posixPath, addTrailingPathSeparator, createAbsoluteFilePathMatcher, @@ -330,13 +329,9 @@ export default async function pluginContentDocs( sourceToPermalink: getSourceToPermalink(), versionsMetadata, onBrokenMarkdownLink: (brokenMarkdownLink) => { - if (siteConfig.onBrokenMarkdownLinks === 'ignore') { - return; - } - reportMessage( - `Docs markdown link couldn't be resolved: (${brokenMarkdownLink.link}) in ${brokenMarkdownLink.filePath} for version ${brokenMarkdownLink.contentPaths.versionName}`, + logger.report( siteConfig.onBrokenMarkdownLinks, - ); + )`Docs markdown link couldn't be resolved: (url=${brokenMarkdownLink.link}) in path=${brokenMarkdownLink.filePath} for version number=${brokenMarkdownLink.contentPaths.versionName}`; }, }; diff --git a/packages/docusaurus-types/src/index.d.ts b/packages/docusaurus-types/src/index.d.ts index ced9b8bf6fe1..b8927464df28 100644 --- a/packages/docusaurus-types/src/index.d.ts +++ b/packages/docusaurus-types/src/index.d.ts @@ -21,7 +21,7 @@ import type {Location} from 'history'; // === Configuration === -export type ReportingSeverity = 'ignore' | 'log' | 'warn' | 'error' | 'throw'; +export type ReportingSeverity = 'ignore' | 'log' | 'warn' | 'throw'; export type PluginOptions = {id?: string} & {[key: string]: unknown}; diff --git a/packages/docusaurus-utils/src/__tests__/jsUtils.test.ts b/packages/docusaurus-utils/src/__tests__/jsUtils.test.ts index 5831ad69626b..9ba72646964c 100644 --- a/packages/docusaurus-utils/src/__tests__/jsUtils.test.ts +++ b/packages/docusaurus-utils/src/__tests__/jsUtils.test.ts @@ -12,7 +12,6 @@ import { removePrefix, mapAsyncSequential, findAsyncSequential, - reportMessage, } from '../jsUtils'; describe('removeSuffix', () => { @@ -108,40 +107,3 @@ describe('findAsyncSequential', () => { expect(timeTotal).toBeLessThan(1000); }); }); - -describe('reportMessage', () => { - it('works with all severities', () => { - const consoleLog = jest.spyOn(console, 'info').mockImplementation(() => {}); - const consoleWarn = jest - .spyOn(console, 'warn') - .mockImplementation(() => {}); - const consoleError = jest - .spyOn(console, 'error') - .mockImplementation(() => {}); - reportMessage('hey', 'ignore'); - reportMessage('hey', 'log'); - reportMessage('hey', 'warn'); - reportMessage('hey', 'error'); - expect(() => - reportMessage('hey', 'throw'), - ).toThrowErrorMatchingInlineSnapshot(`"hey"`); - expect(() => - // @ts-expect-error: for test - reportMessage('hey', 'foo'), - ).toThrowErrorMatchingInlineSnapshot( - `"Unexpected "reportingSeverity" value: foo."`, - ); - expect(consoleLog).toBeCalledTimes(1); - expect(consoleLog).toBeCalledWith( - expect.stringMatching(/.*\[INFO\].* hey/), - ); - expect(consoleWarn).toBeCalledTimes(1); - expect(consoleWarn).toBeCalledWith( - expect.stringMatching(/.*\[WARNING\].* hey/), - ); - expect(consoleError).toBeCalledTimes(1); - expect(consoleError).toBeCalledWith( - expect.stringMatching(/.*\[ERROR\].* hey/), - ); - }); -}); diff --git a/packages/docusaurus-utils/src/index.ts b/packages/docusaurus-utils/src/index.ts index 30549ba99c03..06f553c43f1c 100644 --- a/packages/docusaurus-utils/src/index.ts +++ b/packages/docusaurus-utils/src/index.ts @@ -40,7 +40,6 @@ export { removePrefix, mapAsyncSequential, findAsyncSequential, - reportMessage, } from './jsUtils'; export { normalizeUrl, diff --git a/packages/docusaurus-utils/src/jsUtils.ts b/packages/docusaurus-utils/src/jsUtils.ts index 2d1f228f1eb7..d063e045552a 100644 --- a/packages/docusaurus-utils/src/jsUtils.ts +++ b/packages/docusaurus-utils/src/jsUtils.ts @@ -5,9 +5,6 @@ * LICENSE file in the root directory of this source tree. */ -import logger from '@docusaurus/logger'; -import type {ReportingSeverity} from '@docusaurus/types'; - /** Removes a given string suffix from `str`. */ export function removeSuffix(str: string, suffix: string): string { if (suffix === '') { @@ -60,43 +57,3 @@ export async function findAsyncSequential( } return undefined; } - -/** - * Takes a message and reports it according to the severity that the user wants. - * - * - `ignore`: completely no-op - * - `log`: uses the `INFO` log level - * - `warn`: uses the `WARN` log level - * - `error`: uses the `ERROR` log level - * - `throw`: aborts the process, throws the error. - * - * Since the logger doesn't have logging level filters yet, these severities - * mostly just differ by their colors. - * - * @throws In addition to throwing when `reportingSeverity === "throw"`, this - * function also throws if `reportingSeverity` is not one of the above. - */ -export function reportMessage( - message: string, - reportingSeverity: ReportingSeverity, -): void { - switch (reportingSeverity) { - case 'ignore': - break; - case 'log': - logger.info(message); - break; - case 'warn': - logger.warn(message); - break; - case 'error': - logger.error(message); - break; - case 'throw': - throw new Error(message); - default: - throw new Error( - `Unexpected "reportingSeverity" value: ${reportingSeverity}.`, - ); - } -} diff --git a/packages/docusaurus/src/server/__tests__/__snapshots__/configValidation.test.ts.snap b/packages/docusaurus/src/server/__tests__/__snapshots__/configValidation.test.ts.snap index 5aceb9625550..dcce6277c734 100644 --- a/packages/docusaurus/src/server/__tests__/__snapshots__/configValidation.test.ts.snap +++ b/packages/docusaurus/src/server/__tests__/__snapshots__/configValidation.test.ts.snap @@ -161,3 +161,8 @@ exports[`normalizeConfig throws error for unknown field 1`] = ` If you still want these fields to be in your configuration, put them in the "customFields" field. See https://docusaurus.io/docs/api/docusaurus-config/#customfields" `; + +exports[`normalizeConfig throws for "error" reporting severity 1`] = ` +""onBrokenLinks" must be one of [ignore, log, warn, throw] +" +`; diff --git a/packages/docusaurus/src/server/__tests__/configValidation.test.ts b/packages/docusaurus/src/server/__tests__/configValidation.test.ts index a2aad2aeca58..328d63dde1e1 100644 --- a/packages/docusaurus/src/server/__tests__/configValidation.test.ts +++ b/packages/docusaurus/src/server/__tests__/configValidation.test.ts @@ -308,6 +308,20 @@ describe('normalizeConfig', () => { ), ).toThrowErrorMatchingSnapshot(); }); + + it('throws for "error" reporting severity', () => { + expect(() => + validateConfig( + { + title: 'Site', + url: 'https://example.com', + baseUrl: '/', + onBrokenLinks: 'error', + }, + 'docusaurus.config.js', + ), + ).toThrowErrorMatchingSnapshot(); + }); }); describe('config warnings', () => { diff --git a/packages/docusaurus/src/server/brokenLinks.ts b/packages/docusaurus/src/server/brokenLinks.ts index 22f466772de5..f443a4659cf9 100644 --- a/packages/docusaurus/src/server/brokenLinks.ts +++ b/packages/docusaurus/src/server/brokenLinks.ts @@ -11,12 +11,7 @@ import _ from 'lodash'; import logger from '@docusaurus/logger'; import combinePromises from 'combine-promises'; import {matchRoutes} from 'react-router-config'; -import { - removePrefix, - removeSuffix, - reportMessage, - resolvePathname, -} from '@docusaurus/utils'; +import {removePrefix, removeSuffix, resolvePathname} from '@docusaurus/utils'; import {getAllFinalRoutes} from './utils'; import type {RouteConfig, ReportingSeverity} from '@docusaurus/types'; @@ -247,6 +242,6 @@ export async function handleBrokenLinks({ const errorMessage = getBrokenLinksErrorMessage(allBrokenLinks); if (errorMessage) { - reportMessage(errorMessage, onBrokenLinks); + logger.report(onBrokenLinks)(errorMessage); } } diff --git a/packages/docusaurus/src/server/configValidation.ts b/packages/docusaurus/src/server/configValidation.ts index 4027f84ca4d9..5452c03309f5 100644 --- a/packages/docusaurus/src/server/configValidation.ts +++ b/packages/docusaurus/src/server/configValidation.ts @@ -173,13 +173,13 @@ export const ConfigSchema = Joi.object({ trailingSlash: Joi.boolean(), // No default value! undefined = retrocompatible legacy behavior! i18n: I18N_CONFIG_SCHEMA, onBrokenLinks: Joi.string() - .equal('ignore', 'log', 'warn', 'error', 'throw') + .equal('ignore', 'log', 'warn', 'throw') .default(DEFAULT_CONFIG.onBrokenLinks), onBrokenMarkdownLinks: Joi.string() - .equal('ignore', 'log', 'warn', 'error', 'throw') + .equal('ignore', 'log', 'warn', 'throw') .default(DEFAULT_CONFIG.onBrokenMarkdownLinks), onDuplicateRoutes: Joi.string() - .equal('ignore', 'log', 'warn', 'error', 'throw') + .equal('ignore', 'log', 'warn', 'throw') .default(DEFAULT_CONFIG.onDuplicateRoutes), organizationName: Joi.string().allow(''), staticDirectories: Joi.array() diff --git a/packages/docusaurus/src/server/routes.ts b/packages/docusaurus/src/server/routes.ts index b13c8cd9b11a..56230415f127 100644 --- a/packages/docusaurus/src/server/routes.ts +++ b/packages/docusaurus/src/server/routes.ts @@ -7,12 +7,12 @@ import query from 'querystring'; import _ from 'lodash'; +import logger from '@docusaurus/logger'; import { docuHash, normalizeUrl, simpleHash, escapePath, - reportMessage, } from '@docusaurus/utils'; import {getAllFinalRoutes} from './utils'; import type { @@ -228,15 +228,13 @@ export function handleDuplicateRoutes( return false; }); if (duplicatePaths.length > 0) { - const finalMessage = `Duplicate routes found! -${duplicatePaths - .map( - (duplicateRoute) => - `- Attempting to create page at ${duplicateRoute}, but a page already exists at this route.`, - ) - .join('\n')} + logger.report( + onDuplicateRoutes, + )`Duplicate routes found!${duplicatePaths.map( + (duplicateRoute) => + logger.interpolate`Attempting to create page at url=${duplicateRoute}, but a page already exists at this route.`, + )} This could lead to non-deterministic routing behavior.`; - reportMessage(finalMessage, onDuplicateRoutes); } } diff --git a/website/docs/api/docusaurus.config.js.md b/website/docs/api/docusaurus.config.js.md index 45142e79ed64..4960d9af9d8f 100644 --- a/website/docs/api/docusaurus.config.js.md +++ b/website/docs/api/docusaurus.config.js.md @@ -172,7 +172,7 @@ module.exports = { ### `onBrokenLinks` {#onBrokenLinks} -- Type: `'ignore' | 'log' | 'warn' | 'error' | 'throw'` +- Type: `'ignore' | 'log' | 'warn' | 'throw'` The behavior of Docusaurus when it detects any broken link. @@ -186,7 +186,7 @@ The broken links detection is only available for a production build (`docusaurus ### `onBrokenMarkdownLinks` {#onBrokenMarkdownLinks} -- Type: `'ignore' | 'log' | 'warn' | 'error' | 'throw'` +- Type: `'ignore' | 'log' | 'warn' | 'throw'` The behavior of Docusaurus when it detects any broken Markdown link. @@ -194,7 +194,7 @@ By default, it prints a warning, to let you know about your broken Markdown link ### `onDuplicateRoutes` {#onDuplicateRoutes} -- Type: `'ignore' | 'log' | 'warn' | 'error' | 'throw'` +- Type: `'ignore' | 'log' | 'warn' | 'throw'` The behavior of Docusaurus when it detects any [duplicate routes](/guides/creating-pages.md#duplicate-routes). diff --git a/website/docs/api/misc/logger/logger.md b/website/docs/api/misc/logger/logger.md index e4591bb384d3..51aa14163df4 100644 --- a/website/docs/api/misc/logger/logger.md +++ b/website/docs/api/misc/logger/logger.md @@ -32,6 +32,7 @@ It exports a single object as default export: `logger`. `logger` has the followi - `warn`: prints a warning that should be payed attention to. - `error`: prints an error (not necessarily halting the program) that signals significant problems. - `success`: prints a success message. +- The `report` function. It takes a `ReportingSeverity` value (`ignore`, `log`, `warn`, `throw`) and reports a message according to the severity. :::caution A word on the `error` formatter