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
Show file tree
Hide file tree
Changes from 8 commits
Commits
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
1 change: 1 addition & 0 deletions .eslintrc.js
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
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>
);
}
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>
);
}
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>
);
}
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
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
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
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
@@ -0,0 +1,70 @@
/**
* 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>',
},
{
code: '<a href="https://twitter.com/docusaurus">Twitter</a>',
options: [{ignoreFullyResolved: true}],
},
{
code: '<a href="mailto:viktor@malmedal.dev">Contact</a> ',
options: [{ignoreFullyResolved: true}],
},
{
code: '<a href="tel:123456789">Call</a>',
options: [{ignoreFullyResolved: true}],
},
],
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,
},
{
code: '<a href="mailto:viktor@malmedal.dev">Contact</a> ',
errors: errorsJSX,
},
{
code: '<a href="tel:123456789">Call</a>',
errors: errorsJSX,
},
{
code: '<a href="www.twitter.com/docusaurus">Twitter</a>',
options: [{ignoreFullyResolved: true}],
errors: errorsJSX,
},
],
});
2 changes: 2 additions & 0 deletions packages/eslint-plugin/src/rules/index.ts
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,
};
79 changes: 79 additions & 0 deletions packages/eslint-plugin/src/rules/no-html-links.ts
@@ -0,0 +1,79 @@
/**
* 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 = [
{
ignoreFullyResolved: boolean;
},
];

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',
properties: {
ignoreFullyResolved: {
type: 'boolean',
},
},
additionalProperties: false,
},
],
messages: {
link: `Do not use an \`<a>\` element to navigate. Use \`<Link />\` from \`@docusaurus/Link\` instead. See: ${docsUrl}`,
},
},
defaultOptions: [
{
ignoreFullyResolved: false,
},
],

create(context, [options]) {
const {ignoreFullyResolved} = options;

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

if (ignoreFullyResolved) {
const hrefAttr = node.attributes.find(
(attr) => (attr as TSESTree.JSXAttribute).name.name === 'href',
) as TSESTree.JSXAttribute | undefined;

if (hrefAttr?.value && hrefAttr.value.type === 'Literal') {
const href = hrefAttr.value.value as string;
try {
const url = new URL(href);
if (url.protocol) {
return;
}
} catch (e) {}
}
}

context.report({node, messageId: 'link'});
},
};
},
});
21 changes: 5 additions & 16 deletions website/_dogfooding/_pages tests/hydration-tests.tsx
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
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 | ✅ |

✅ = recommended

Expand Down