style(v2): reduce number of ESLint warnings #4993

Josh-Cena · 2021-06-17T09:12:51Z

Motivation

When I'm working on the project I was constantly overwhelmed (exaggeration here; there aren't so many) by ESLint warnings. Although they aren't serious issues, I decide it's better that we address these issues, since this is what we have the linter for.

There are some warnings that I probably need help with, especially regarding any types. (I realize how intractable typing objects can become.) I don't want to easily resort to eslint-disable comments.

Have you read the Contributing Guidelines on pull requests?

Yes

Test Plan

These are mostly style changes that don't really impact functionality to any degree.

Signed-off-by: Josh-Cena <sidachen2003@gmail.com>

netlify · 2021-06-17T09:17:42Z

✔️ [V2]
Built without sensitive environment variables

🔨 Explore the source changes: 269c971

🔍 Inspect the deploy log: https://app.netlify.com/sites/docusaurus-2/deploys/60d490fd4a60f80007cc98af

😎 Browse the preview: https://deploy-preview-4993--docusaurus-2.netlify.app

github-actions · 2021-06-17T09:17:57Z

⚡️ Lighthouse report for the changes in this PR:

Category	Score
🟠 Performance	82
🟢 Accessibility	96
🟢 Best practices	100
🟢 SEO	100
🟢 PWA	95

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

Signed-off-by: Josh-Cena <sidachen2003@gmail.com>

packages/docusaurus-utils/src/markdownParser.ts

slorber

thanks, not agreeing with everything but overwall happy to reduce eslint warnings

I personally like TS function return type inference but it seems to be a best practice to type return types explicitly so why not

packages/docusaurus-utils/src/markdownParser.ts

packages/docusaurus-plugin-content-blog/src/blogUtils.ts

slorber · 2021-06-18T10:35:29Z

packages/docusaurus-plugin-content-docs/src/sidebars.ts

@@ -127,7 +127,10 @@ function assertIsCategory(
 );
 }
 // "collapsed" is an optional property
- if (item.hasOwnProperty('collapsed') && typeof item.collapsed !== 'boolean') {
+ if (
+ Object.prototype.hasOwnProperty.call(item, 'collapsed') &&


too much ceremony, I'd just replace by typeof item.collapsed !== "undefined" ?

There is actually a consideration about collapsed existing on the prototype chain of item. Is it safe to assume here that typeof item.collapsed !== "undefined" works the same?

yes it looks to me as the sidebar is a regular object

can you replace with typeof item.collapsed !== "undefined" ? it has not been done here

packages/docusaurus-plugin-content-docs/src/versions.ts

packages/docusaurus-theme-bootstrap/src/theme/ThemedImage/index.tsx

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

Co-authored-by: Sébastien Lorber <slorber@users.noreply.github.com>

…x.tsx Co-authored-by: Sébastien Lorber <slorber@users.noreply.github.com>

Josh-Cena · 2021-06-18T10:44:39Z

@slorber If I'm delving into typing (the any types), this PR is likely to shift its focus away from some simple style improvements. Should I do it here or should I open a new PR?

Signed-off-by: Josh-Cena <sidachen2003@gmail.com>

slorber · 2021-06-18T13:08:23Z

@slorber If I'm delving into typing (the any types), this PR is likely to shift its focus away from some simple style improvements. Should I do it here or should I open a new PR?

smaller PRs are always easier to review/merge, so splitting the work is not a bad idea.

Signed-off-by: Josh-Cena <sidachen2003@gmail.com>

Josh-Cena · 2021-06-18T14:53:49Z

@slorber I think I'm mostly done—reduced warning count by >50. The rest of them (33) contain:

iterators/generators require regenerator-runtime, which is too heavyweight for this guide to allow them. Separately, loops should be avoided in favor of array iterations

docusaurus/packages/docusaurus-plugin-content-docs/src/sidebarItemsGenerator.ts

Lines 270 to 274 in 7dc9fe8

 // async process made sequential on purpose! order matters 

 for (const doc of docs) { 

 // eslint-disable-next-line no-await-in-loop 

 await handleDocItem(doc); 

 }

Need some help here... Airbnb is complaining about for...of loops being too heavy. Would this be transformed to a reduce loop like this? I'm not so confident with async functions.

await docs.reduce(
  (p, doc) => p.then(() => handleDocItem(doc)),
  Promise.resolve(),
);

Visible, non-interactive elements with click handlers must have at least one keyboard listener

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

Lines 56 to 60 in 7dc9fe8

 <div 

 className="react-toggle-track" 

 role="button" 

 tabIndex={-1} 

 onClick={() => inputRef.current?.click()}>

I see that we are planning to add hotkey binding maps. This warning will remind us of this feature :P

Lots of Unexpected any. Specify a different type errors. Need substantial typing improvements

Signed-off-by: Josh-Cena <sidachen2003@gmail.com>

slorber · 2021-06-24T13:38:30Z

Need some help here... Airbnb is complaining about for...of loops being too heavy. Would this be transformed to a reduce loop like this? I'm not so confident with async functions.

Honestly don't like Airbnb preset, it's way too opinionated with weird rules, but it's the current docu preset so let's just disable what is annoying. This rule can also be disabled, it's not really applicable for NodeJS code IMHO.

I'll merge this PR as is, any further improvements are welcome

slorber

finally, asking for a few changes 🤪

slorber · 2021-06-24T13:42:58Z

packages/docusaurus-plugin-content-docs/src/sidebarItemsGenerator.ts

- // eslint-disable-next-line no-await-in-loop
- await handleDocItem(doc);
- }
+ await docs.reduce(


actually I'd prefer to revert this change

Although I reverted this, I'm pretty confident that the new code works correctly😂 Just that it is less readable

yes definitively works with reduce but much more complicated :)

slorber · 2021-06-24T13:42:58Z

packages/docusaurus-plugin-content-docs/src/sidebars.ts

@@ -127,7 +127,10 @@ function assertIsCategory(
 );
 }
 // "collapsed" is an optional property
