Skip to content

Commit

Permalink
feat(v2): warn user when there are conflicting routes (#3083)
Browse files Browse the repository at this point in the history
* feat(v2): add warning for path override

* feat(v2): check all routes recursively

* docs(v2): add docs for conflicting routes

* style(v2): improve comments in code

* refactor(v2): remove unused lifecycle method from docs plugin

* Revert "refactor(v2): remove unused lifecycle method from docs plugin"

This reverts commit 8b2caaa.

* feat(v2): add option for changing duplicate path behavior

* feat(v2): decouple logging from logic and detect duplicate routes in one pass

* test(v2): fix failing tests

* test(v2): add tests for duplicateRoutes

* test(v2): add test for handleDuplicateRoutes

* style(v2): add else statement

* docs(v2): modify documentation for duplicate routes

* docs(v2): move doc into guides folder

* fix(v2): fix broken links

* docs(v2): move docs for docusaurus config into api folder

* style(v2): add comments

* refactor(v2): extract getFinalRoutes

* refactor(v2): scope getFinalRoutes to docusaurus package

* test(v2): remove obsolete snapshots

* docs(v2): remove some docs

* fix(v2): rerun github actions

* docs(v2): change slug of docs in api folder

* refactor(v2): extract out a reportMessage method

* refactor(v2): extract getAllFinalRoutes

* test(v2): replace snapshots with actual value

* style(v2): remove unnecessary comment and change type

* chore(v2): remove unused dependency

* style(v2): remove unused code

* Update packages/docusaurus/src/server/utils.ts

* Update website/docs/guides/creating-pages.md

Co-authored-by: Sébastien Lorber <[email protected]>
  • Loading branch information
teikjun and slorber authored Jul 31, 2020
1 parent 62c61c9 commit 300aecb
Show file tree
Hide file tree
Showing 19 changed files with 234 additions and 33 deletions.
5 changes: 3 additions & 2 deletions packages/docusaurus-types/src/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,16 @@ import {Command} from 'commander';
import {ParsedUrlQueryInput} from 'querystring';
import {MergeStrategy} from 'webpack-merge';

export type OnBrokenLinks = 'ignore' | 'log' | 'error' | 'throw';
export type ReportingSeverity = 'ignore' | 'log' | 'warn' | 'error' | 'throw';

export interface DocusaurusConfig {
baseUrl: string;
favicon: string;
tagline?: string;
title: string;
url: string;
onBrokenLinks: OnBrokenLinks;
onBrokenLinks: ReportingSeverity;
onDuplicateRoutes: ReportingSeverity;
organizationName?: string;
projectName?: string;
githubHost?: string;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ Object {
"customFields": Object {},
"favicon": "img/docusaurus.ico",
"onBrokenLinks": "throw",
"onDuplicateRoutes": "warn",
"organizationName": "endiliey",
"plugins": Array [
Array [
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`duplicateRoutes getDuplicateRoutesMessage 1`] = `
"Attempting to create page at /, but a page already exists at this route
Attempting to create page at /, but a page already exists at this route
Attempting to create page at /blog, but a page already exists at this route
Attempting to create page at /doc/search, but a page already exists at this route"
`;

exports[`duplicateRoutes handleDuplicateRoutes 1`] = `
"Duplicate routes found!
Attempting to create page at /search, but a page already exists at this route
Attempting to create page at /sameDoc, but a page already exists at this route
This could lead to non-deterministic routing behavior"
`;
54 changes: 54 additions & 0 deletions packages/docusaurus/src/server/__tests__/duplicateRoutes.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
/**
* Copyright (c) Facebook, Inc. and its affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/
import {
getAllDuplicateRoutes,
getDuplicateRoutesMessage,
handleDuplicateRoutes,
} from '../duplicateRoutes';
import {RouteConfig} from '@docusaurus/types';

const routes: RouteConfig[] = [
{
path: '/',
component: '',
routes: [
{path: '/search', component: ''},
{path: '/sameDoc', component: ''},
],
},
{
path: '/',
component: '',
routes: [
{path: '/search', component: ''},
{path: '/sameDoc', component: ''},
{path: '/uniqueDoc', component: ''},
],
},
];

describe('duplicateRoutes', () => {
test('getDuplicateRoutesMessage', () => {
const message = getDuplicateRoutesMessage([
'/',
'/',
'/blog',
'/doc/search',
]);
expect(message).toMatchSnapshot();
});

test('getAllDuplicateRoutes', () => {
expect(getAllDuplicateRoutes(routes)).toEqual(['/search', '/sameDoc']);
});

test('handleDuplicateRoutes', () => {
expect(() => {
handleDuplicateRoutes(routes, 'throw');
}).toThrowErrorMatchingSnapshot();
});
});
32 changes: 32 additions & 0 deletions packages/docusaurus/src/server/__tests__/utils.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
/**
* Copyright (c) Facebook, Inc. and its affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/
import {RouteConfig} from '@docusaurus/types';
import {getAllFinalRoutes} from '../utils';

describe('getAllFinalRoutes', () => {
test('should get final routes correctly', () => {
const routes: RouteConfig[] = [
{
path: '/docs',
component: '',
routes: [
{path: '/docs/someDoc', component: ''},
{path: '/docs/someOtherDoc', component: ''},
],
},
{
path: '/community',
component: '',
},
];
expect(getAllFinalRoutes(routes)).toEqual([
routes[0].routes[0],
routes[0].routes[1],
routes[1],
]);
});
});
26 changes: 6 additions & 20 deletions packages/docusaurus/src/server/brokenLinks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,11 @@

import {matchRoutes, RouteConfig as RRRouteConfig} from 'react-router-config';
import resolvePathname from 'resolve-pathname';
import chalk from 'chalk';
import fs from 'fs-extra';
import {mapValues, pickBy, flatMap} from 'lodash';
import {RouteConfig, OnBrokenLinks} from '@docusaurus/types';
import {mapValues, pickBy} from 'lodash';
import {RouteConfig, ReportingSeverity} from '@docusaurus/types';
import {removePrefix} from '@docusaurus/utils';
import {getAllFinalRoutes, reportMessage} from './utils';

function toReactRouterRoutes(routes: RouteConfig[]): RRRouteConfig[] {
// @ts-expect-error: types incompatible???
Expand Down Expand Up @@ -59,12 +59,8 @@ function getPageBrokenLinks({
// For this reason, we only consider the "final routes", that do not have subroutes
// We also need to remove the match all 404 route
function filterIntermediateRoutes(routesInput: RouteConfig[]): RouteConfig[] {
function getFinalRoutes(route: RouteConfig): RouteConfig[] {
return route.routes ? flatMap(route.routes, getFinalRoutes) : [route];
}

const routesWithout404 = routesInput.filter((route) => route.path !== '*');
return flatMap(routesWithout404, getFinalRoutes);
return getAllFinalRoutes(routesWithout404);
}

export function getAllBrokenLinks({
Expand Down Expand Up @@ -152,7 +148,7 @@ export async function handleBrokenLinks({
outDir,
}: {
allCollectedLinks: Record<string, string[]>;
onBrokenLinks: OnBrokenLinks;
onBrokenLinks: ReportingSeverity;
routes: RouteConfig[];
baseUrl: string;
outDir: string;
Expand All @@ -177,16 +173,6 @@ export async function handleBrokenLinks({
const errorMessage = getBrokenLinksErrorMessage(allBrokenLinks);
if (errorMessage) {
const finalMessage = `${errorMessage}\nNote: it's possible to ignore broken links with the 'onBrokenLinks' Docusaurus configuration.\n\n`;

// Useful to ensure the CI fails in case of broken link
if (onBrokenLinks === 'throw') {
throw new Error(finalMessage);
} else if (onBrokenLinks === 'error') {
console.error(chalk.red(finalMessage));
} else if (onBrokenLinks === 'log') {
console.log(chalk.blue(finalMessage));
} else {
throw new Error(`unexpected onBrokenLinks value=${onBrokenLinks}`);
}
reportMessage(finalMessage, onBrokenLinks);
}
}
9 changes: 7 additions & 2 deletions packages/docusaurus/src/server/configValidation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,18 +11,20 @@ import Joi from '@hapi/joi';
import {
isValidationDisabledEscapeHatch,
logValidationBugReportHint,
} from './validationUtils';
} from './utils';

export const DEFAULT_CONFIG: Pick<
DocusaurusConfig,
| 'onBrokenLinks'
| 'onDuplicateRoutes'
| 'plugins'
| 'themes'
| 'presets'
| 'customFields'
| 'themeConfig'
> = {
onBrokenLinks: 'throw',
onDuplicateRoutes: 'warn',
plugins: [],
themes: [],
presets: [],
Expand Down Expand Up @@ -54,8 +56,11 @@ const ConfigSchema = Joi.object({
title: Joi.string().required(),
url: Joi.string().uri().required(),
onBrokenLinks: Joi.string()
.equal('ignore', 'log', 'error', 'throw')
.equal('ignore', 'log', 'warn', 'error', 'throw')
.default(DEFAULT_CONFIG.onBrokenLinks),
onDuplicateRoutes: Joi.string()
.equal('ignore', 'log', 'warn', 'error', 'throw')
.default(DEFAULT_CONFIG.onDuplicateRoutes),
organizationName: Joi.string().allow(''),
projectName: Joi.string().allow(''),
customFields: Joi.object().unknown().default(DEFAULT_CONFIG.customFields),
Expand Down
53 changes: 53 additions & 0 deletions packages/docusaurus/src/server/duplicateRoutes.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
/**
* Copyright (c) Facebook, Inc. and its affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/
import {RouteConfig, ReportingSeverity} from '@docusaurus/types';
import {getAllFinalRoutes, reportMessage} from './utils';

export function getAllDuplicateRoutes(
pluginsRouteConfigs: RouteConfig[],
): string[] {
const allRoutes: string[] = getAllFinalRoutes(pluginsRouteConfigs).map(
(routeConfig) => routeConfig.path,
);
const seenRoutes: Record<string, any> = {};
const duplicateRoutes: string[] = allRoutes.filter(function (route) {
if (seenRoutes.hasOwnProperty(route)) {
return true;
} else {
seenRoutes[route] = true;
return false;
}
});
return duplicateRoutes;
}

export function getDuplicateRoutesMessage(
allDuplicateRoutes: string[],
): string {
const message = allDuplicateRoutes
.map(
(duplicateRoute) =>
`Attempting to create page at ${duplicateRoute}, but a page already exists at this route`,
)
.join('\n');
return message;
}

export function handleDuplicateRoutes(
pluginsRouteConfigs: RouteConfig[],
onDuplicateRoutes: ReportingSeverity,
): void {
if (onDuplicateRoutes === 'ignore') {
return;
}
const duplicatePaths: string[] = getAllDuplicateRoutes(pluginsRouteConfigs);
const message: string = getDuplicateRoutesMessage(duplicatePaths);
if (message) {
const finalMessage = `Duplicate routes found!\n${message}\nThis could lead to non-deterministic routing behavior`;
reportMessage(finalMessage, onDuplicateRoutes);
}
}
3 changes: 3 additions & 0 deletions packages/docusaurus/src/server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import {
} from '@docusaurus/types';
import {loadHtmlTags} from './html-tags';
import {getPackageJsonVersion} from './versions';
import {handleDuplicateRoutes} from './duplicateRoutes';

export function loadContext(
siteDir: string,
Expand Down Expand Up @@ -79,6 +80,8 @@ export async function load(
context,
});

handleDuplicateRoutes(pluginsRouteConfigs, siteConfig.onDuplicateRoutes);

// Site config must be generated after plugins
// We want the generated config to have been normalized by the plugins!
const genSiteConfig = generate(
Expand Down
2 changes: 1 addition & 1 deletion packages/docusaurus/src/server/plugins/init.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import * as Joi from '@hapi/joi';
import {
isValidationDisabledEscapeHatch,
logValidationBugReportHint,
} from '../validationUtils';
} from '../utils';

function pluginOptionsValidator<T>(
schema: ValidationSchema<T>,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
* LICENSE file in the root directory of this source tree.
*/
import chalk from 'chalk';
import flatMap from 'lodash.flatmap';
import {RouteConfig, ReportingSeverity} from '@docusaurus/types';

// TODO temporary escape hatch for alpha-60: to be removed soon
// Our validation schemas might be buggy at first
Expand All @@ -31,3 +33,36 @@ export const logValidationBugReportHint = () => {
)}\n`,
);
};

// Recursively get the final routes (routes with no subroutes)
export function getAllFinalRoutes(routeConfig: RouteConfig[]): RouteConfig[] {
function getFinalRoutes(route: RouteConfig): RouteConfig[] {
return route.routes ? flatMap(route.routes, getFinalRoutes) : [route];
}
return flatMap(routeConfig, getFinalRoutes);
}

export function reportMessage(
message: string,
reportingSeverity: ReportingSeverity,
): void {
switch (reportingSeverity) {
case 'ignore':
break;
case 'log':
console.log(chalk.bold.blue('info ') + chalk.blue(message));
break;
case 'warn':
console.warn(chalk.bold.yellow('warn ') + chalk.yellow(message));
break;
case 'error':
console.error(chalk.bold.red('error ') + chalk.red(message));
break;
case 'throw':
throw new Error(message);
default:
throw new Error(
`unexpected reportingSeverity value: ${reportingSeverity}`,
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
id: docusaurus.config.js
title: docusaurus.config.js
description: API reference for Docusaurus configuration file.
slug: /docusaurus.config.js
---

## Overview
Expand Down Expand Up @@ -81,7 +82,7 @@ module.exports = {

### `onBrokenLinks`

- Type: `'ignore' | 'log' | 'error' | 'throw'`
- Type: `'ignore' | 'log' | 'warn' | 'error' | 'throw'`

The behavior of Docusaurus, when it detects any broken link.

Expand All @@ -93,6 +94,14 @@ The broken links detection is only available for a production build (`docusaurus

:::

### `onDuplicateRoutes`

- Type: `'ignore' | 'log' | 'warn' | 'error' | 'throw'`

The behavior of Docusaurus when it detects any [duplicate routes](/guides/creating-pages.md#duplicate-routes).

By default, it displays a warning after you run `yarn start` or `yarn build`.

### `tagline`

- Type: `string`
Expand Down
2 changes: 1 addition & 1 deletion website/docs/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ The high-level overview of Docusaurus configuration can be categorized into:
- [Custom configurations](#custom-configurations)
- [Customizing Babel Configuration](#customizing-babel-configuration)

For exact reference to each of the configurable fields, you may refer to [**`docusaurus.config.js` API reference**](docusaurus.config.js.md).
For exact reference to each of the configurable fields, you may refer to [**`docusaurus.config.js` API reference**](api/docusaurus.config.js.md).

### Site metadata

Expand Down
2 changes: 1 addition & 1 deletion website/docs/docusaurus-core.md
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ function MyComponent() {

### `useDocusaurusContext`

React hook to access Docusaurus Context. Context contains `siteConfig` object from [docusaurus.config.js](docusaurus.config.js.md), and some additional site metadata.
React hook to access Docusaurus Context. Context contains `siteConfig` object from [docusaurus.config.js](api/docusaurus.config.js.md), and some additional site metadata.

```ts
type DocusaurusPluginVersionInformation =
Expand Down
Loading

0 comments on commit 300aecb

Please sign in to comment.