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

fix(core): various broken anchor link fixes #9732

Merged
merged 25 commits into from
Jan 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
3697f95
add more parseURLPath unit tests
slorber Jan 11, 2024
194642d
add try/catch/cause to improve troubleshooting in case of getBrokenLi…
slorber Jan 11, 2024
4cb761b
DropdownNavbarItem should not check for broken links on # anchors
slorber Jan 11, 2024
30b7852
add broken anchors test page
slorber Jan 11, 2024
4070bfc
Link should report anchors links to the broken links checker
slorber Jan 11, 2024
6dbc6dc
fix #wrapping anchor link
slorber Jan 11, 2024
1cb85f4
fix migration anchor link
slorber Jan 11, 2024
f1dd8c1
fix preset options anchor links
slorber Jan 11, 2024
fddce81
fix migration-automated.mdx broken anchors
slorber Jan 11, 2024
dae7c8d
APITable should collectAnchor ids
slorber Jan 11, 2024
0b9c28b
fix autogenerated link
slorber Jan 11, 2024
ac34d64
fix #sidebar-association anchor link
slorber Jan 11, 2024
d244b0d
Broken link checker should accept empty anchors
slorber Jan 11, 2024
eb1763f
Broken link checker should accept empty hash links and unsafe/bad col…
slorber Jan 11, 2024
7084ad4
revert DropdownNavbarItemDesktop change
slorber Jan 11, 2024
01dfbe9
Relax broken link checker, let it accept any pathname that was collected
slorber Jan 11, 2024
58d7c40
remove problematic anchor link, see bug reported https://github.com/f…
slorber Jan 11, 2024
1ba0a88
broken link checker should support hash uri encoding
slorber Jan 11, 2024
64d1b42
Link component should collect id anchors
slorber Jan 11, 2024
d292dc1
refactor: apply lint autofix
slorber Jan 11, 2024
4fe43f6
micro optim
slorber Jan 11, 2024
706f0e6
MDX footnotes LI should collect anchors
slorber Jan 12, 2024
62aac7f
Merge remote-tracking branch 'origin/slorber/fix-Link-broken-anchors-…
slorber Jan 12, 2024
8e66e05
Allow to collect undefined links/anchors
slorber Jan 12, 2024
890e8b6
fix useBrokenLinks docs examples
slorber Jan 12, 2024
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
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
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