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: add eslint plugin no-html-links #8156

Merged
merged 17 commits into from
Dec 14, 2022
Merged
1 change: 1 addition & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -374,6 +374,7 @@ module.exports = {
// locals must be justified with a disable comment.
'@typescript-eslint/no-unused-vars': [ERROR, {ignoreRestSiblings: true}],
'@typescript-eslint/prefer-optional-chain': ERROR,
'@docusaurus/no-html-links': ERROR,
'@docusaurus/no-untranslated-text': [
WARNING,
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,22 +8,19 @@
import React from 'react';
import Translate from '@docusaurus/Translate';
import {ThemeClassNames} from '@docusaurus/theme-common';
import Link from '@docusaurus/Link';
import IconEdit from '@theme/Icon/Edit';
import type {Props} from '@theme/EditThisPage';

export default function EditThisPage({editUrl}: Props): JSX.Element {
return (
<a
href={editUrl}
target="_blank"
rel="noreferrer noopener"
className={ThemeClassNames.common.editThisPage}>
<Link to={editUrl} className={ThemeClassNames.common.editThisPage}>
<IconEdit />
<Translate
id="theme.common.editThisPage"
description="The link label to edit the current page">
Edit this page
</Translate>
</a>
</Link>
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import React from 'react';
import clsx from 'clsx';
import {translate} from '@docusaurus/Translate';
import {useThemeConfig} from '@docusaurus/theme-common';
import Link from '@docusaurus/Link';
import type {Props} from '@theme/Heading';

import styles from './styles.module.css';
Expand All @@ -33,16 +34,16 @@ export default function Heading({as: As, id, ...props}: Props): JSX.Element {
)}
id={id}>
{props.children}
<a
<Link
className="hash-link"
href={`#${id}`}
to={`#${id}`}
title={translate({
id: 'theme.common.headingLinkTitle',
message: 'Direct link to heading',
description: 'Title for link to heading',
})}>
&#8203;
</a>
</Link>
</As>
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import React from 'react';
import Translate, {translate} from '@docusaurus/Translate';
import {useSkipToContent} from '@docusaurus/theme-common/internal';

import Link from '@docusaurus/Link';
import styles from './styles.module.css';

export default function SkipToContent(): JSX.Element {
Expand All @@ -19,13 +20,13 @@ export default function SkipToContent(): JSX.Element {
role="region"
aria-label={translate({id: 'theme.common.skipToMainContent'})}>
{/* eslint-disable-next-line jsx-a11y/anchor-is-valid */}
<a href="#" className={styles.skipToContent} onClick={handleSkip}>
<Link to="#" className={styles.skipToContent} onClick={handleSkip}>
<Translate
id="theme.common.skipToMainContent"
description="The skip to content label used for accessibility, allowing to rapidly navigate to main content with keyboard tab/enter navigation">
Skip to main content
</Translate>
</a>
</Link>
</div>
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*/

import React from 'react';
import Link from '@docusaurus/Link';
import type {Props} from '@theme/TOCItems/Tree';

// Recursive component rendering the toc tree
Expand All @@ -22,12 +23,10 @@ function TOCItemTree({
<ul className={isChild ? undefined : className}>
{toc.map((heading) => (
<li key={heading.id}>
{/* eslint-disable-next-line jsx-a11y/control-has-associated-label */}
<a
href={`#${heading.id}`}
<Link
to={`#${heading.id}`}
Copy link
Collaborator

Choose a reason for hiding this comment

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

wonder if we could also allow <a> for hardcoded hash links?

(not all links can be evaluated but hardcoded ones could? Maybe eslint can know it starts with a hash? 🤷‍♂️)

Copy link
Collaborator

Choose a reason for hiding this comment

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

It's not undoable. Even for template literals, you just need to check quasis[0][0] === "#".

Copy link
Contributor Author

@JohnVicke JohnVicke Oct 5, 2022

Choose a reason for hiding this comment

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

Good point!

Lets say that we add this to the plugin, should it still warn (encourage) the user to use <Link> instead of the a tag?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I guess we can just ignore hashes. This is a bonus, we can merge this PR without this feature if complicated to implement.

className={linkClassName ?? undefined}
// Developer provided the HTML, so assume it's safe.
// eslint-disable-next-line react/no-danger
dangerouslySetInnerHTML={{__html: heading.value}}
/>
<TOCItemTree
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -426,10 +426,8 @@ function SearchPageContent(): JSX.Element {
'text--right',
styles.searchLogoColumn,
)}>
<a
target="_blank"
rel="noopener noreferrer"
href="https://www.algolia.com/"
<Link
to="https://www.algolia.com/"
aria-label={translate({
id: 'theme.SearchPage.algoliaLabel',
message: 'Search by Algolia',
Expand All @@ -451,7 +449,7 @@ function SearchPageContent(): JSX.Element {
/>
</g>
</svg>
</a>
</Link>
</div>
</div>

Expand Down
2 changes: 1 addition & 1 deletion packages/docusaurus/src/client/exports/Link.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ function Link(
}

return isRegularHtmlLink ? (
// eslint-disable-next-line jsx-a11y/anchor-has-content
// eslint-disable-next-line jsx-a11y/anchor-has-content, @docusaurus/no-html-links
<a
ref={innerRef}
href={targetLink}
Expand Down
2 changes: 2 additions & 0 deletions packages/eslint-plugin/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,14 @@ export = {
recommended: {
rules: {
'@docusaurus/string-literal-i18n-messages': 'error',
'@docusaurus/no-html-links': 'warn',
},
},
all: {
rules: {
'@docusaurus/string-literal-i18n-messages': 'error',
'@docusaurus/no-untranslated-text': 'warn',
'@docusaurus/no-html-links': 'warn',
},
},
},
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
/**
* 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 rule from '../no-html-links';
import {RuleTester} from './testUtils';

const errorsJSX = [{messageId: 'link'}] as const;

const ruleTester = new RuleTester({
parser: '@typescript-eslint/parser',
parserOptions: {
ecmaFeatures: {
jsx: true,
},
},
});

ruleTester.run('prefer-docusaurus-link', rule, {
valid: [
{
code: '<Link to="/test">test</Link>',
},
{
code: '<Link to="https://twitter.com/docusaurus">Twitter</Link>',
},
],
invalid: [
{
code: '<a href="/test">test</a>',
errors: errorsJSX,
},
{
code: '<a href="https://twitter.com/docusaurus" target="_blank">test</a>',
errors: errorsJSX,
},
{
code: '<a href="https://twitter.com/docusaurus" target="_blank" rel="noopener noreferrer">test</a>',
errors: errorsJSX,
},
],
});
2 changes: 2 additions & 0 deletions packages/eslint-plugin/src/rules/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,12 @@
* LICENSE file in the root directory of this source tree.
*/

import noHtmlLinks from './no-html-links';
import noUntranslatedText from './no-untranslated-text';
import stringLiteralI18nMessages from './string-literal-i18n-messages';

export default {
'no-untranslated-text': noUntranslatedText,
'string-literal-i18n-messages': stringLiteralI18nMessages,
'no-html-links': noHtmlLinks,
};
48 changes: 48 additions & 0 deletions packages/eslint-plugin/src/rules/no-html-links.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
/**
* 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 {createRule} from '../util';
import type {TSESTree} from '@typescript-eslint/types/dist/ts-estree';

const docsUrl = 'https://docusaurus.io/docs/docusaurus-core#link';

type Options = [];

type MessageIds = 'link';
slorber marked this conversation as resolved.
Show resolved Hide resolved

export default createRule<Options, MessageIds>({
slorber marked this conversation as resolved.
Show resolved Hide resolved
name: 'no-html-links',
meta: {
type: 'problem',
docs: {
description: 'enforce using Docusaurus Link component instead of <a> tag',
recommended: false,
},
schema: [
{
type: 'object',
additionalProperties: false,
},
],
messages: {
link: `Do not use an \`<a>\` element to navigate. Use \`<Link />\` from \`@docusaurus/Link\` instead. See: ${docsUrl}`,
},
},
defaultOptions: [],

create(context) {
return {
JSXOpeningElement(node) {
if ((node.name as TSESTree.JSXIdentifier).name !== 'a') {
return;
}

context.report({node, messageId: 'link'});
},
};
},
});
21 changes: 5 additions & 16 deletions website/_dogfooding/_pages tests/hydration-tests.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,28 +6,17 @@
*/

import React from 'react';
import Link from '@docusaurus/Link';
import Layout from '@theme/Layout';

// Repro for hydration issue https://github.com/facebook/docusaurus/issues/5617
function BuggyText() {
return (
<span>
Built using the{' '}
<a href="https://www.electronjs.org/" target="_blank" rel="noreferrer">
Electron
</a>{' '}
, based on{' '}
<a href="https://www.chromium.org/" target="_blank" rel="noreferrer">
Chromium
</a>
, and written using{' '}
<a
href="https://www.typescriptlang.org/"
target="_blank"
rel="noreferrer">
TypeScript
</a>{' '}
, Xplorer promises you an unprecedented experience.
Built using the <Link to="https://www.electronjs.org/">Electron</Link> ,
based on <Link to="https://www.chromium.org/">Chromium</Link>, and written
using <Link to="https://www.typescriptlang.org/">TypeScript</Link> ,
Xplorer promises you an unprecedented experience.
</span>
);
}
Expand Down
1 change: 1 addition & 0 deletions website/docs/api/misc/eslint-plugin/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ Each config contains a set of rules. For more fine-grained control, you can also
| --- | --- | --- |
| [`@docusaurus/no-untranslated-text`](./no-untranslated-text.md) | Enforce text labels in JSX to be wrapped by translate calls | |
| [`@docusaurus/string-literal-i18n-messages`](./string-literal-i18n-messages.md) | Enforce translate APIs to be called on plain text labels | ✅ |
| [`@docusaurus/no-html-links`](./no-html-links.md) | Ensures @docusaurus/Link is used instead of a tags | ✅ |
JohnVicke marked this conversation as resolved.
Show resolved Hide resolved

✅ = recommended

Expand Down
24 changes: 24 additions & 0 deletions website/docs/api/misc/eslint-plugin/no-html-links.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
---
slug: /api/misc/@docusaurus/eslint-plugin/no-html-links
---

# no-html-links

Ensure that docusaurus `<Link>` is used instead of `<a>` tags.
slorber marked this conversation as resolved.
Show resolved Hide resolved

## Rule Details {#details}

Examples of **incorrect** code for this rule:

```html
<a href="/page">go to page!</a>
```

Examples of **correct** code for this rule:

```js
//
import Link from '@docusaurus/Link';

<Link to="/page">go to page!</Link>;
```
slorber marked this conversation as resolved.
Show resolved Hide resolved
9 changes: 4 additions & 5 deletions website/src/components/HackerNewsIcon.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,16 @@
*/

import React from 'react';
import Link from '@docusaurus/Link';

export default function HackerNewsIcon({
size = 54,
}: {
size?: number;
}): JSX.Element {
return (
<a
href="https://news.ycombinator.com/item?id=32303052"
target="_blank"
rel="noreferrer"
<Link
to="https://news.ycombinator.com/item?id=32303052"
style={{display: 'block', width: size, height: size}}>
<svg
xmlns="http://www.w3.org/2000/svg"
Expand All @@ -30,6 +29,6 @@ export default function HackerNewsIcon({
d="M23 32h2v-6l5.5-10h-2.1L24 24.1 19.6 16h-2.1L23 26z"
/>
</svg>
</a>
</Link>
);
}
9 changes: 4 additions & 5 deletions website/src/components/ProductHuntCard.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

import type {ComponentProps} from 'react';
import React from 'react';
import Link from '@docusaurus/Link';

export default function ProductHuntCard({
className,
Expand All @@ -16,10 +17,8 @@ export default function ProductHuntCard({
style?: ComponentProps<'a'>['style'];
}): JSX.Element {
return (
<a
href="https://www.producthunt.com/posts/docusaurus-2-0?utm_source=badge-featured&utm_medium=badge&utm_souce=badge-docusaurus-2-0"
target="_blank"
rel="noreferrer"
<Link
to="https://www.producthunt.com/posts/docusaurus-2-0?utm_source=badge-featured&utm_medium=badge&utm_souce=badge-docusaurus-2-0"
className={className}
style={{display: 'block', width: 250, height: 54, ...style}}>
<img
Expand All @@ -29,6 +28,6 @@ export default function ProductHuntCard({
width={250}
height={54}
/>
</a>
</Link>
);
}
Loading