Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: remove "error" reporting level, move reportMessage to logger #7642

Merged
merged 1 commit into from Jun 17, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/docusaurus-logger/README.md
Expand Up @@ -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

Expand Down
30 changes: 30 additions & 0 deletions packages/docusaurus-logger/src/__tests__/index.test.ts
Expand Up @@ -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/),
);
});
});
45 changes: 45 additions & 0 deletions packages/docusaurus-logger/src/index.ts
Expand Up @@ -6,6 +6,7 @@
*/

import chalk from 'chalk';
import type {ReportingSeverity} from '@docusaurus/types';

type InterpolatableValue = string | number | (string | number)[];

Expand Down Expand Up @@ -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),
Expand All @@ -144,6 +188,7 @@ const logger = {
warn,
error,
success,
report,
newLine,
};

Expand Down
7 changes: 3 additions & 4 deletions packages/docusaurus-plugin-content-blog/src/index.ts
Expand Up @@ -6,12 +6,12 @@
*/

import path from 'path';
import logger from '@docusaurus/logger';
import {
normalizeUrl,
docuHash,
aliasedSitePath,
getPluginI18nPath,
reportMessage,
posixPath,
addTrailingPathSeparator,
createAbsoluteFilePathMatcher,
Expand Down Expand Up @@ -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}`;
},
};

Expand Down
9 changes: 2 additions & 7 deletions packages/docusaurus-plugin-content-docs/src/index.ts
Expand Up @@ -13,7 +13,6 @@ import {
docuHash,
aliasedSitePath,
getContentPathList,
reportMessage,
posixPath,
addTrailingPathSeparator,
createAbsoluteFilePathMatcher,
Expand Down Expand Up @@ -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}`;
},
};

Expand Down
2 changes: 1 addition & 1 deletion packages/docusaurus-types/src/index.d.ts
Expand Up @@ -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};

Expand Down
38 changes: 0 additions & 38 deletions packages/docusaurus-utils/src/__tests__/jsUtils.test.ts
Expand Up @@ -12,7 +12,6 @@ import {
removePrefix,
mapAsyncSequential,
findAsyncSequential,
reportMessage,
} from '../jsUtils';

describe('removeSuffix', () => {
Expand Down Expand Up @@ -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/),
);
});
});
1 change: 0 additions & 1 deletion packages/docusaurus-utils/src/index.ts
Expand Up @@ -40,7 +40,6 @@ export {
removePrefix,
mapAsyncSequential,
findAsyncSequential,
reportMessage,
} from './jsUtils';
export {
normalizeUrl,
Expand Down
43 changes: 0 additions & 43 deletions packages/docusaurus-utils/src/jsUtils.ts
Expand Up @@ -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 === '') {
Expand Down Expand Up @@ -60,43 +57,3 @@ export async function findAsyncSequential<T>(
}
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}.`,
);
}
}
Expand Up @@ -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]
"
`;
Expand Up @@ -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', () => {
Expand Down
9 changes: 2 additions & 7 deletions packages/docusaurus/src/server/brokenLinks.ts
Expand Up @@ -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';

Expand Down Expand Up @@ -247,6 +242,6 @@ export async function handleBrokenLinks({

const errorMessage = getBrokenLinksErrorMessage(allBrokenLinks);
if (errorMessage) {
reportMessage(errorMessage, onBrokenLinks);
logger.report(onBrokenLinks)(errorMessage);
}
}
6 changes: 3 additions & 3 deletions packages/docusaurus/src/server/configValidation.ts
Expand Up @@ -173,13 +173,13 @@ export const ConfigSchema = Joi.object<DocusaurusConfig>({
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()
Expand Down
16 changes: 7 additions & 9 deletions packages/docusaurus/src/server/routes.ts
Expand Up @@ -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 {
Expand Down Expand Up @@ -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);
}
}

Expand Down