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

feat(v2): broken links detection #3059

Merged
merged 8 commits into from
Jul 21, 2020
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions packages/docusaurus-init/templates/bootstrap/docs/doc1.md
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,6 @@ Strikethrough uses two tildes. ~~Scratch this.~~

[I'm a reference-style link][arbitrary case-insensitive reference text]

[I'm a relative reference to a repository file](../blob/master/LICENSE)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI I removed it because this template link is actually broken (this was failing the CI).

It is interpreted as an internal SPA link, and ReactRouter shows the 404:

image

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well then, I am glad this feature already found one broken link :)


[You can use numbers for reference-style link definitions][1]

Or leave it empty and use the [link text itself].
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ module.exports = {
tagline: 'The tagline of my site',
url: 'https://your-docusaurus-test-site.com',
baseUrl: '/',
onBrokenLinks: 'throw',
favicon: 'img/favicon.ico',
organizationName: 'facebook', // Usually your GitHub org/user name.
projectName: 'docusaurus', // Usually your repo name.
Expand Down
2 changes: 0 additions & 2 deletions packages/docusaurus-init/templates/classic/docs/doc1.md
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,6 @@ Strikethrough uses two tildes. ~~Scratch this.~~

[I'm a reference-style link][arbitrary case-insensitive reference text]

[I'm a relative reference to a repository file](../blob/master/LICENSE)

[You can use numbers for reference-style link definitions][1]

Or leave it empty and use the [link text itself].
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ module.exports = {
tagline: 'The tagline of my site',
url: 'https://your-docusaurus-test-site.com',
baseUrl: '/',
onBrokenLinks: 'throw',
favicon: 'img/favicon.ico',
organizationName: 'facebook', // Usually your GitHub org/user name.
projectName: 'docusaurus', // Usually your repo name.
Expand Down
2 changes: 0 additions & 2 deletions packages/docusaurus-init/templates/facebook/docs/doc1.md
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,6 @@ Strikethrough uses two tildes. ~~Scratch this.~~

[I'm a reference-style link][arbitrary case-insensitive reference text]

[I'm a relative reference to a repository file](../blob/master/LICENSE)

[You can use numbers for reference-style link definitions][1]

Or leave it empty and use the [link text itself].
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ module.exports = {
tagline: 'The tagline of my site',
url: 'https://your-docusaurus-test-site.com',
baseUrl: '/',
onBrokenLinks: 'throw',
favicon: 'img/favicon.ico',
organizationName: 'facebook', // Usually your GitHub org/user name.
projectName: 'docusaurus', // Usually your repo name.
Expand Down
4 changes: 4 additions & 0 deletions packages/docusaurus-types/src/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,15 @@ import {Command} from 'commander';
import {ParsedUrlQueryInput} from 'querystring';
import {MergeStrategy} from 'webpack-merge';

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

export interface DocusaurusConfig {
baseUrl: string;
favicon: string;
tagline?: string;
title: string;
url: string;
onBrokenLinks: OnBrokenLinks;
organizationName?: string;
projectName?: string;
githubHost?: string;
Expand Down Expand Up @@ -111,6 +114,7 @@ export interface InjectedHtmlTags {
export type HtmlTags = string | HtmlTagObject | (string | HtmlTagObject)[];

export interface Props extends LoadContext, InjectedHtmlTags {
routes: RouteConfig[];
routesPaths: string[];
plugins: Plugin<any, unknown>[];
}
Expand Down
2 changes: 2 additions & 0 deletions packages/docusaurus/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@
"import-fresh": "^3.2.1",
"inquirer": "^7.2.0",
"is-root": "^2.1.0",
"lodash": "^4.5.2",
"lodash.has": "^4.5.2",
"lodash.isplainobject": "^4.0.6",
"lodash.isstring": "^4.0.1",
Expand All @@ -90,6 +91,7 @@
"react-router": "^5.1.2",
"react-router-config": "^5.1.1",
"react-router-dom": "^5.1.2",
"resolve-pathname": "^3.0.0",
"semver": "^6.3.0",
"serve-handler": "^6.1.3",
"shelljs": "^0.8.4",
Expand Down
50 changes: 50 additions & 0 deletions packages/docusaurus/src/client/LinksCollector.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
/**
* 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 React, {ReactNode, useContext, createContext} from 'react';

type LinksCollector = {
collectLink: (link: string) => void;
};

type StatefulLinksCollector = LinksCollector & {
getCollectedLinks: () => string[];
};

export const createStatefulLinksCollector = (): StatefulLinksCollector => {
// Set to dedup, as it's not useful to collect multiple times the same link
const allLinks = new Set<string>();
return {
collectLink: (link: string): void => {
allLinks.add(link);
},
getCollectedLinks: (): string[] => {
return [...allLinks];
},
};
};

const Context = createContext<LinksCollector>({
collectLink: () => {
// noop by default for client
// we only use the broken links checker server-side
},
});

export const useLinksCollector = () => {
return useContext(Context);
};

export const ProvideLinksCollector = ({
children,
linksCollector,
}: {
children: ReactNode;
linksCollector: LinksCollector;
}) => {
return <Context.Provider value={linksCollector}>{children}</Context.Provider>;
};
11 changes: 10 additions & 1 deletion packages/docusaurus/src/client/exports/Link.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import React, {ReactNode, useEffect, useRef} from 'react';
import {NavLink, Link as RRLink} from 'react-router-dom';
import isInternalUrl from './isInternalUrl';
import ExecutionEnvironment from './ExecutionEnvironment';
import {useLinksCollector} from '../LinksCollector';

declare global {
interface Window {
Expand All @@ -26,6 +27,7 @@ interface Props {
}

function Link({isNavLink, activeClassName, ...props}: Props): JSX.Element {
const linksCollector = useLinksCollector();
const {to, href} = props;
const targetLink = to || href;
const isInternal = isInternalUrl(targetLink);
Expand Down Expand Up @@ -84,7 +86,14 @@ function Link({isNavLink, activeClassName, ...props}: Props): JSX.Element {
};
}, [targetLink, IOSupported, isInternal]);

return !targetLink || !isInternal || targetLink.startsWith('#') ? (
const isAnchorLink = targetLink?.startsWith('#') ?? false;
const isRegularHtmlLink = !targetLink || !isInternal || isAnchorLink;

if (isInternal && !isAnchorLink) {
linksCollector.collectLink(targetLink);
}

return isRegularHtmlLink ? (
// eslint-disable-next-line jsx-a11y/anchor-has-content
<a
// @ts-expect-error: href specified twice needed to pass children and other user specified props
Expand Down
21 changes: 18 additions & 3 deletions packages/docusaurus/src/client/serverEntry.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,22 +22,38 @@ import packageJson from '../../package.json';
import preload from './preload';
// eslint-disable-next-line import/no-unresolved
import App from './App';
import {
createStatefulLinksCollector,
ProvideLinksCollector,
} from './LinksCollector';
import ssrTemplate from './templates/ssr.html.template';

// Renderer for static-site-generator-webpack-plugin (async rendering via promises).
export default async function render(locals) {
const {routesLocation, headTags, preBodyTags, postBodyTags} = locals;
const {
routesLocation,
headTags,
preBodyTags,
postBodyTags,
onLinksCollected,
baseUrl,
} = locals;
const location = routesLocation[locals.path];
await preload(routes, location);
const modules = new Set();
const context = {};

const linksCollector = createStatefulLinksCollector();
const appHtml = ReactDOMServer.renderToString(
<Loadable.Capture report={(moduleName) => modules.add(moduleName)}>
<StaticRouter location={location} context={context}>
<App />
<ProvideLinksCollector linksCollector={linksCollector}>
<App />
</ProvideLinksCollector>
</StaticRouter>
</Loadable.Capture>,
);
onLinksCollected(location, linksCollector.getCollectedLinks());

const helmet = Helmet.renderStatic();
const htmlAttributes = helmet.htmlAttributes.toString();
Expand All @@ -59,7 +75,6 @@ export default async function render(locals) {
const bundles = getBundles(manifest, modulesToBeLoaded);
const stylesheets = (bundles.css || []).map((b) => b.file);
const scripts = (bundles.js || []).map((b) => b.file);
const {baseUrl} = locals;

const renderedHtml = eta.render(
ssrTemplate.trim(),
Expand Down
22 changes: 20 additions & 2 deletions packages/docusaurus/src/commands/build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ import {BundleAnalyzerPlugin} from 'webpack-bundle-analyzer';
import merge from 'webpack-merge';
import {STATIC_DIR_NAME} from '../constants';
import {load} from '../server';
import {handleBrokenLinks} from '../server/brokenLinks';

import {BuildCLIOptions, Props} from '@docusaurus/types';
import createClientConfig from '../webpack/client';
import createServerConfig from '../webpack/server';
Expand All @@ -33,7 +35,13 @@ export default async function build(
const props: Props = await load(siteDir, cliOptions.outDir);

// Apply user webpack config.
const {outDir, generatedFilesDir, plugins} = props;
const {
outDir,
generatedFilesDir,
plugins,
siteConfig: {onBrokenLinks},
routes,
} = props;

const clientManifestPath = path.join(
generatedFilesDir,
Expand All @@ -55,7 +63,14 @@ export default async function build(
},
);

let serverConfig: Configuration = createServerConfig(props);
const allCollectedLinks: Record<string, string[]> = {};

let serverConfig: Configuration = createServerConfig({
props,
onLinksCollected: (staticPagePath, links) => {
allCollectedLinks[staticPagePath] = links;
},
});

const staticDir = path.resolve(siteDir, STATIC_DIR_NAME);
if (fs.existsSync(staticDir)) {
Expand Down Expand Up @@ -124,6 +139,8 @@ export default async function build(
}),
);

handleBrokenLinks({allCollectedLinks, routes, onBrokenLinks});

const relativeDir = path.relative(process.cwd(), outDir);
console.log(
`\n${chalk.green('Success!')} Generated static files in ${chalk.cyan(
Expand All @@ -135,5 +152,6 @@ export default async function build(
if (forceTerminate && !cliOptions.bundleAnalyzer) {
process.exit(0);
}

return outDir;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`brokenLinks getBrokenLinksErrorMessage 1`] = `
"Broken links found!

- Page path = /docs/mySourcePage:
-> link to ./myBrokenLink (resolved as: /docs/myBrokenLink)
-> link to ../otherBrokenLink (resolved as: /otherBrokenLink),

- Page path = /otherSourcePage:
-> link to /badLink
"
`;
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ Object {
"baseUrl": "/",
"customFields": Object {},
"favicon": "img/docusaurus.ico",
"onBrokenLinks": "throw",
"organizationName": "endiliey",
"plugins": Array [
Array [
Expand Down
Loading