chore(v2): Fix linter warnings#4442
chore(v2): Fix linter warnings#4442slorber merged 3 commits intofacebook:masterfrom SamChou19815:fix-linter-warnings
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
slorber
left a comment
There was a problem hiding this comment.
thanks, almost good
just wondering if it's not better to use TS inference for some return types?
Particularly on hooks that are not public API, I find declaring full fn signature type creates a lot of boilerplate
| ): { | ||
| 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
| 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
| // 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 || ''); |
| // 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
|
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