Skip to content

Commit

Permalink
chore: Enable ESLint rules of hooks + fix new lint errors (#5714)
Browse files Browse the repository at this point in the history
  • Loading branch information
slorber authored Oct 20, 2021
1 parent 3db4fca commit 098f210
Show file tree
Hide file tree
Showing 16 changed files with 109 additions and 47 deletions.
4 changes: 3 additions & 1 deletion .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ module.exports = {
'eslint:recommended',
'plugin:@typescript-eslint/eslint-recommended',
'plugin:@typescript-eslint/recommended',
'plugin:react-hooks/recommended',
'airbnb',
'prettier',
'prettier/react',
Expand All @@ -41,6 +42,8 @@ module.exports = {
},
plugins: ['react-hooks', 'header'],
rules: {
'react-hooks/rules-of-hooks': ERROR,
'react-hooks/exhaustive-deps': ERROR,
'class-methods-use-this': OFF, // It's a way of allowing private variables.
'func-names': OFF,
// Ignore certain webpack alias because it can't be resolved
Expand Down Expand Up @@ -77,7 +80,6 @@ module.exports = {
'react/destructuring-assignment': OFF, // Too many lines.
'react/prefer-stateless-function': WARNING,
'react/jsx-props-no-spreading': OFF,
'react-hooks/rules-of-hooks': ERROR,
'react/require-default-props': [ERROR, {ignoreFunctionalComponents: true}],
'@typescript-eslint/no-inferrable-types': OFF,
'import/first': OFF,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ function DocPageContent({
setHiddenSidebar(false);
}

setHiddenSidebarContainer(!hiddenSidebarContainer);
setHiddenSidebarContainer((value) => !value);
}, [hiddenSidebar]);

return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ function useAutoExpandActiveCategory({
if (justBecameActive && collapsed) {
setCollapsed(false);
}
}, [isActive, wasActive, collapsed]);
}, [isActive, wasActive, collapsed, setCollapsed]);
}

function DocSidebarItemCategory({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ function DropdownNavbarItemMobile({
if (containsActive) {
setCollapsed(!containsActive);
}
}, [localPathname, containsActive]);
}, [localPathname, containsActive, setCollapsed]);

return (
<li
Expand Down
4 changes: 2 additions & 2 deletions packages/docusaurus-theme-classic/src/theme/hooks/useTheme.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ const useTheme = (): useThemeReturns => {
} catch (err) {
console.error(err);
}
}, [setTheme]);
}, [disableSwitch, setTheme]);

useEffect(() => {
if (disableSwitch && !respectPrefersColorScheme) {
Expand All @@ -80,7 +80,7 @@ const useTheme = (): useThemeReturns => {
.addListener(({matches}) => {
setTheme(matches ? themes.dark : themes.light);
});
}, []);
}, [disableSwitch, respectPrefersColorScheme]);

return {
isDarkTheme: theme === themes.dark,
Expand Down
5 changes: 5 additions & 0 deletions packages/docusaurus-theme-common/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,3 +88,8 @@ export {
useScrollPosition,
useScrollPositionBlocker,
} from './utils/scrollUtils';

export {
useIsomorphicLayoutEffect,
useDynamicCallback,
} from './utils/reactUtils';
Original file line number Diff line number Diff line change
Expand Up @@ -83,14 +83,14 @@ const useAnnouncementBarContextValue = (): AnnouncementBarAPI => {
if (isNewAnnouncement || !isDismissedInStorage()) {
setClosed(false);
}
}, []);
}, [announcementBar]);

return useMemo(() => {
return {
isActive: !!announcementBar && !isClosed,
close: handleClose,
};
}, [isClosed]);
}, [announcementBar, isClosed, handleClose]);
};

const AnnouncementBarContext = createContext<AnnouncementBarAPI | null>(null);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ function useContextValue() {
return {
savePreferredVersion,
};
}, [setState]);
}, [versionPersistence]);

return [state, api] as const;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ export function useDocsPreferredVersion(
(versionName: string) => {
api.savePreferredVersion(pluginId, versionName);
},
[api],
[api, pluginId],
);

