From fab9974c9786ed4870492e4f8a3fd6ec88b8b1ba Mon Sep 17 00:00:00 2001 From: Raquel Amorim <36057168+amorimr@users.noreply.github.com> Date: Wed, 28 Sep 2022 10:55:42 -0400 Subject: [PATCH] chore(website): Enable react-hooks exhaustive deps rules (#5663) * enable rules * fix warnings * add missing dependencies * fix infinite rerenders * fix expansion of tree items --- packages/website/.eslintrc.js | 1 - .../website/src/components/ASTViewerTS.tsx | 12 ++- .../src/components/OptionsSelector.tsx | 4 +- .../website/src/components/Playground.tsx | 15 ++-- .../website/src/components/ast/Elements.tsx | 6 +- .../src/components/ast/PropertyName.tsx | 14 ++-- .../website/src/components/ast/SimpleItem.tsx | 2 +- .../src/components/config/ConfigEditor.tsx | 15 ++-- .../src/components/config/ConfigEslint.tsx | 21 ++--- .../components/config/ConfigTypeScript.tsx | 17 ++-- .../src/components/editor/LoadedEditor.tsx | 84 ++++++++++++++----- .../components/editor/useSandboxServices.ts | 5 +- .../components/hooks/useDebouncedToggle.ts | 2 +- .../src/components/inputs/Checkbox.tsx | 21 ++--- .../website/src/components/modals/Modal.tsx | 10 ++- 15 files changed, 148 insertions(+), 81 deletions(-) diff --git a/packages/website/.eslintrc.js b/packages/website/.eslintrc.js index 8b30fb62498..0b55f475b33 100644 --- a/packages/website/.eslintrc.js +++ b/packages/website/.eslintrc.js @@ -22,7 +22,6 @@ module.exports = { 'react/jsx-no-target-blank': 'off', 'react/no-unescaped-entities': 'off', '@typescript-eslint/internal/prefer-ast-types-enum': 'off', - 'react-hooks/exhaustive-deps': 'off', // TODO: enable it later }, settings: { react: { diff --git a/packages/website/src/components/ASTViewerTS.tsx b/packages/website/src/components/ASTViewerTS.tsx index 1bbe5799a05..30dda954a43 100644 --- a/packages/website/src/components/ASTViewerTS.tsx +++ b/packages/website/src/components/ASTViewerTS.tsx @@ -53,7 +53,17 @@ export default function ASTViewerTS({ ['TypeFlags', typeFlags], ); setModel(serialize(value, scopeSerializer)); - }, [value, syntaxKind]); + }, [ + value, + syntaxKind, + nodeFlags, + tokenFlags, + modifierFlags, + objectFlags, + symbolFlags, + flowFlags, + typeFlags, + ]); return ( diff --git a/packages/website/src/components/OptionsSelector.tsx b/packages/website/src/components/OptionsSelector.tsx index a39797d062b..fe573ef86ab 100644 --- a/packages/website/src/components/OptionsSelector.tsx +++ b/packages/website/src/components/OptionsSelector.tsx @@ -51,7 +51,7 @@ function OptionsSelectorContent({ .then(() => { setCopyLink(true); }); - }, []); + }, [setCopyLink]); const copyMarkdownToClipboard = useCallback(() => { if (isLoading) { @@ -60,7 +60,7 @@ function OptionsSelectorContent({ void navigator.clipboard.writeText(createMarkdown(state)).then(() => { setCopyMarkdown(true); }); - }, [state, isLoading]); + }, [isLoading, state, setCopyMarkdown]); const openIssue = useCallback(() => { if (isLoading) { diff --git a/packages/website/src/components/Playground.tsx b/packages/website/src/components/Playground.tsx index 3330e499861..2a643a93976 100644 --- a/packages/website/src/components/Playground.tsx +++ b/packages/website/src/components/Playground.tsx @@ -84,6 +84,15 @@ function Playground(): JSX.Element { [setState], ); + const onLoaded = useCallback( + (ruleNames: RuleDetails[], tsVersions: readonly string[]): void => { + setRuleNames(ruleNames); + setTSVersion(tsVersions); + setIsLoading(false); + }, + [], + ); + return (
{ruleNames.length > 0 && ( @@ -150,11 +159,7 @@ function Playground(): JSX.Element { onMarkersChange={setMarkers} decoration={selectedRange} onChange={setState} - onLoaded={(ruleNames, tsVersions): void => { - setRuleNames(ruleNames); - setTSVersion(tsVersions); - setIsLoading(false); - }} + onLoaded={onLoaded} onSelect={setPosition} />
diff --git a/packages/website/src/components/ast/Elements.tsx b/packages/website/src/components/ast/Elements.tsx index 985a5f01d81..b8a9d7c823c 100644 --- a/packages/website/src/components/ast/Elements.tsx +++ b/packages/website/src/components/ast/Elements.tsx @@ -30,7 +30,7 @@ export function ComplexItem({ } } }, - [data], + [data.model.range, onSelectNode], ); useEffect(() => { @@ -44,10 +44,10 @@ export function ComplexItem({ level !== 'ast' && selected && !hasChildInRange(selection, data.model), ); - if (selected && !isExpanded) { + if (selected) { setIsExpanded(selected); } - }, [selection, data]); + }, [selection, data, level]); return ( ) => { e.preventDefault(); - props.onClick?.(e); + onClickProps?.(e); }, - [props.onClick], + [onClickProps], ); const onMouseEnter = useCallback(() => { - props.onHover?.(true); - }, [props.onHover]); + onHover?.(true); + }, [onHover]); const onMouseLeave = useCallback(() => { - props.onHover?.(false); - }, [props.onHover]); + onHover?.(false); + }, [onHover]); return props.onClick || props.onHover ? ( <> diff --git a/packages/website/src/components/ast/SimpleItem.tsx b/packages/website/src/components/ast/SimpleItem.tsx index 1bc3d368049..23a25a8a57d 100644 --- a/packages/website/src/components/ast/SimpleItem.tsx +++ b/packages/website/src/components/ast/SimpleItem.tsx @@ -20,7 +20,7 @@ export function SimpleItem({ onSelectNode(state ? data.model.range : null); } }, - [data], + [data.model.range, onSelectNode], ); return ( diff --git a/packages/website/src/components/config/ConfigEditor.tsx b/packages/website/src/components/config/ConfigEditor.tsx index 538ee85dd1b..1d6e9346e0b 100644 --- a/packages/website/src/components/config/ConfigEditor.tsx +++ b/packages/website/src/components/config/ConfigEditor.tsx @@ -89,26 +89,27 @@ function isDefault(value: unknown, defaults?: unknown[]): boolean { } function ConfigEditor(props: ConfigEditorProps): JSX.Element { + const { onClose: onCloseProps, isOpen, values } = props; const [filter, setFilter] = useState(''); const [config, setConfig] = useReducer(reducerObject, {}); const [filterInput, setFilterFocus] = useFocus(); const onClose = useCallback(() => { - props.onClose(config); - }, [props.onClose, config]); + onCloseProps(config); + }, [onCloseProps, config]); useEffect(() => { - setConfig({ type: 'init', config: props.values }); - }, [props.values]); + setConfig({ type: 'init', config: values }); + }, [values]); useEffect(() => { - if (props.isOpen) { + if (isOpen) { setFilterFocus(); } - }, [props.isOpen]); + }, [isOpen, setFilterFocus]); return ( - +
([]); const [configObject, updateConfigObject] = useState(); useEffect(() => { - if (props.isOpen) { - updateConfigObject(parseESLintRC(props.config)); + if (isOpen) { + updateConfigObject(parseESLintRC(config)); } - }, [props.isOpen, props.config]); + }, [isOpen, config]); useEffect(() => { updateOptions([ { heading: 'Rules', - fields: props.ruleOptions + fields: ruleOptions .filter(item => item.name.startsWith('@typescript')) .map(item => ({ key: item.name, @@ -52,7 +53,7 @@ function ConfigEslint(props: ConfigEslintProps): JSX.Element { }, { heading: 'Core rules', - fields: props.ruleOptions + fields: ruleOptions .filter(item => !item.name.startsWith('@typescript')) .map(item => ({ key: item.name, @@ -62,7 +63,7 @@ function ConfigEslint(props: ConfigEslintProps): JSX.Element { })), }, ]); - }, [props.ruleOptions]); + }, [ruleOptions]); const onClose = useCallback( (newConfig: Record) => { @@ -76,14 +77,14 @@ function ConfigEslint(props: ConfigEslintProps): JSX.Element { .filter(checkOptions), ); if (!shallowEqual(cfg, configObject?.rules)) { - props.onClose({ + onCloseProps({ eslintrc: toJson({ ...(configObject ?? {}), rules: cfg }), }); } else { - props.onClose(); + onCloseProps(); } }, - [props.onClose, configObject], + [onCloseProps, configObject], ); return ( @@ -91,7 +92,7 @@ function ConfigEslint(props: ConfigEslintProps): JSX.Element { header="Eslint Config" options={options} values={configObject?.rules ?? {}} - isOpen={props.isOpen} + isOpen={isOpen} onClose={onClose} /> ); diff --git a/packages/website/src/components/config/ConfigTypeScript.tsx b/packages/website/src/components/config/ConfigTypeScript.tsx index 78cccd9e9e6..40cd634ffb1 100644 --- a/packages/website/src/components/config/ConfigTypeScript.tsx +++ b/packages/website/src/components/config/ConfigTypeScript.tsx @@ -13,14 +13,15 @@ interface ConfigTypeScriptProps { } function ConfigTypeScript(props: ConfigTypeScriptProps): JSX.Element { + const { onClose: onCloseProps, isOpen, config } = props; const [tsConfigOptions, updateOptions] = useState([]); const [configObject, updateConfigObject] = useState(); useEffect(() => { - if (props.isOpen) { - updateConfigObject(parseTSConfig(props.config)); + if (isOpen) { + updateConfigObject(parseTSConfig(config)); } - }, [props.isOpen, props.config]); + }, [isOpen, config]); useEffect(() => { if (window.ts) { @@ -54,20 +55,20 @@ function ConfigTypeScript(props: ConfigTypeScriptProps): JSX.Element { ), ); } - }, [props.isOpen]); + }, [isOpen]); const onClose = useCallback( (newConfig: Record) => { const cfg = { ...newConfig }; if (!shallowEqual(cfg, configObject?.compilerOptions)) { - props.onClose({ + onCloseProps({ tsconfig: toJson({ ...(configObject ?? {}), compilerOptions: cfg }), }); } else { - props.onClose(); + onCloseProps(); } }, - [props.onClose, configObject], + [onCloseProps, configObject], ); return ( @@ -75,7 +76,7 @@ function ConfigTypeScript(props: ConfigTypeScriptProps): JSX.Element { header="TypeScript Config" options={tsConfigOptions} values={configObject?.compilerOptions ?? {}} - isOpen={props.isOpen} + isOpen={isOpen} onClose={onClose} /> ); diff --git a/packages/website/src/components/editor/LoadedEditor.tsx b/packages/website/src/components/editor/LoadedEditor.tsx index 3b959580193..e641d77baa5 100644 --- a/packages/website/src/components/editor/LoadedEditor.tsx +++ b/packages/website/src/components/editor/LoadedEditor.tsx @@ -47,7 +47,8 @@ export const LoadedEditor: React.FC = ({ webLinter, activeTab, }) => { - const [decorations, setDecorations] = useState([]); + const [_, setDecorations] = useState([]); + const codeActions = useRef(new Map()).current; const [tabs] = useState>(() => { const tabsDefault = { @@ -75,7 +76,12 @@ export const LoadedEditor: React.FC = ({ resource: model.uri, }); onMarkersChange(parseMarkers(markers, codeActions, sandboxInstance.editor)); - }, []); + }, [ + codeActions, + onMarkersChange, + sandboxInstance.editor, + sandboxInstance.monaco.editor, + ]); useEffect(() => { const newPath = jsx ? '/input.tsx' : '/input.ts'; @@ -92,7 +98,13 @@ export const LoadedEditor: React.FC = ({ tabs.code.dispose(); tabs.code = newModel; } - }, [jsx]); + }, [ + jsx, + sandboxInstance.editor, + sandboxInstance.monaco.Uri, + sandboxInstance.monaco.editor, + tabs, + ]); useEffect(() => { const config = createCompilerOptions( @@ -101,19 +113,19 @@ export const LoadedEditor: React.FC = ({ ); webLinter.updateCompilerOptions(config); sandboxInstance.setCompilerSettings(config); - }, [jsx, tsconfig]); + }, [jsx, sandboxInstance, tsconfig, webLinter]); useEffect(() => { webLinter.updateRules(parseESLintRC(eslintrc).rules); - }, [eslintrc]); + }, [eslintrc, webLinter]); useEffect(() => { sandboxInstance.editor.setModel(tabs[activeTab]); updateMarkers(); - }, [activeTab]); + }, [activeTab, sandboxInstance.editor, tabs, updateMarkers]); - useEffect( - debounce(() => { + useEffect(() => { + const lintEditor = debounce(() => { // eslint-disable-next-line no-console console.info('[Editor] linting triggered'); @@ -146,9 +158,28 @@ export const LoadedEditor: React.FC = ({ onTsASTChange(webLinter.storedTsAST); onScopeChange(webLinter.storedScope); onSelect(sandboxInstance.editor.getPosition()); - }, 500), - [code, jsx, tsconfig, eslintrc, sourceType, webLinter], - ); + }, 500); + + lintEditor(); + }, [ + code, + jsx, + tsconfig, + eslintrc, + sourceType, + webLinter, + onEsASTChange, + onTsASTChange, + onScopeChange, + onSelect, + sandboxInstance.editor, + sandboxInstance.monaco.editor, + sandboxInstance.monaco.Uri, + codeActions, + tabs.code, + updateMarkers, + onMarkersChange, + ]); useEffect(() => { // configure the JSON language support with schemas and schema associations @@ -222,7 +253,20 @@ export const LoadedEditor: React.FC = ({ } } }; - }, []); + }, [ + codeActions, + main.languages, + onChange, + onSelect, + sandboxInstance.editor, + sandboxInstance.monaco.editor, + sandboxInstance.monaco.languages.json.jsonDefaults, + tabs.code, + tabs.eslintrc, + tabs.tsconfig, + updateMarkers, + webLinter.ruleNames, + ]); const resize = useMemo(() => { return debounce(() => sandboxInstance.editor.layout(), 1); @@ -237,7 +281,7 @@ export const LoadedEditor: React.FC = ({ return new ResizeObserver(() => { resize(); }); - }, []); + }, [resize]); useEffect(() => { if (domNode) { @@ -246,7 +290,7 @@ export const LoadedEditor: React.FC = ({ return (): void => resizeObserver.unobserve(domNode); } return (): void => {}; - }, [domNode]); + }, [domNode, resizeObserver]); useEffect(() => { window.addEventListener('resize', resize); @@ -264,7 +308,7 @@ export const LoadedEditor: React.FC = ({ }, ]); } - }, [code]); + }, [code, tabs.code]); useEffect(() => { if (tsconfig !== tabs.tsconfig.getValue()) { @@ -275,7 +319,7 @@ export const LoadedEditor: React.FC = ({ }, ]); } - }, [tsconfig]); + }, [tabs.tsconfig, tsconfig]); useEffect(() => { if (eslintrc !== tabs.eslintrc.getValue()) { @@ -286,7 +330,7 @@ export const LoadedEditor: React.FC = ({ }, ]); } - }, [eslintrc]); + }, [eslintrc, tabs.eslintrc]); useEffect(() => { sandboxInstance.monaco.editor.setTheme(darkTheme ? 'vs-dark' : 'vs-light'); @@ -294,9 +338,9 @@ export const LoadedEditor: React.FC = ({ useEffect(() => { if (sandboxInstance.editor.getModel() === tabs.code) { - setDecorations( + setDecorations(prevDecorations => sandboxInstance.editor.deltaDecorations( - decorations, + prevDecorations, decoration && showAST ? [ { @@ -316,7 +360,7 @@ export const LoadedEditor: React.FC = ({ ), ); } - }, [decoration, sandboxInstance, showAST]); + }, [decoration, sandboxInstance, showAST, tabs.code]); return null; }; diff --git a/packages/website/src/components/editor/useSandboxServices.ts b/packages/website/src/components/editor/useSandboxServices.ts index d316f75acf9..73c336676b3 100644 --- a/packages/website/src/components/editor/useSandboxServices.ts +++ b/packages/website/src/components/editor/useSandboxServices.ts @@ -32,6 +32,7 @@ export interface SandboxServices { export const useSandboxServices = ( props: SandboxServicesProps, ): Error | SandboxServices | undefined => { + const { onLoaded } = props; const [services, setServices] = useState(); const [loadedTs, setLoadedTs] = useState(props.ts); const { colorMode } = useColorMode(); @@ -109,7 +110,7 @@ export const useSandboxServices = ( const webLinter = new WebLinter(system, compilerOptions, lintUtils); - props.onLoaded( + onLoaded( webLinter.ruleNames, Array.from( new Set([...sandboxInstance.supportedVersions, window.ts.version]), @@ -144,7 +145,7 @@ export const useSandboxServices = ( model.dispose(); } }; - }, [props.ts]); + }, [props.ts, colorMode, props.jsx, onLoaded]); return services; }; diff --git a/packages/website/src/components/hooks/useDebouncedToggle.ts b/packages/website/src/components/hooks/useDebouncedToggle.ts index b34bb889b0a..d857f4aea2c 100644 --- a/packages/website/src/components/hooks/useDebouncedToggle.ts +++ b/packages/website/src/components/hooks/useDebouncedToggle.ts @@ -19,7 +19,7 @@ export default function useDebouncedToggle( setState(value); }, timeout); }, - [timeoutIdRef], + [timeout, value], ); return [state, update]; diff --git a/packages/website/src/components/inputs/Checkbox.tsx b/packages/website/src/components/inputs/Checkbox.tsx index ed5a473b157..0134bfa4001 100644 --- a/packages/website/src/components/inputs/Checkbox.tsx +++ b/packages/website/src/components/inputs/Checkbox.tsx @@ -1,4 +1,4 @@ -import React, { createRef, useEffect } from 'react'; +import React, { useCallback } from 'react'; export interface CheckboxProps { readonly name: string; @@ -10,17 +10,18 @@ export interface CheckboxProps { } function Checkbox(props: CheckboxProps): JSX.Element { - const checkboxRef = createRef(); + const { indeterminate } = props; - useEffect(() => { - if (!checkboxRef.current) { - return; - } + const checkboxRef = useCallback( + (node: HTMLInputElement | null) => { + if (!node) { + return; + } - if (props.indeterminate !== checkboxRef.current.indeterminate) { - checkboxRef.current.indeterminate = props.indeterminate ?? false; - } - }, [props.indeterminate]); + node.indeterminate = indeterminate ?? false; + }, + [indeterminate], + ); return ( { const closeOnEscapeKeyDown = (e: KeyboardEvent): void => { if ( @@ -21,7 +23,7 @@ function Modal(props: ModalProps): JSX.Element { // eslint-disable-next-line deprecation/deprecation -- intentional fallback for old browsers e.keyCode === 27 ) { - props.onClose(); + onClose(); } }; @@ -29,15 +31,15 @@ function Modal(props: ModalProps): JSX.Element { return (): void => { document.body.removeEventListener('keydown', closeOnEscapeKeyDown); }; - }, []); + }, [onClose]); const onClick = useCallback( (e: MouseEvent) => { if (e.currentTarget === e.target) { - props.onClose(); + onClose(); } }, - [props.onClose], + [onClose], ); return (