chore(v2): Fix linter warnings #4442
Conversation
|
[V1] Deploy preview failure Built without sensitive environment variables with commit 97513ba https://app.netlify.com/sites/docusaurus-1/deploys/6052286fec1fc50008eff9e5 |
|
Deploy preview for docusaurus-2 ready! Built without sensitive environment variables with commit 97513ba |
|
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-4442--docusaurus-2.netlify.app/classic/ |
223 warnings to 145 warnings
| ): { | ||
| readonly preferredVersion: any; | ||
| readonly savePreferredVersionName: (versionName: string) => void; | ||
| } { |
There was a problem hiding this comment.
do we really want to have explicit returns everywhere?
I find this a bit annoying personally and like to leverage inference in some cases
| @@ -32,11 +34,39 @@ export type Navbar = { | |||
| logo?: NavbarLogo; | |||
| }; | |||
|
|
|||
| export type ColorModeConfig = { | |||
There was a problem hiding this comment.
no worries, i have to redo this pr anyway, and i'm going to split it to smaller pieces
| @@ -113,22 +116,22 @@ function Link({ | |||
| if (IOSupported && ref && isInternal) { | |||
| // If IO supported and element reference found, setup Observer functionality. | |||
| handleIntersection(ref, () => { | |||
| window.docusaurus.prefetch(targetLink); | |||
| window.docusaurus.prefetch(targetLink || ''); | |||
There was a problem hiding this comment.
Should we prefetch anything if there is no targetLink ?
| }); | ||
| } | ||
| }; | ||
|
|
||
| const onMouseEnter = () => { | ||
| if (!preloaded.current) { | ||
| window.docusaurus.preload(targetLink); | ||
| window.docusaurus.preload(targetLink || ''); |
| preloaded.current = true; | ||
| } | ||
| }; | ||
|
|
||
| useEffect(() => { | ||
| // If IO is not supported. We prefetch by default (only once). | ||
| if (!IOSupported && isInternal) { | ||
| window.docusaurus.prefetch(targetLink); | ||
| window.docusaurus.prefetch(targetLink || ''); |
| export function usePluralForm() { | ||
| export function usePluralForm(): { | ||
| selectMessage: (count: number, pluralMessages: string) => string; | ||
| } { |
There was a problem hiding this comment.
also prefer inference here
| locale: string; | ||
| fullyQualified: boolean; | ||
| }) => string; | ||
| } { |
There was a problem hiding this comment.
😅 I'd prefer just using inference here
slorber
added
the
pr: polish
|
thanks :) |
Motivation
Help reduce number of linter warnings. .
223 warnings to 148 warnings.
Have you read the Contributing Guidelines on pull requests?
Yes
Test Plan
I avoided changes that can potentially cause runtime behavior changes. All the changes here are almost all type-only. Since it passes the type checker and linter, it shouldn't affect any runtime behavior.
Related PRs
N/A