feat: Docusaurus ESLint plugin to enforce best Docusaurus practices #7206

elias-pap · 2022-04-18T22:57:09Z

Motivation

Explained in the description of #6472

Have you read the Contributing Guidelines on pull requests?

Yes

Test Plan

Doc URL: https://deploy-preview-7206--docusaurus-2.netlify.app/docs/api/misc/@docusaurus/eslint-plugin/

Unit tests are included and integrated with existing jest setup.

Output of `yarn test eslint-plugin`

yarn run v1.22.17
$ jest eslint-plugin
 PASS  packages/eslint-plugin/__tests__/lib/rules/no-dynamic-i18n-messages.test.js
  no-dynamic-i18n-messages
    valid
      ✓ <Translate>text</Translate> (20 ms)
      ✓ <Translate> text </Translate> (1 ms)
      ✓ <Translate>"text"</Translate> (2 ms)
      ✓ <Translate>'text'</Translate> (1 ms)
      ✓ <Translate>`text`</Translate> (1 ms)
      ✓ <Translate>{"text"}</Translate> (2 ms)
      ✓ <Translate>{'text'}</Translate> (1 ms)
      ✓ <Translate>{`text`}</Translate> (1 ms)
      ✓ <Component>{text}</Component> (2 ms)
      ✓ <Component> {text} </Component> (1 ms)
      ✓ <Translate
                id="homepage.title"
                description="The homepage welcome message">
                  Welcome to my website
              </Translate> (2 ms)
      ✓ <Translate
                values={{firstName: 'Sébastien'}}>
                  {'Welcome, {firstName}! How are you?'}
              </Translate> (5 ms)
      ✓ translate({message: 'My page meta title'}) (1 ms)
      ✓ translate({message: 'The logo of site {siteName}'}, {siteName: 'Docusaurus'}) (1 ms)
      ✓ translate({otherProp: metaTitle}) (1 ms)
      ✓ translate({otherProp: `My page meta title`}) (1 ms)
    invalid
      ✓ <Translate>{text}</Translate> (2 ms)
      ✓ <Translate> {text} </Translate> (1 ms)
      ✓ <Translate>`{text}`</Translate> (2 ms)
      ✓ <Translate>{`${text}`}</Translate> (1 ms)
      ✓ translate({message: metaTitle}) (1 ms)
      ✓ translate({message: `My page meta title`}) (1 ms)

 PASS  packages/eslint-plugin/__tests__/lib/rules/no-untranslated-text.test.js
  no-untranslated-text
    valid
      ✓ <Translate>text</Translate> (9 ms)
      ✓ <Translate> text </Translate> (1 ms)
      ✓ <Translate>"text"</Translate> (1 ms)
      ✓ <Translate>'text'</Translate> (1 ms)
      ✓ <Translate>`text`</Translate> (1 ms)
      ✓ <Translate>{"text"}</Translate> (1 ms)
      ✓ <Translate>{'text'}</Translate> (1 ms)
      ✓ <Translate>{`text`}</Translate> (1 ms)
      ✓ <Component>{text}</Component> (1 ms)
      ✓ <Component> {text} </Component> (1 ms)
    invalid
      ✓ <Component>text</Component> (1 ms)
      ✓ <Component> text </Component> (2 ms)
      ✓ <Component>"text"</Component>
      ✓ <Component>'text'</Component>
      ✓ <Component>`text`</Component> (1 ms)
      ✓ <Component>{"text"}</Component> (1 ms)
      ✓ <Component>{'text'}</Component> (1 ms)
      ✓ <Component>{`text`}</Component> (1 ms)
      ✓ <>text</> (1 ms)

Test Suites: 2 passed, 2 total
Tests:       41 passed, 41 total
Snapshots:   0 total
Time:        0.512 s, estimated 1 s
Ran all test suites matching /eslint-plugin/i.
Done in 1.25s.

This is also dogfooded in our own monorepo.

netlify · 2022-04-18T23:03:48Z

✅ [V2]

Built without sensitive environment variables

Name	Link
🔨 Latest commit	`40cfd23`
🔍 Latest deploy log	https://app.netlify.com/sites/docusaurus-2/deploys/626c043637a15500083fb170
😎 Deploy Preview	https://deploy-preview-7206--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-04-18T23:04:08Z

⚡️ Lighthouse report for the changes in this PR:

Category	Score
🟠 Performance	67
🟢 Accessibility	100
🟠 Best practices	83
🟢 SEO	100
🟢 PWA	90

Lighthouse ran on https://deploy-preview-7206--docusaurus-2.netlify.app/

Josh-Cena · 2022-04-18T23:48:37Z

At a glance, it looks much, much better than I could've imagined 👍 Thanks a bunch! I'll play with it later.