- if (item.hasOwnProperty('collapsed') && typeof item.collapsed !== 'boolean') {
+ if (
+ Object.prototype.hasOwnProperty.call(item, 'collapsed') &&


can you replace with typeof item.collapsed !== "undefined" ? it has not been done here

packages/docusaurus/src/server/configValidation.ts

Josh-Cena · 2021-06-24T14:00:37Z

Okay cool. Lemme address those...

This reverts commit c7525d4.

Signed-off-by: Josh-Cena <sidachen2003@gmail.com>

slorber · 2021-06-24T16:12:40Z

thanks 👍

Initial work

6d3d416

Signed-off-by: Josh-Cena <sidachen2003@gmail.com>

Josh-Cena requested review from lex111 and slorber as code owners Jun 17, 2021

facebook-github-bot added the CLA Signed label Jun 17, 2021

Josh-Cena marked this pull request as draft Jun 17, 2021

More fixes

c7c833f

Signed-off-by: Josh-Cena <sidachen2003@gmail.com>

Josh-Cena reviewed Jun 17, 2021

View changes

packages/docusaurus-utils/src/markdownParser.ts Show resolved Hide resolved

slorber requested changes Jun 18, 2021

View changes

Josh-Cena and others added 2 commits Jun 18, 2021

Update packages/docusaurus-theme-classic/src/theme/ThemedImage/index.tsx

9ff6f92

Co-authored-by: Sébastien Lorber <slorber@users.noreply.github.com>

Update packages/docusaurus-theme-bootstrap/src/theme/ThemedImage/inde…

4c17e4c

…x.tsx Co-authored-by: Sébastien Lorber <slorber@users.noreply.github.com>

Josh-Cena added 2 commits Jun 18, 2021

Fix

90196bb

Signed-off-by: Josh-Cena <sidachen2003@gmail.com>

Replace versionPathPart with function

2dc74dd

Signed-off-by: Josh-Cena <sidachen2003@gmail.com>

Josh-Cena added 4 commits Jun 18, 2021

Prefer non-null assertions

e7e9fa7

Signed-off-by: Josh-Cena <sidachen2003@gmail.com>

Substitute for-of with forEach

8a4c6ac

Signed-off-by: Josh-Cena <sidachen2003@gmail.com>

Fill catch block with placeholder comment

6cc7655

Signed-off-by: Josh-Cena <sidachen2003@gmail.com>

Ignore local require

c1beedc

Signed-off-by: Josh-Cena <sidachen2003@gmail.com>

Josh-Cena added 3 commits Jun 20, 2021

Revert global require change

03e2174

Signed-off-by: Josh-Cena <sidachen2003@gmail.com>

Tighten eslint disable range

a36c287

Signed-off-by: Josh-Cena <sidachen2003@gmail.com>

Make eslint ignore examples and more tolerating to templates

1e8b06d

Signed-off-by: Josh-Cena <sidachen2003@gmail.com>

Josh-Cena force-pushed the Josh-Cena:eslint-warn branch from 2f7ae4a to 1e8b06d Jun 21, 2021

Use reduce to handle doc items sequentially

c7525d4

Signed-off-by: Josh-Cena <sidachen2003@gmail.com>

Josh-Cena marked this pull request as ready for review Jun 21, 2021

Josh-Cena requested a review from slorber Jun 21, 2021

slorber requested changes Jun 24, 2021

View changes

Josh-Cena added 2 commits Jun 24, 2021

Revert "Use reduce to handle doc items sequentially"

4f38010

This reverts commit c7525d4.

Address change requests

269c971

Signed-off-by: Josh-Cena <sidachen2003@gmail.com>

slorber added the tag: polish label Jun 24, 2021

Josh-Cena deleted the Josh-Cena:eslint-warn branch Jun 24, 2021

facebook / docusaurus

style(v2): reduce number of ESLint warnings #4993

style(v2): reduce number of ESLint warnings #4993

Josh-Cena commented Jun 17, 2021 •

edited

netlify bot commented Jun 17, 2021 •

edited

github-actions bot commented Jun 17, 2021 •

edited

slorber left a comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Josh-Cena commented Jun 18, 2021

slorber commented Jun 18, 2021

Josh-Cena commented Jun 18, 2021 •

edited

slorber commented Jun 24, 2021

slorber left a comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Josh-Cena commented Jun 24, 2021

slorber commented Jun 24, 2021

facebook / docusaurus

Sponsor facebook/docusaurus

style(v2): reduce number of ESLint warnings #4993

style(v2): reduce number of ESLint warnings #4993

Conversation

Josh-Cena commented Jun 17, 2021 • edited

Motivation

Have you read the Contributing Guidelines on pull requests?

Test Plan

netlify bot commented Jun 17, 2021 • edited

github-actions bot commented Jun 17, 2021 • edited

slorber left a comment

This comment has been minimized.

slorber Jun 18, 2021 Collaborator

This comment has been minimized.

Josh-Cena Jun 18, 2021 • edited Author Contributor

This comment has been minimized.

slorber Jun 18, 2021 Collaborator

This comment has been minimized.

slorber Jun 24, 2021 Collaborator

Josh-Cena commented Jun 18, 2021

slorber commented Jun 18, 2021

Josh-Cena commented Jun 18, 2021 • edited

slorber commented Jun 24, 2021

slorber left a comment

This comment has been minimized.

slorber Jun 24, 2021 Collaborator

This comment has been minimized.

Josh-Cena Jun 24, 2021 Author Contributor

This comment has been minimized.

slorber Jun 24, 2021 Collaborator

This comment has been minimized.

slorber Jun 24, 2021 Collaborator

Josh-Cena commented Jun 24, 2021

slorber commented Jun 24, 2021

Josh-Cena commented Jun 17, 2021 •

edited

netlify bot commented Jun 17, 2021 •

edited

github-actions bot commented Jun 17, 2021 •

edited

slorber Jun 18, 2021
Collaborator

Josh-Cena Jun 18, 2021 •

edited

Author Contributor

slorber Jun 18, 2021
Collaborator

slorber Jun 24, 2021
Collaborator

Josh-Cena commented Jun 18, 2021 •

edited

slorber Jun 24, 2021
Collaborator

Josh-Cena Jun 24, 2021
Author Contributor

slorber Jun 24, 2021
Collaborator

slorber Jun 24, 2021
Collaborator