feat(eslint-plugin): new prefer-docusaurus-heading rule by Devansu-Yadav · Pull Request #8384 · facebook/docusaurus

Devansu-Yadav · 2022-11-28T16:08:51Z

Pre-flight checklist

I have read the Contributing Guidelines on pull requests.
If this is a code change: I have written unit tests and/or added dogfooding pages to fully verify the new behavior.
If this is a new API or substantial change: the PR has an accompanying issue (closes #0000) and the maintainers have approved on my working plan.

Motivation

Described in #6472. Basically, this plugin enforces the use of @theme/Heading component instead of <h2> tags.

Test Plan

Added Jest unit tests for this plugin.

Test links

Deploy preview: https://deploy-preview-8384--docusaurus-2.netlify.app/

Related issues/PRs

#6472

Comments

netlify · 2022-11-28T16:13:34Z

✅ [V2]

Built without sensitive environment variables

Name	Link
🔨 Latest commit	`47c9b7a`
🔍 Latest deploy log	https://app.netlify.com/sites/docusaurus-2/deploys/63c9230407d46200089ad22a
😎 Deploy Preview	https://deploy-preview-8384--docusaurus-2.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

github-actions · 2022-11-28T16:14:05Z

⚡️ Lighthouse report for the deploy preview of this PR

URL	Performance	Accessibility	Best Practices	SEO	PWA	Report
/	🟠 67	🟢 97	🟢 92	🟢 100	🟢 90	Report
/docs/installation	🟠 67	🟢 100	🟢 92	🟢 100	🟢 90	Report

slorber · 2022-12-07T18:37:39Z

@Josh-Cena no strong opinion but what's the motivation to force users to use Heading instead of h2?

That looks like a valid thing to do to use a custom h2 (or any other level) in some cases.

Is there a reason to handle h2 in a different way than any other heading level?

Not against this rule but we probably need to think a bit deeper about what we are trying to prevent here, and also what should be the default experience.

Note @theme/Heading is a theme component, and it may technically not exist on a user site (although it's unlikely to happen in practice)

Josh-Cena · 2022-12-07T21:06:09Z

@slorber @theme/Heading has anchor links, and can help us build link graphs that are section-title-aware. It also leads to a more uniform UX in general. See for example: typescript-eslint/typescript-eslint#5951

I do think this should not only apply to h2 though, but to all hn headings.

Devansu-Yadav · 2022-12-08T13:54:59Z

@slorber @theme/Heading has anchor links, and can help us build link graphs that are section-title-aware. It also leads to a more uniform UX in general. See for example: typescript-eslint/typescript-eslint#5951

I do think this should not only apply to h2 though, but to all hn headings.

@Josh-Cena @slorber I agree that we should have this rule apply to all possible headings and in fact, I have already added an option includeAllHeadings that checks against all hn headings. Do you reckon this should be the default behaviour of this rule?

PS: Just a heads up, I'll be able to continue my work on this PR after the 19th Dec, as I'd be unavailable due to my ongoing University exams 😅

slorber · 2022-12-08T15:13:54Z

We should probably not use and special edge case for h2, and just apply this to all headings by default.

Eventually pass a list of headings to check as an option, but I really wonder who would use it 🤔

Do we want to make a special case for h1 @Josh-Cena ? In this case there's no anchor/id.

Devansu-Yadav · 2022-12-20T10:36:00Z

We should probably not use and special edge case for h2, and just apply this to all headings by default.

Eventually pass a list of headings to check as an option, but I really wonder who would use it 🤔

Do we want to make a special case for h1 @Josh-Cena ? In this case there's no anchor/id.

@Josh-Cena @slorber Any updates on how I should proceed with this? What I mean is that, do we need to have any special cases for h1 or h2 in this rule?

slorber · 2022-12-22T13:23:22Z

I think we can move on and apply the rule for all hX elements 👍

We'll add more options later if needed, once people come with a good use-case to explain why an option is needed

Devansu-Yadav · 2022-12-31T12:29:45Z

I think we can move on and apply the rule for all hX elements 👍

We'll add more options later if needed, once people come with a good use-case to explain why an option is needed

@slorber I have implemented the eslint rule without any options for now. For fixing all the eslint errors introduced due to this new rule, I have made use of Heading defined in @theme/Heading module. Is this correct or should I be directly using the Heading component?

I know the above question might be a bit naive, but from what I understand after digging through source code, all of the @theme and @docusaurus modules or aliases are linked with their corresponding React component implementations and bundled using Webpack to load them in our React apps.

slorber

Almost LGTM thanks 👍

Some changes requested in comments

@Josh-Cena let me know if you want to review this

slorber · 2023-01-04T17:52:23Z

website/docs/api/misc/eslint-plugin/README.mdx

 | [`@docusaurus/no-untranslated-text`](./no-untranslated-text.mdx) | Enforce text labels in JSX to be wrapped by translate calls |  |
 | [`@docusaurus/string-literal-i18n-messages`](./string-literal-i18n-messages.mdx) | Enforce translate APIs to be called on plain text labels | ✅ |
 | [`@docusaurus/no-html-links`](./no-html-links.mdx) | Ensures @docusaurus/Link is used instead of `<a>` tags | ✅ |
+| [`@docusaurus/prefer-docusaurus-heading`](./prefer-docusaurus-heading.mdx) | Ensures @theme/Heading is used instead of `<hn>` tags for headings | ✅ |


maybe prefer-theme-heading would be a more accurate rule name? (also maybe more confusing name?)

Any opinion @Josh-Cena ?

Yeah, I agree prefer-theme-heading sounds more accurate but doesn't docusaurus provide different themes containing the Heading component?

docusaurus core doesn't provide any heading for now, only the theme does.

But maybe we could have a core heading component later if we want to create a graph of pages + their respective headings or whatever. We can keep this name for now, it doesn't matter much 😅

website/src/pages/versions.tsx

packages/docusaurus-plugin-debug/src/theme/DebugConfig/index.tsx

.eslintrc.js

packages/docusaurus-theme-classic/src/theme/BlogArchivePage/index.tsx

packages/docusaurus-theme-classic/src/theme/TagsListByLetter/index.tsx

packages/docusaurus/src/client/theme-fallback/Error/index.tsx

Devansu-Yadav · 2023-01-19T10:54:45Z

Almost LGTM thanks 👍

Some changes requested in comments

@Josh-Cena let me know if you want to review this

@Josh-Cena @slorber Any updates from your end on the changes that I have made recently in this PR?

Signed-off-by: Devansu <devansuyadav@gmail.com>

slorber

Thanks, LGTM 👍

slorber · 2023-01-19T15:00:38Z

website/docs/api/misc/eslint-plugin/README.mdx

 | [`@docusaurus/no-untranslated-text`](./no-untranslated-text.mdx) | Enforce text labels in JSX to be wrapped by translate calls |  |
 | [`@docusaurus/string-literal-i18n-messages`](./string-literal-i18n-messages.mdx) | Enforce translate APIs to be called on plain text labels | ✅ |
 | [`@docusaurus/no-html-links`](./no-html-links.mdx) | Ensures @docusaurus/Link is used instead of `<a>` tags | ✅ |
+| [`@docusaurus/prefer-docusaurus-heading`](./prefer-docusaurus-heading.mdx) | Ensures @theme/Heading is used instead of `<hn>` tags for headings | ✅ |


docusaurus core doesn't provide any heading for now, only the theme does.

But maybe we could have a core heading component later if we want to create a graph of pages + their respective headings or whatever. We can keep this name for now, it doesn't matter much 😅

facebook-github-bot added the CLA Signed Signed Facebook CLA label Nov 28, 2022

Josh-Cena changed the title ~~Add eslint plugin prefer-docusaurus-heading~~ feat(eslint-plugin): new prefer-docusaurus-heading rule Dec 21, 2022

Josh-Cena added the pr: new feature This PR adds a new API or behavior. label Dec 21, 2022

Devansu-Yadav force-pushed the add-eslint-plugin branch from 2e06132 to bf09d47 Compare December 29, 2022 08:12

Devansu-Yadav force-pushed the add-eslint-plugin branch from e351bf9 to 039cc45 Compare December 31, 2022 14:31

Devansu-Yadav marked this pull request as ready for review December 31, 2022 15:47

Devansu-Yadav requested review from Josh-Cena, lex111 and slorber as code owners December 31, 2022 15:47

slorber requested changes Jan 4, 2023

View reviewed changes

Devansu-Yadav force-pushed the add-eslint-plugin branch from 9fd48c5 to c01304d Compare January 5, 2023 12:48

Devansu-Yadav requested review from slorber and removed request for Josh-Cena and lex111 January 8, 2023 12:02

Devansu-Yadav added 6 commits January 19, 2023 16:30

feat: add eslint plugin prefer-docusaurus-heading

801e93b

Signed-off-by: Devansu <devansuyadav@gmail.com>

fix: eslint prefer-docusaurus-heading errors

8ed4116

Signed-off-by: Devansu <devansuyadav@gmail.com>

docs: add docs for prefer-docusaurus-heading rule

e2a3837

Signed-off-by: Devansu <devansuyadav@gmail.com>

docs: rename file for prefer-docusaurus-heading

c5f177e

Signed-off-by: Devansu <devansuyadav@gmail.com>

test: add test for prefer-docusaurus-heading rule

ed298d6

Signed-off-by: Devansu <devansuyadav@gmail.com>

fix: fix broken links in docs

5eebc3e

Signed-off-by: Devansu <devansuyadav@gmail.com>

Devansu-Yadav added 2 commits January 19, 2023 16:30

re-factor: disable prefer-docusaurus-heading

0502adc

Signed-off-by: Devansu <devansuyadav@gmail.com>

minor: add relevant ids to Heading

47c9b7a

Signed-off-by: Devansu <devansuyadav@gmail.com>

Devansu-Yadav force-pushed the add-eslint-plugin branch from b41c634 to 47c9b7a Compare January 19, 2023 11:01

slorber approved these changes Jan 19, 2023

View reviewed changes

slorber added the to backport This PR is planned to be backported to a stable version of Docusaurus label Jan 19, 2023

slorber merged commit 90e7e32 into facebook:main Jan 19, 2023

slorber removed the to backport This PR is planned to be backported to a stable version of Docusaurus label Jan 26, 2023

This was referenced Oct 19, 2023

chore: v3.0.0-rc.0 release #9418

Merged

chore: v3.0.0-rc.1 release #9453

Merged

Abuchtela mentioned this pull request May 20, 2024

[Snyk] Upgrade @docusaurus/preset-classic from 3.0.0 to 3.2.1 Abuchtela/aptos-core#42

Closed

Uh oh!

Conversation

Devansu-Yadav commented Nov 28, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-flight checklist

Motivation

Test Plan

Test links

Related issues/PRs

Comments

Uh oh!

netlify bot commented Nov 28, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ [V2]

Uh oh!

github-actions bot commented Nov 28, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⚡️ Lighthouse report for the deploy preview of this PR

Uh oh!

slorber commented Dec 7, 2022

Uh oh!

Josh-Cena commented Dec 7, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Devansu-Yadav commented Dec 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

slorber commented Dec 8, 2022

Uh oh!

Devansu-Yadav commented Dec 20, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

slorber commented Dec 22, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Devansu-Yadav commented Dec 31, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

slorber left a comment

Choose a reason for hiding this comment

Uh oh!

slorber Jan 4, 2023

Choose a reason for hiding this comment

Uh oh!

Devansu-Yadav Jan 5, 2023

Choose a reason for hiding this comment

Uh oh!

slorber Jan 19, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Devansu-Yadav commented Jan 19, 2023

Uh oh!

slorber left a comment

Choose a reason for hiding this comment

Uh oh!

slorber Jan 19, 2023

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Devansu-Yadav commented Nov 28, 2022 •

edited

Loading

netlify bot commented Nov 28, 2022 •

edited

Loading

github-actions bot commented Nov 28, 2022 •

edited

Loading

Josh-Cena commented Dec 7, 2022 •

edited

Loading

Devansu-Yadav commented Dec 8, 2022 •

edited

Loading

Devansu-Yadav commented Dec 20, 2022 •

edited

Loading

slorber commented Dec 22, 2022 •

edited

Loading

Devansu-Yadav commented Dec 31, 2022 •

edited

Loading