return {preferredVersion, savePreferredVersionName} as const;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ function useShallowMemoizedObject<O extends Record<string, unknown>>(obj: O) {
return useMemo(
() => obj,
// Is this safe?
// eslint-disable-next-line react-hooks/exhaustive-deps
[...Object.keys(obj), ...Object.values(obj)],
);
}
Expand Down
34 changes: 34 additions & 0 deletions packages/docusaurus-theme-common/src/utils/reactUtils.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/**
* Copyright (c) Facebook, Inc. and its affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

import {useCallback, useEffect, useLayoutEffect, useRef} from 'react';

// This hook is like useLayoutEffect, but without the SSR warning
// It seems hacky but it's used in many React libs (Redux, Formik...)
// Also mentioned here: https://github.com/facebook/react/issues/16956
// It is useful when you need to update a ref as soon as possible after a React render (before useEffect)
export const useIsomorphicLayoutEffect =
typeof window !== 'undefined' ? useLayoutEffect : useEffect;

// Permits to transform an unstable callback (like an arrow function provided as props)
// to a "stable" callback that is safe to use in a useEffect dependency array
// Useful to avoid React stale closure problems + avoid useless effect re-executions
//
// Workaround until the React team recommends a good solution, see https://github.com/facebook/react/issues/16956
// This generally works has some potential drawbacks, such as https://github.com/facebook/react/issues/16956#issuecomment-536636418
export function useDynamicCallback<T extends (...args: never[]) => unknown>(
callback: T,
): T {
const ref = useRef<T>(callback);

useIsomorphicLayoutEffect(() => {
ref.current = callback;
}, [callback]);

// @ts-expect-error: TODO, not sure how to fix this TS error
return useCallback<T>((...args) => ref.current(...args), []);
}
36 changes: 22 additions & 14 deletions packages/docusaurus-theme-common/src/utils/scrollUtils.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import React, {
useMemo,
useRef,
} from 'react';
import {useDynamicCallback} from './reactUtils';
import ExecutionEnvironment from '@docusaurus/ExecutionEnvironment';

/**
Expand Down Expand Up @@ -103,20 +104,22 @@ export function useScrollPosition(
const {scrollEventsEnabledRef} = useScrollController();
const lastPositionRef = useRef<ScrollPosition | null>(getScrollPosition());

const handleScroll = () => {
if (!scrollEventsEnabledRef.current) {
return;
}
const currentPosition = getScrollPosition()!;
const dynamicEffect = useDynamicCallback(effect);

if (effect) {
effect(currentPosition, lastPositionRef.current);
}
useEffect(() => {
const handleScroll = () => {
if (!scrollEventsEnabledRef.current) {
return;
}
const currentPosition = getScrollPosition()!;

lastPositionRef.current = currentPosition;
};
if (dynamicEffect) {
dynamicEffect(currentPosition, lastPositionRef.current);
}

lastPositionRef.current = currentPosition;
};

useEffect(() => {
const opts: AddEventListenerOptions & EventListenerOptions = {
passive: true,
};
Expand All @@ -125,7 +128,12 @@ export function useScrollPosition(
window.addEventListener('scroll', handleScroll, opts);

return () => window.removeEventListener('scroll', handleScroll, opts);
}, deps);
}, [
dynamicEffect,
scrollEventsEnabledRef,
// eslint-disable-next-line react-hooks/exhaustive-deps
...deps,
]);
}

type UseScrollPositionSaver = {
Expand Down Expand Up @@ -170,7 +178,7 @@ function useScrollPositionSaver(): UseScrollPositionSaver {
return {restored: heightDiff !== 0};
}, []);

return useMemo(() => ({save, restore}), []);
return useMemo(() => ({save, restore}), [restore, save]);
}

type UseScrollPositionBlockerReturn = {
Expand Down Expand Up @@ -217,7 +225,7 @@ export function useScrollPositionBlocker(): UseScrollPositionBlockerReturn {
}
};
},
[scrollController],
[scrollController, scrollPositionSaver],
);

useLayoutEffect(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {useEffect} from 'react';
import {useLocation} from '@docusaurus/router';
import {Location} from '@docusaurus/history';
import {usePrevious} from './usePrevious';
import {useDynamicCallback} from './reactUtils';

type LocationChangeEvent = {
location: Location;
Expand All @@ -21,10 +22,12 @@ export function useLocationChange(onLocationChange: OnLocationChange): void {
const location = useLocation();
const previousLocation = usePrevious(location);

const onLocationChangeDynamic = useDynamicCallback(onLocationChange);

useEffect(() => {
onLocationChange({
onLocationChangeDynamic({
location,
previousLocation,
});
}, [location]);
}, [onLocationChangeDynamic, location, previousLocation]);
}
5 changes: 3 additions & 2 deletions packages/docusaurus-theme-common/src/utils/usePrevious.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,13 @@
* LICENSE file in the root directory of this source tree.
*/

import {useRef, useEffect} from 'react';
import {useRef} from 'react';
import {useIsomorphicLayoutEffect} from './reactUtils';