Josh-Cena · 2022-04-19T02:16:43Z

packages/eslint-plugin/lib/rules/no-dynamic-i18n-messages.js

+      node.children.every((child) => isTextLabelChild({child}));
+
+    return {
+      "JSXElement[openingElement.name.name='Translate']": (node) => {


I think this is fine for now, but in the long term, we may need to first check the imported name because it may not be imported as Translate (cf our translationExtractor implementation). I'm not aware if we can let ESLint do a two-round traverse in an efficient way—I think people usually use Program:exit?

Josh-Cena

This looks really solid and it's already ready to be shipped IMO.

There are many useful extensions to make, but I think this will make a great initial kickstart.

packages/eslint-plugin/__tests__/lib/rules/no-dynamic-i18n-messages.test.js

Josh-Cena · 2022-04-19T02:30:14Z

packages/eslint-plugin/lib/rules/no-dynamic-i18n-messages.js

+    const isMessageTypeValid = (type) => type === 'Literal';
+
+    const isNodeValid = (node) =>
+      node.children.every((child) => isTextLabelChild({child}));


It would be useful to add an option ignoreJSXAttributes. We may want to flag <div aria-label="untranslated" /> as well. Doesn't have to be in this PR though

I didn't understand that, could you provide an example usage of ignoreJSXAttributes ? Also what is the purpose of <div aria-label="untranslated" /> ? Self-closing tags do not have children (excluding the children prop) therefore the current rules do not apply in this case, right ?

Oh, sorry, wrong comment place. I was talking about no-untranslated-text. There should be an option that turns on warning for JSX attributes like aria-label as well, not just string literal children.

packages/eslint-plugin/docs/rules/no-dynamic-i18n-messages.md

packages/eslint-plugin/lib/rules/no-untranslated-text.js

Josh-Cena · 2022-04-19T02:53:43Z

Seems to work pretty nicely 🎉

(Although 99% of them are false-positives, not something we can help, lol)

packages/docusaurus/src/client/exports/__tests__/Translate.test.tsx

…terals

…n-messages

packages/eslint-plugin/lib/rules/string-literal-i18n-messages.js

packages/eslint-plugin/package.json

website/docs/api/plugins/eslint-plugin.md

website/docs/api/plugins/overview.md

Josh-Cena

Thanks @elias-pap I'm sorry but this would probably take a bit longer than expected to be merged, because @slorber needs to review this as well, but he has limited availability. Luckily it doesn't touch existing code and should have very little merge conflict over time, so we'll probably go back to it in June, after 2.0 has been released. Thanks again!

website/docs/api/plugin-methods/_category_.yml

website/docs/api/misc/eslint-plugin/no-untranslated-text.md

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

- Reorder sidebar entries - Fix title size - Use Markdown file paths - Simplify relative links

Josh-Cena

This is fully ready. @elias-pap There's no more feedback from my side. If @slorber manages to review this, we can merge it.

elias-pap · 2022-04-22T08:57:57Z

Great, thanks for the reviews @Josh-Cena.

slorber

Thanks, looks nice

As this is an optional plugin, not enabled by default the risk to merge this is quite low so we can give it a try 👍

packages/eslint-plugin/.eslintrc.js

website/docs/api/misc/eslint-plugin/no-untranslated-text.md

website/docs/api/misc/eslint-plugin/string-literal-i18n-messages.md

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

Josh-Cena · 2022-04-29T14:55:56Z

Hold on—there are some minor issues. I would first fix all internal warnings and merge it no later than tomorrow.

Josh-Cena · 2022-04-29T15:29:38Z

Will merge this soon. Thanks again @elias-pap much appreciated! It's caught a lot of issues in our own repo which I've just fixed.

feat: add eslint plugin

52f00e4

elias-pap requested review from slorber, lex111 and Josh-Cena as code owners April 18, 2022 22:57

facebook-github-bot added the CLA Signed Signed Facebook CLA label Apr 18, 2022

Josh-Cena reviewed Apr 19, 2022

View reviewed changes

Josh-Cena added 2 commits April 19, 2022 10:23

refactor

e6ac161

add tests

e48d6e1

Josh-Cena approved these changes Apr 19, 2022

View reviewed changes

Josh-Cena reviewed Apr 19, 2022

View reviewed changes

packages/eslint-plugin/lib/rules/no-untranslated-text.js Outdated Show resolved Hide resolved

fixups!

5375beb

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

Josh-Cena changed the title ~~feat: add eslint plugin~~ feat: Docusaurus eslint plugin to ensure best Docusaurus practices Apr 19, 2022

Josh-Cena reviewed Apr 19, 2022

View reviewed changes

packages/docusaurus/src/client/exports/__tests__/Translate.test.tsx Outdated Show resolved Hide resolved

fix(no-dynamic-i18n-messages): make translate() recognize template li…

e33bcd2

…terals

elias-pap requested a review from Josh-Cena April 19, 2022 20:30

elias-pap added 5 commits April 20, 2022 13:09

refactor: rename rule no-dynamic-i18n-messages --> string-literal-i18…

19a7f89

…n-messages

feat: add ignoreStrings option and refactor

7ae0e64

docs: migrate docs to /docs/api/plugins

aa58521

docs: fix anchor links in README.md

8776087

fix: add some ignored strings

3d5ecd0

Josh-Cena requested changes Apr 21, 2022

View reviewed changes

docs: update eslint-plugin docs

9f04ae1

elias-pap requested a review from Josh-Cena April 21, 2022 18:04

fix: update README link

05e2fed

Josh-Cena requested changes Apr 22, 2022

View reviewed changes

elias-pap and others added 2 commits April 22, 2022 11:21

docs: various updates

492615f

- Reorder sidebar entries - Fix title size - Use Markdown file paths - Simplify relative links

Merge branch 'main' into add-eslint-plugin

3fd35d6

Josh-Cena approved these changes Apr 22, 2022

View reviewed changes

slorber requested changes Apr 29, 2022

View reviewed changes

Merge branch 'main' into add-eslint-plugin

05a5f3c

Josh-Cena changed the title ~~feat: Docusaurus eslint plugin to ensure best Docusaurus practices~~ feat: Docusaurus ESLint plugin to enforce best Docusaurus practices Apr 29, 2022

Josh-Cena added 3 commits April 29, 2022 21:15

address reviews

35ef3b2

wording polish

beabcf4

add npmignore

ecdc3bd

Josh-Cena added 3 commits April 29, 2022 23:17

fix all internal warnings

d1d9add

doc improvements

50f0de4

fix test

40cfd23

Josh-Cena requested a review from slorber April 29, 2022 15:29

Josh-Cena merged commit 3b1170e into facebook:main Apr 29, 2022

elias-pap deleted the add-eslint-plugin branch April 29, 2022 17:24

Josh-Cena mentioned this pull request Apr 30, 2022

refactor(eslint-plugin): migrate to TS-ESLint infrastructure #7276

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Docusaurus ESLint plugin to enforce best Docusaurus practices #7206

feat: Docusaurus ESLint plugin to enforce best Docusaurus practices #7206

elias-pap commented Apr 18, 2022 •

edited by slorber

Loading

netlify bot commented Apr 18, 2022 •

edited

Loading

github-actions bot commented Apr 18, 2022 •

edited

Loading

Josh-Cena commented Apr 18, 2022

Josh-Cena Apr 19, 2022 •

edited

Loading

Josh-Cena left a comment

Josh-Cena Apr 19, 2022

elias-pap Apr 21, 2022

Josh-Cena Apr 21, 2022

Josh-Cena commented Apr 19, 2022

Josh-Cena left a comment

Josh-Cena left a comment

elias-pap commented Apr 22, 2022

slorber left a comment

Josh-Cena commented Apr 29, 2022

Josh-Cena commented Apr 29, 2022 •

edited

Loading

feat: Docusaurus ESLint plugin to enforce best Docusaurus practices #7206

feat: Docusaurus ESLint plugin to enforce best Docusaurus practices #7206

Conversation

elias-pap commented Apr 18, 2022 • edited by slorber Loading

Motivation

Have you read the Contributing Guidelines on pull requests?

Test Plan

netlify bot commented Apr 18, 2022 • edited Loading

✅ [V2]

github-actions bot commented Apr 18, 2022 • edited Loading

Josh-Cena commented Apr 18, 2022

Josh-Cena Apr 19, 2022 • edited Loading

Choose a reason for hiding this comment

Josh-Cena left a comment

Choose a reason for hiding this comment

Josh-Cena Apr 19, 2022

Choose a reason for hiding this comment

elias-pap Apr 21, 2022

Choose a reason for hiding this comment

Josh-Cena Apr 21, 2022

Choose a reason for hiding this comment

Josh-Cena commented Apr 19, 2022

Josh-Cena left a comment

Choose a reason for hiding this comment

Josh-Cena left a comment

Choose a reason for hiding this comment

elias-pap commented Apr 22, 2022

slorber left a comment

Choose a reason for hiding this comment

Josh-Cena commented Apr 29, 2022

Josh-Cena commented Apr 29, 2022 • edited Loading

elias-pap commented Apr 18, 2022 •

edited by slorber

Loading

netlify bot commented Apr 18, 2022 •

edited

Loading

github-actions bot commented Apr 18, 2022 •

edited

Loading

Josh-Cena Apr 19, 2022 •

edited

Loading

Josh-Cena commented Apr 29, 2022 •

edited

Loading