From cbc313f92562a6f880585cf202c0d5ceada2b6b4 Mon Sep 17 00:00:00 2001 From: Todd Schiller Date: Wed, 13 Nov 2024 09:25:35 -0500 Subject: [PATCH 1/2] Enforce react-hooks/exhaustive-deps --- applications/browser-extension/.eslintrc.js | 8 ++++++ .../components/ConfirmationModal.stories.tsx | 27 +++++++++++-------- .../pages/mods/ModsPageActions.tsx | 1 + .../pages/mods/ModsPageSidebar.tsx | 8 +++++- .../pages/mods/gridView/GridView.tsx | 2 +- .../preview/ElementPreview.tsx | 2 +- .../pageEditor/hooks/useAddNewModComponent.ts | 6 ----- .../modals/addBrickModal/useAddBrick.ts | 2 +- 8 files changed, 35 insertions(+), 21 deletions(-) diff --git a/applications/browser-extension/.eslintrc.js b/applications/browser-extension/.eslintrc.js index 0a951ed410..98c6cc7468 100644 --- a/applications/browser-extension/.eslintrc.js +++ b/applications/browser-extension/.eslintrc.js @@ -64,6 +64,14 @@ module.exports = { "error", { ignore: ["eslint-enable"] }, ], + "react-hooks/exhaustive-deps": [ + "error", + { + // Can't add useAsyncEffect to the additional hooks rule because ruleset complains about passing + // `async` methods to hooks. (Because it's technically non-deterministic in execution order) + // additionalHooks: "", + }, + ], "jest/valid-title": [ "error", { diff --git a/applications/browser-extension/src/components/ConfirmationModal.stories.tsx b/applications/browser-extension/src/components/ConfirmationModal.stories.tsx index cf85a22d2f..93ee9290b4 100644 --- a/applications/browser-extension/src/components/ConfirmationModal.stories.tsx +++ b/applications/browser-extension/src/components/ConfirmationModal.stories.tsx @@ -15,7 +15,7 @@ * along with this program. If not, see . */ -import React, { type ComponentProps, useCallback } from "react"; +import React, { type ComponentProps } from "react"; import { type ComponentMeta, type Story } from "@storybook/react"; import AsyncButton from "./AsyncButton"; import { ModalProvider, useModals } from "@/components/ConfirmationModal"; @@ -48,17 +48,22 @@ const ChildComponent = ({ cancelCaption, }: ChildComponentType) => { const { showConfirmation } = useModals(); - const buttonAction = useCallback(async () => { - await showConfirmation({ - title, - message, - submitCaption, - cancelCaption, - }); + return ( + { + await showConfirmation({ + title, + message, + submitCaption, + cancelCaption, + }); - // Do any action here if confirm === true - }, [showConfirmation]); - return Confirm Modal; + // Do any action here if confirm === true + }} + > + Confirm Modal + + ); }; const Template: Story = (args) => { diff --git a/applications/browser-extension/src/extensionConsole/pages/mods/ModsPageActions.tsx b/applications/browser-extension/src/extensionConsole/pages/mods/ModsPageActions.tsx index 606dabb664..c3816086c4 100644 --- a/applications/browser-extension/src/extensionConsole/pages/mods/ModsPageActions.tsx +++ b/applications/browser-extension/src/extensionConsole/pages/mods/ModsPageActions.tsx @@ -203,6 +203,7 @@ const ModsPageActions: React.FunctionComponent<{ history, marketplaceListingUrl, modId, + editablePackageId, name, showDeactivate, showDelete, diff --git a/applications/browser-extension/src/extensionConsole/pages/mods/ModsPageSidebar.tsx b/applications/browser-extension/src/extensionConsole/pages/mods/ModsPageSidebar.tsx index 4fdd08fec1..2ec935a667 100644 --- a/applications/browser-extension/src/extensionConsole/pages/mods/ModsPageSidebar.tsx +++ b/applications/browser-extension/src/extensionConsole/pages/mods/ModsPageSidebar.tsx @@ -210,7 +210,13 @@ const ModsPageSidebar: React.VoidFunctionComponent = ({ if (debouncedSearchInput) { setActiveTab(MODS_PAGE_TABS.all); } - }, [globalFilter, debouncedSearchInput, setActiveTab, setGlobalFilter]); + }, [ + globalFilter, + debouncedSearchInput, + setActiveTab, + setSearchQuery, + setGlobalFilter, + ]); return (
diff --git a/applications/browser-extension/src/extensionConsole/pages/mods/gridView/GridView.tsx b/applications/browser-extension/src/extensionConsole/pages/mods/gridView/GridView.tsx index 5f207ee16d..6db20d9fa3 100644 --- a/applications/browser-extension/src/extensionConsole/pages/mods/gridView/GridView.tsx +++ b/applications/browser-extension/src/extensionConsole/pages/mods/gridView/GridView.tsx @@ -114,7 +114,7 @@ const GridView: React.VoidFunctionComponent = ({ // Re-render the list when expandedRows changes. useEffect(() => { listRef.current?.resetAfterIndex(0); - }, [expandedGridRows, columnCount]); + }, [listRef, expandedGridRows, columnCount]); const GridRow = useCallback( ({ diff --git a/applications/browser-extension/src/pageEditor/documentBuilder/preview/ElementPreview.tsx b/applications/browser-extension/src/pageEditor/documentBuilder/preview/ElementPreview.tsx index be434711f9..d525335cf2 100644 --- a/applications/browser-extension/src/pageEditor/documentBuilder/preview/ElementPreview.tsx +++ b/applications/browser-extension/src/pageEditor/documentBuilder/preview/ElementPreview.tsx @@ -111,7 +111,7 @@ const useScrollIntoViewEffect = (elementName: string, isActive: boolean) => { scrollIntoView, ); }; - }, []); + }, [elementName, isActive]); return elementRef; }; diff --git a/applications/browser-extension/src/pageEditor/hooks/useAddNewModComponent.ts b/applications/browser-extension/src/pageEditor/hooks/useAddNewModComponent.ts index db4d8f0ac1..75aaab1e5c 100644 --- a/applications/browser-extension/src/pageEditor/hooks/useAddNewModComponent.ts +++ b/applications/browser-extension/src/pageEditor/hooks/useAddNewModComponent.ts @@ -24,7 +24,6 @@ import { isSpecificError } from "@/errors/errorHelpers"; import { type ModComponentFormStateAdapter } from "@/pageEditor/starterBricks/modComponentFormStateAdapter"; import { updateDraftModComponent } from "@/contentScript/messenger/api"; import { type SettingsState } from "@/store/settings/settingsTypes"; -import useFlags from "@/hooks/useFlags"; import { type ModComponentFormState } from "@/pageEditor/starterBricks/formStateTypes"; import reportEvent from "@/telemetry/reportEvent"; import { Events } from "@/telemetry/events"; @@ -71,10 +70,6 @@ function useFreshModNameGenerator(): () => string { function useAddNewModComponent(modMetadata?: ModMetadata): AddNewModComponent { const dispatch = useDispatch(); const { setInsertingStarterBrickType } = useInsertPane(); - // XXX: useFlags is async. The flag query might not be initialized by the time the callback is called. Ensure - // useFlags has already been used on the page, e.g., the AddStarterBrickButton, to ensure the flags have loaded by - // the time the returned callback is called. - const { flagOff } = useFlags(); const suggestElements = useSelector<{ settings: SettingsState }, boolean>( (x) => x.settings.suggestElements ?? false, ); @@ -171,7 +166,6 @@ function useAddNewModComponent(modMetadata?: ModMetadata): AddNewModComponent { }, [ dispatch, - flagOff, getInitialModComponentFormState, getModDraftStateForModId, modMetadata, diff --git a/applications/browser-extension/src/pageEditor/modals/addBrickModal/useAddBrick.ts b/applications/browser-extension/src/pageEditor/modals/addBrickModal/useAddBrick.ts index edb0026085..976a718bbf 100644 --- a/applications/browser-extension/src/pageEditor/modals/addBrickModal/useAddBrick.ts +++ b/applications/browser-extension/src/pageEditor/modals/addBrickModal/useAddBrick.ts @@ -143,7 +143,7 @@ function useAddBrick(): AddBrick { return {}; }, - [activeModComponent, modalData, makeNewBrick], + [activeModComponent, getModDraftStateForModId, modalData, makeNewBrick], ); const addBrick = useCallback( From 2e518acdb40e5fe7a689b2827ee853b217e835a1 Mon Sep 17 00:00:00 2001 From: Todd Schiller Date: Wed, 13 Nov 2024 09:27:59 -0500 Subject: [PATCH 2/2] Improve comment --- applications/browser-extension/.eslintrc.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/applications/browser-extension/.eslintrc.js b/applications/browser-extension/.eslintrc.js index 98c6cc7468..4727cad38d 100644 --- a/applications/browser-extension/.eslintrc.js +++ b/applications/browser-extension/.eslintrc.js @@ -67,9 +67,9 @@ module.exports = { "react-hooks/exhaustive-deps": [ "error", { - // Can't add useAsyncEffect to the additional hooks rule because ruleset complains about passing + // Can't add useAsyncEffect to the additionalHooks property because rule complains about passing // `async` methods to hooks. (Because it's technically non-deterministic in execution order) - // additionalHooks: "", + // https://github.com/facebook/react/blob/main/packages/eslint-plugin-react-hooks/README.md#advanced-configuration }, ], "jest/valid-title": [