export function usePrevious<T>(value: T): T | undefined {
const ref = useRef<T>();

useEffect(() => {
useIsomorphicLayoutEffect(() => {
ref.current = value;
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,11 @@ import clsx from 'clsx';
import Head from '@docusaurus/Head';
import Link from '@docusaurus/Link';
import ExecutionEnvironment from '@docusaurus/ExecutionEnvironment';
import {useTitleFormatter, usePluralForm} from '@docusaurus/theme-common';
import {
useTitleFormatter,
usePluralForm,
useDynamicCallback,
} from '@docusaurus/theme-common';
import useDocusaurusContext from '@docusaurus/useDocusaurusContext';
import {useAllDocsData} from '@theme/hooks/useDocs';
import useSearchQuery from '@theme/hooks/useSearchQuery';
Expand Down Expand Up @@ -173,6 +177,7 @@ function SearchPage() {
},
initialSearchResultState,
);

const algoliaClient = algoliaSearch(appId, apiKey);
const algoliaHelper = algoliaSearchHelper(algoliaClient, indexName, {
hitsPerPage: 15,
Expand Down Expand Up @@ -271,7 +276,7 @@ function SearchPage() {
description: 'The search page title for empty query',
});

const makeSearch = (page = 0) => {
const makeSearch = useDynamicCallback((page = 0) => {
algoliaHelper.addDisjunctiveFacetRefinement('docusaurus_tag', 'default');
algoliaHelper.addDisjunctiveFacetRefinement('language', currentLocale);

Expand All @@ -285,18 +290,16 @@ function SearchPage() {
);

algoliaHelper.setQuery(searchQuery).setPage(page).search();
};
});

useEffect(() => {
if (!loaderRef) {
return undefined;
}
const currentObserver = observer.current;

observer.current.observe(loaderRef);

return () => {
observer.current.unobserve(loaderRef);
};
currentObserver.observe(loaderRef);
return () => currentObserver.unobserve(loaderRef);
}, [loaderRef]);

useEffect(() => {
Expand All @@ -311,21 +314,26 @@ function SearchPage() {
makeSearch();
}, 300);
}
}, [searchQuery, docsSearchVersionsHelpers.searchVersions]);
}, [
searchQuery,
docsSearchVersionsHelpers.searchVersions,
updateSearchPath,
makeSearch,
]);

useEffect(() => {
if (!searchResultState.lastPage || searchResultState.lastPage === 0) {
return;
}

makeSearch(searchResultState.lastPage);
}, [searchResultState.lastPage]);
}, [makeSearch, searchResultState.lastPage]);

useEffect(() => {
if (searchValue && searchValue !== searchQuery) {
setSearchQuery(searchValue);
}
}, [searchValue]);
}, [searchQuery, searchValue]);

return (
<Layout wrapperClassName="search-page-wrapper">
Expand Down
16 changes: 8 additions & 8 deletions packages/docusaurus/src/client/exports/Link.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -90,24 +90,24 @@ function Link({

const IOSupported = ExecutionEnvironment.canUseIntersectionObserver;

let io: IntersectionObserver;
const ioRef = useRef<IntersectionObserver>();
const handleIntersection = (el: HTMLAnchorElement, cb: () => void) => {
io = new window.IntersectionObserver((entries) => {
ioRef.current = new window.IntersectionObserver((entries) => {
entries.forEach((entry) => {
if (el === entry.target) {
// If element is in viewport, stop listening/observing and run callback.
// https://developer.mozilla.org/en-US/docs/Web/API/Intersection_Observer_API
if (entry.isIntersecting || entry.intersectionRatio > 0) {
io.unobserve(el);
io.disconnect();
ioRef.current!.unobserve(el);
ioRef.current!.disconnect();
cb();
}
}
});
});

// Add element to the observer.
io.observe(el);
ioRef.current!.observe(el);
};

const handleRef = (ref: HTMLAnchorElement | null) => {
Expand Down Expand Up @@ -138,11 +138,11 @@ function Link({

// When unmounting, stop intersection observer from watching.
return () => {
if (IOSupported && io) {
io.disconnect();
if (IOSupported && ioRef.current) {
ioRef.current.disconnect();
}
};
}, [targetLink, IOSupported, isInternal]);
}, [ioRef, targetLink, IOSupported, isInternal]);

const isAnchorLink = targetLink?.startsWith('#') ?? false;
const isRegularHtmlLink = !targetLink || !isInternal || isAnchorLink;
Expand Down

0 comments on commit 098f210

Please sign in to comment.