Skip to content

Commit

Permalink
fix(core): various broken anchor link fixes (#9732)
Browse files Browse the repository at this point in the history
  • Loading branch information
slorber authored Jan 12, 2024
1 parent d94adf6 commit 4388267
Show file tree
Hide file tree
Showing 63 changed files with 345 additions and 115 deletions.
5 changes: 4 additions & 1 deletion .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,10 @@ module.exports = {
})),
],
'no-template-curly-in-string': WARNING,
'no-unused-expressions': [WARNING, {allowTaggedTemplates: true}],
'no-unused-expressions': [
WARNING,
{allowTaggedTemplates: true, allowShortCircuit: true},
],
'no-useless-escape': WARNING,
'no-void': [ERROR, {allowAsStatement: true}],
'prefer-destructuring': WARNING,
Expand Down
4 changes: 2 additions & 2 deletions packages/docusaurus-module-type-aliases/src/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -262,8 +262,8 @@ declare module '@docusaurus/useRouteContext' {

declare module '@docusaurus/useBrokenLinks' {
export type BrokenLinks = {
collectLink: (link: string) => void;
collectAnchor: (anchor: string) => void;
collectLink: (link: string | undefined) => void;
collectAnchor: (anchor: string | undefined) => void;
};

export default function useBrokenLinks(): BrokenLinks;
Expand Down
8 changes: 8 additions & 0 deletions packages/docusaurus-theme-classic/src/theme-classic.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -867,6 +867,14 @@ declare module '@theme/MDXComponents/Ul' {
export default function MDXUl(props: Props): JSX.Element;
}

declare module '@theme/MDXComponents/Li' {
import type {ComponentProps} from 'react';

export interface Props extends ComponentProps<'li'> {}

export default function MDXLi(props: Props): JSX.Element;
}

declare module '@theme/MDXComponents/Img' {
import type {ComponentProps} from 'react';

Expand Down
17 changes: 17 additions & 0 deletions packages/docusaurus-theme-classic/src/theme/MDXComponents/Li.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
/**
* 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, {type ReactNode} from 'react';
import useBrokenLinks from '@docusaurus/useBrokenLinks';
import type {Props} from '@theme/MDXComponents/Li';

export default function MDXLi(props: Props): ReactNode | undefined {
// MDX Footnotes have ids such as <li id="user-content-fn-1-953011">
useBrokenLinks().collectAnchor(props.id);

return <li {...props} />;
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import MDXPre from '@theme/MDXComponents/Pre';
import MDXDetails from '@theme/MDXComponents/Details';
import MDXHeading from '@theme/MDXComponents/Heading';
import MDXUl from '@theme/MDXComponents/Ul';
import MDXLi from '@theme/MDXComponents/Li';
import MDXImg from '@theme/MDXComponents/Img';
import Admonition from '@theme/Admonition';
import Mermaid from '@theme/Mermaid';
Expand All @@ -27,6 +28,7 @@ const MDXComponents: MDXComponentsObject = {
a: MDXA,
pre: MDXPre,
ul: MDXUl,
li: MDXLi,
img: MDXImg,
h1: (props: ComponentProps<'h1'>) => <MDXHeading as="h1" {...props} />,
h2: (props: ComponentProps<'h2'>) => <MDXHeading as="h2" {...props} />,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,9 @@ function DropdownNavbarItemDesktop({
aria-haspopup="true"
aria-expanded={showDropdown}
role="button"
// # hash permits to make the <a> tag focusable in case no link target
// See https://github.com/facebook/docusaurus/pull/6003
// There's probably a better solution though...
href={props.to ? undefined : '#'}
className={clsx('navbar__link', className)}
{...props}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import React, {
type ReactElement,
} from 'react';
import clsx from 'clsx';
import useBrokenLinks from '@docusaurus/useBrokenLinks';
import useIsBrowser from '@docusaurus/useIsBrowser';
import {useCollapsible, Collapsible} from '../Collapsible';
import styles from './styles.module.css';
Expand Down Expand Up @@ -47,6 +48,8 @@ export function Details({
children,
...props
}: DetailsProps): JSX.Element {
useBrokenLinks().collectAnchor(props.id);

const isBrowser = useIsBrowser();
const detailsRef = useRef<HTMLDetailsElement>(null);

Expand Down
23 changes: 23 additions & 0 deletions packages/docusaurus-utils/src/__tests__/urlUtils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,29 @@ describe('parseURLPath', () => {
});
});

it('parse anchor', () => {
expect(parseURLPath('#anchor')).toEqual({
pathname: '/',
search: undefined,
hash: 'anchor',
});
expect(parseURLPath('#anchor', '/page')).toEqual({
pathname: '/page',
search: undefined,
hash: 'anchor',
});
expect(parseURLPath('#')).toEqual({
pathname: '/',
search: undefined,
hash: '',
});
expect(parseURLPath('#', '/page')).toEqual({
pathname: '/page',
search: undefined,
hash: '',
});
});

it('parse hash', () => {
expect(parseURLPath('/page')).toEqual({
pathname: '/page',
Expand Down
8 changes: 4 additions & 4 deletions packages/docusaurus/src/client/BrokenLinksContext.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,11 @@ export const createStatefulBrokenLinks = (): StatefulBrokenLinks => {
const allAnchors = new Set<string>();
const allLinks = new Set<string>();
return {
collectAnchor: (anchor: string): void => {
allAnchors.add(anchor);
collectAnchor: (anchor: string | undefined): void => {
typeof anchor !== 'undefined' && allAnchors.add(anchor);
},
collectLink: (link: string): void => {
allLinks.add(link);
collectLink: (link: string | undefined): void => {
typeof link !== 'undefined' && allLinks.add(link);
},
getCollectedAnchors: (): string[] => [...allAnchors],
getCollectedLinks: (): string[] => [...allLinks],
Expand Down
9 changes: 8 additions & 1 deletion packages/docusaurus/src/client/exports/Link.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -140,13 +140,20 @@ function Link(
};
}, [ioRef, targetLink, IOSupported, isInternal]);

// It is simple local anchor link targeting current page?
const isAnchorLink = targetLink?.startsWith('#') ?? false;

// Should we use a regular <a> tag instead of React-Router Link component?
const isRegularHtmlLink = !targetLink || !isInternal || isAnchorLink;

if (!isRegularHtmlLink && !noBrokenLinkCheck) {
if (!noBrokenLinkCheck && (isAnchorLink || !isRegularHtmlLink)) {
brokenLinks.collectLink(targetLink!);
}

if (props.id) {
brokenLinks.collectAnchor(props.id);
}

return isRegularHtmlLink ? (
// eslint-disable-next-line jsx-a11y/anchor-has-content, @docusaurus/no-html-links
<a
Expand Down
152 changes: 130 additions & 22 deletions packages/docusaurus/src/server/__tests__/brokenLinks.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,71 @@ describe('handleBrokenLinks', () => {
});
});

it('accepts valid link with anchor reported with hash prefix', async () => {
await testBrokenLinks({
routes: [{path: '/page1'}, {path: '/page2'}],
collectedLinks: {
'/page1': {links: ['/page2#page2anchor'], anchors: []},
'/page2': {links: [], anchors: ['#page2anchor']},
},
});
});

it('accepts valid links and anchors, sparse arrays', async () => {
await testBrokenLinks({
routes: [{path: '/page1'}, {path: '/page2'}],
collectedLinks: {
'/page1': {
links: [
'/page1',
// @ts-expect-error: invalid type on purpose
undefined,
// @ts-expect-error: invalid type on purpose
null,
// @ts-expect-error: invalid type on purpose
42,
'/page2',
'/page2#page2anchor1',
'/page2#page2anchor2',
],
anchors: [],
},
'/page2': {
links: [],
anchors: [
'page2anchor1',
// @ts-expect-error: invalid type on purpose
undefined,
// @ts-expect-error: invalid type on purpose
null,
// @ts-expect-error: invalid type on purpose
42,
'page2anchor2',
],
},
},
});
});

it('accepts valid link and anchor to collected pages that are not in routes', async () => {
// This tests the edge-case of the 404 page:
// We don't have a {path: '404.html'} route
// But yet we collect links/anchors to it and allow linking to it
await testBrokenLinks({
routes: [],
collectedLinks: {
'/page 1': {
links: ['/page 2#anchor-page-2'],
anchors: ['anchor-page-1'],
},
'/page 2': {
links: ['/page 1#anchor-page-1', '/page%201#anchor-page-1'],
anchors: ['anchor-page-2'],
},
},
});
});

it('accepts valid link with querystring + anchor', async () => {
await testBrokenLinks({
routes: [{path: '/page1'}, {path: '/page2'}],
Expand Down Expand Up @@ -132,10 +197,75 @@ describe('handleBrokenLinks', () => {
'/page%202',
'/page%202?age=42',
'/page%202?age=42#page2anchor',

'/some dir/page 3',
'/some dir/page 3#page3anchor',
'/some%20dir/page%203',
'/some%20dir/page%203#page3anchor',
'/some%20dir/page 3',
'/some dir/page%203',
'/some dir/page%203#page3anchor',
],
anchors: [],
},
'/page 2': {links: [], anchors: ['page2anchor']},
'/some dir/page 3': {links: [], anchors: ['page3anchor']},
},
});
});

it('accepts valid link with anchor with spaces and encoding', async () => {
await testBrokenLinks({
routes: [{path: '/page 1'}, {path: '/page 2'}],
collectedLinks: {
'/page 1': {
links: [
'/page 1#a b',
'#a b',
'#a%20b',
'#c d',
'#c%20d',

'/page 2#你好',
'/page%202#你好',
'/page 2#%E4%BD%A0%E5%A5%BD',
'/page%202#%E4%BD%A0%E5%A5%BD',

'/page 2#schrödingers-cat-principle',
'/page%202#schrödingers-cat-principle',
'/page 2#schr%C3%B6dingers-cat-principle',
'/page%202#schr%C3%B6dingers-cat-principle',
],
anchors: ['a b', 'c%20d'],
},
'/page 2': {
links: ['/page 1#a b', '/page%201#c d'],
anchors: ['你好', '#schr%C3%B6dingers-cat-principle'],
},
},
});
});

it('accepts valid link with empty anchor', async () => {
await testBrokenLinks({
routes: [{path: '/page 1'}, {path: '/page 2'}],
collectedLinks: {
'/page 1': {
links: [
'/page 1',
'/page 2',
'/page 1#',
'/page 2#',
'/page 1?age=42#',
'/page 2?age=42#',
'#',
'#',
'./page 1#',
'./page 2#',
],
anchors: [],
},
'/page 2': {links: [], anchors: []},
},
});
});
Expand Down Expand Up @@ -225,28 +355,6 @@ describe('handleBrokenLinks', () => {
`);
});

it('rejects valid link with empty broken anchor', async () => {
await expect(() =>
testBrokenLinks({
routes: [{path: '/page1'}, {path: '/page2'}],
collectedLinks: {
'/page1': {links: ['/page2#'], anchors: []},
'/page2': {links: [], anchors: []},
},
}),
).rejects.toThrowErrorMatchingInlineSnapshot(`
"Docusaurus found broken anchors!
Please check the pages of your site in the list below, and make sure you don't reference any anchor that does not exist.
Note: it's possible to ignore broken anchors with the 'onBrokenAnchors' Docusaurus configuration, and let the build pass.
Exhaustive list of all broken anchors found:
- Broken anchor on source page path = /page1:
-> linking to /page2#
"
`);
});

it('rejects valid link with broken anchor + query-string', async () => {
await expect(() =>
testBrokenLinks({
Expand Down
Loading

0 comments on commit 4388267

Please sign in to comment.