Enable react hooks linting and fix issues (#3761)

fix #3745
microsoft · Jul 8, 2024 · 4553419 · 4553419
1 parent 53836c5
commit 4553419
Show file tree

Hide file tree

Showing 25 changed files with 133 additions and 88 deletions.
diff --git a/.chronus/changes/fix-react-hooks-2024-6-8-15-40-17.md b/.chronus/changes/fix-react-hooks-2024-6-8-15-40-17.md
@@ -0,0 +1,9 @@
+---
+# Change versionKind to one of: internal, fix, dependencies, feature, deprecation, breaking
+changeKind: internal
+packages:
+  - "@typespec/html-program-viewer"
+  - "@typespec/playground"
+---
+
+Enable react hooks linting and fix issues
diff --git a/eslint.config.js b/eslint.config.js
@@ -1,6 +1,7 @@
 // @ts-check
 import eslint from "@eslint/js";
 import deprecation from "eslint-plugin-deprecation";
+import reactHooks from "eslint-plugin-react-hooks";
 import unicorn from "eslint-plugin-unicorn";
 import vitest from "eslint-plugin-vitest";
 import { dirname } from "path";
@@ -105,10 +106,20 @@ const testFilesConfig = tsEslint.config({
   },
 });
 
+const jsxFilesConfig = tsEslint.config({
+  files: ["**/*.tsx"],
+  plugins: { "react-hooks": reactHooks },
+  rules: {
+    "react-hooks/rules-of-hooks": "error",
+    "react-hooks/exhaustive-deps": "warn",
+  },
+});
+
 export const TypeSpecCommonEslintConfigs = [
   eslint.configs.recommended,
   ...tsEslint.configs.recommended,
   ...allFilesConfig,
+  ...jsxFilesConfig,
   ...testFilesConfig,
 ];
 

diff --git a/package.json b/package.json
@@ -52,6 +52,7 @@
     "eslint": "^8.57.0",
     "eslint-plugin-deprecation": "^3.0.0",
     "eslint-plugin-import": "^2.29.1",
+    "eslint-plugin-react-hooks": "^4.6.2",
     "eslint-plugin-unicorn": "^54.0.0",
     "eslint-plugin-vitest": "^0.5.4",
     "picocolors": "~1.0.1",

diff --git a/packages/html-program-viewer/src/react/current-path/current-path.tsx b/packages/html-program-viewer/src/react/current-path/current-path.tsx
@@ -97,7 +97,7 @@ const Search = ({ onBlur }: SearchProps) => {
         setSearch(item.optionValue.slice(2));
       }
     },
-    [nav.selectPath, setSearch]
+    [nav]
   );
 
   return (

diff --git a/packages/html-program-viewer/src/react/inspect-type/inspect-type.tsx b/packages/html-program-viewer/src/react/inspect-type/inspect-type.tsx
@@ -37,7 +37,7 @@ const EntityUI: FunctionComponent<EntityUIProps> = ({ entity }) => {
 const TypeUI: FunctionComponent<{ type: Type }> = ({ type }) => {
   const nav = useTreeNavigatorOptional();
 
-  const navToType = useCallback(() => nav?.navToType(type), [nav?.navToType, type]);
+  const navToType = useCallback(() => nav?.navToType(type), [nav, type]);
   return (
     <div className={style["type-ui"]}>
       <div className={style["type-ui-header"]} onClick={nav && navToType}>
@@ -54,7 +54,7 @@ const NamedTypeRef: FunctionComponent<{ type: NamedType }> = ({ type }) => {
 
   const navToType = useCallback(() => {
     nav?.navToType(type);
-  }, [nav?.navToType, type]);
+  }, [nav, type]);
   return (
     <a className={style["named-type-ref"]} onClick={nav && navToType}>
       {getTypeName(type)}

diff --git a/packages/html-program-viewer/src/react/list-type-view/list-type-view.tsx b/packages/html-program-viewer/src/react/list-type-view/list-type-view.tsx
@@ -32,7 +32,7 @@ export const ListTypeView = ({ nav, node }: ListTypeViewProps) => {
 const Item = ({ item, nav }: { nav: TreeNavigator; item: TypeGraphNode }) => {
   const select = useCallback(() => {
     nav.selectPath(item.id);
-  }, [nav.selectPath]);
+  }, [item.id, nav]);
   return (
     <ListItem onAction={select} className={style["item"]}>
       {item.name}

diff --git a/packages/playground/src/react/diagnostic-list/diagnostic-list.tsx b/packages/playground/src/react/diagnostic-list/diagnostic-list.tsx
@@ -17,16 +17,15 @@ export const DiagnosticList: FunctionComponent<DiagnosticListProps> = ({
   diagnostics,
   onDiagnosticSelected,
 }) => {
-  if (diagnostics.length === 0) {
-    return <div className={style["list"]}>No errors</div>;
-  }
-
   const handleItemSelected = useCallback(
     (diagnostic: Diagnostic) => {
       onDiagnosticSelected?.(diagnostic);
     },
     [onDiagnosticSelected]
   );
+  if (diagnostics.length === 0) {
+    return <div className={style["list"]}>No errors</div>;
+  }
   return (
     <div className={style["list"]}>
       {diagnostics.map((x, i) => {

diff --git a/packages/playground/src/react/editor.tsx b/packages/playground/src/react/editor.tsx
@@ -28,6 +28,8 @@ export const Editor: FunctionComponent<EditorProps> = ({ model, options, actions
       ...options,
     });
     onMount?.({ editor: editorRef.current });
+    // This needs special handling where we only want to run this effect once
+    // eslint-disable-next-line react-hooks/exhaustive-deps
   }, []);
 
   useEffect(() => {

diff --git a/packages/playground/src/react/file-output/file-output.tsx b/packages/playground/src/react/file-output/file-output.tsx
@@ -23,24 +23,22 @@ export const FileOutput: FunctionComponent<FileOutputProps> = ({ filename, conte
   );
   const keys = Object.keys(resolvedViewers);
 
-  if (keys.length === 0) {
-    return <>No viewers</>;
-  } else if (keys.length === 1) {
-    return resolvedViewers[keys[0]].render({ filename, content });
-  }
-
   const [selected, setSelected] = useState<string>(keys[0]);
 
-  const handleSelected = useCallback(
-    (_: unknown, data: SelectOnChangeData) => {
-      setSelected(data.value);
-    },
-    [selected]
-  );
+  const handleSelected = useCallback((_: unknown, data: SelectOnChangeData) => {
+    setSelected(data.value);
+  }, []);
 
   const selectedRender = useMemo(() => {
     return resolvedViewers[selected].render;
   }, [selected, resolvedViewers]);
+
+  if (keys.length === 0) {
+    return <>No viewers</>;
+  } else if (keys.length === 1) {
+    return resolvedViewers[keys[0]].render({ filename, content });
+  }
+
   return (
     <div className={style["file-output"]}>
       <div className={style["viewer-selector"]}>

diff --git a/packages/playground/src/react/output-view/file-viewer.tsx b/packages/playground/src/react/output-view/file-viewer.tsx
@@ -14,6 +14,14 @@ const FileViewerComponent = ({
   const [filename, setFilename] = useState<string>("");
   const [content, setContent] = useState<string>("");
 
+  const loadOutputFile = useCallback(
+    async (path: string) => {
+      const contents = await program.host.readFile("./tsp-output/" + path);
+      setContent(contents.text);
+    },
+    [program.host]
+  );
+
   useEffect(() => {
     if (outputFiles.length > 0) {
       const fileStillThere = outputFiles.find((x) => x === filename);
@@ -23,19 +31,14 @@ const FileViewerComponent = ({
     } else {
       setFilename("");
     }
-  }, [program, outputFiles]);
-
-  async function loadOutputFile(path: string) {
-    const contents = await program.host.readFile("./tsp-output/" + path);
-    setContent(contents.text);
-  }
+  }, [program, outputFiles, loadOutputFile, filename]);
 
   const handleTabSelection = useCallback(
     (newFilename: string) => {
       setFilename(newFilename);
       void loadOutputFile(newFilename);
     },
-    [setFilename]
+    [loadOutputFile]
   );
 
   if (outputFiles.length === 0) {

diff --git a/packages/playground/src/react/output-view/output-view.tsx b/packages/playground/src/react/output-view/output-view.tsx
@@ -24,13 +24,17 @@ export const OutputView: FunctionComponent<OutputViewProps> = ({
   fileViewers,
   editorOptions,
 }) => {
+  const resolvedViewers = useMemo(
+    () => resolveViewers(viewers, fileViewers),
+    [fileViewers, viewers]
+  );
+
   if (compilationState === undefined) {
     return <></>;
   }
   if ("internalCompilerError" in compilationState) {
     return <></>;
   }
-  const resolvedViewers = useMemo(() => resolveViewers(viewers, fileViewers), [viewers]);
   return (
     <OutputViewInternal
       compilationResult={compilationState}

diff --git a/packages/playground/src/react/playground.tsx b/packages/playground/src/react/playground.tsx
@@ -159,7 +159,7 @@ export const Playground: FunctionComponent<PlaygroundProps> = (props) => {
   );
   useEffect(() => {
     updateTypeSpec(props.defaultContent ?? "");
-  }, []);
+  }, [props.defaultContent, updateTypeSpec]);
 
   useEffect(() => {
     if (selectedSampleName && props.samples) {
@@ -174,7 +174,13 @@ export const Playground: FunctionComponent<PlaygroundProps> = (props) => {
         }
       }
     }
-  }, [updateTypeSpec, selectedSampleName]);
+  }, [
+    updateTypeSpec,
+    selectedSampleName,
+    props.samples,
+    onSelectedEmitterChange,
+    onCompilerOptionsChange,
+  ]);
 
   useEffect(() => {
     const debouncer = debounce(() => doCompile(), 200);
@@ -209,14 +215,14 @@ export const Playground: FunctionComponent<PlaygroundProps> = (props) => {
 
   const formatCode = useCallback(() => {
     void editorRef.current?.getAction("editor.action.formatDocument")?.run();
-  }, [typespecModel]);
+  }, []);
 
   const fileBug = useCallback(async () => {
     if (props.onFileBug) {
       saveCode();
       props.onFileBug();
     }
-  }, [saveCode, typespecModel, props.onFileBug]);
+  }, [props, saveCode]);
 
   const typespecEditorActions = useMemo(
     (): editor.IActionDescriptor[] => [
@@ -251,7 +257,7 @@ export const Playground: FunctionComponent<PlaygroundProps> = (props) => {
     (diagnostic: Diagnostic) => {
       editorRef.current?.setSelection(getMonacoRange(host.compiler, diagnostic.target));
     },
-    [setVerticalPaneSizes]
+    [host.compiler]
   );
 
   return (

diff --git a/packages/playground/src/react/samples-dropdown.tsx b/packages/playground/src/react/samples-dropdown.tsx
@@ -23,7 +23,7 @@ export const SamplesDropdown: FunctionComponent<SamplesDropdownProps> = ({
         onSelectedSampleNameChange(evt.target.value);
       }
     },
-    [onSelectedSampleNameChange]
+    [onSelectedSampleNameChange, samples]
   );
   return (
     <Select

diff --git a/packages/playground/src/react/settings/compiler-settings.tsx b/packages/playground/src/react/settings/compiler-settings.tsx
@@ -26,7 +26,7 @@ export const CompilerSettings: FunctionComponent<CompilerSettingsProps> = ({
         options: emitterOptions,
       });
     },
-    [options]
+    [onOptionsChanged, options]
   );
   const library = host.libraries[selectedEmitter];
   const linterRuleSet = options.linterRuleSet ?? {};
@@ -37,7 +37,7 @@ export const CompilerSettings: FunctionComponent<CompilerSettingsProps> = ({
         linterRuleSet,
       });
     },
-    [options]
+    [onOptionsChanged, options]
   );
   return (
     <div>

diff --git a/packages/playground/src/react/settings/emitter-options-form.tsx b/packages/playground/src/react/settings/emitter-options-form.tsx
@@ -25,12 +25,6 @@ export const EmitterOptionsForm: FunctionComponent<EmitterOptionsFormProps> = ({
   options,
   optionsChanged,
 }) => {
-  const emitterOptionsSchema = library.definition?.emitter?.options?.properties;
-  if (emitterOptionsSchema === undefined) {
-    return <>"No options"</>;
-  }
-  const entries = Object.entries(emitterOptionsSchema);
-
   const handleChange = useCallback(
     ({ name, value }: { name: string; value: unknown }) => {
       optionsChanged({
@@ -41,8 +35,15 @@ export const EmitterOptionsForm: FunctionComponent<EmitterOptionsFormProps> = ({
         },
       });
     },
-    [options, optionsChanged]
+    [library.name, options, optionsChanged]
   );
+
+  const emitterOptionsSchema = library.definition?.emitter?.options?.properties;
+  if (emitterOptionsSchema === undefined) {
+    return <>"No options"</>;
+  }
+  const entries = Object.entries(emitterOptionsSchema);
+
   return (
     <div className={style["form"]}>
       {entries.map(([key, value]) => {
@@ -90,7 +91,7 @@ const JsonSchemaPropertyInput: FunctionComponent<JsonSchemaPropertyInputProps> =
   const handleChange = useCallback(
     (_: unknown, data: RadioGroupOnChangeData | SwitchOnChangeData | InputOnChangeData) =>
       onChange({ name, value: "value" in data ? data.value : data.checked }),
-    [onChange]
+    [name, onChange]
   );
 
   switch (prop.type) {

diff --git a/packages/playground/src/react/settings/linter-form.tsx b/packages/playground/src/react/settings/linter-form.tsx
@@ -60,7 +60,7 @@ const RuleSetCheckbox = ({ ruleSet, checked, onChange }: RuleSetCheckbox) => {
     (_: any, data: CheckboxOnChangeData) => {
       onChange(ruleSet, !!data.checked);
     },
-    [ruleSet, checked, onChange]
+    [ruleSet, onChange]
   );
   return <Checkbox label={ruleSet} checked={checked} onChange={handleChange} />;
 };
diff --git a/packages/playground/src/react/standalone.tsx b/packages/playground/src/react/standalone.tsx
@@ -54,7 +54,7 @@ function useStandalonePlaygroundContext(
       setContext({ host, initialState, stateStorage });
     };
     void load();
-  }, []);
+  }, [config.importConfig, config.libraries]);
   return context;
 }
 
@@ -91,7 +91,7 @@ export const StandalonePlayground: FunctionComponent<ReactPlaygroundConfig> = (c
         defaultCompilerOptions: context.initialState.options,
         defaultSampleName: context.initialState.sampleName,
       },
-    [context]
+    [config.defaultEmitter, config.libraries, context]
   );
   if (context === undefined || fixedOptions === undefined) {
     return config.fallback;

diff --git a/packages/playground/src/react/typespec-editor.tsx b/packages/playground/src/react/typespec-editor.tsx
@@ -29,6 +29,7 @@ export const OutputEditor: FunctionComponent<{
   value: string;
   editorOptions?: PlaygroundEditorsOptions;
 }> = ({ filename, value, editorOptions }) => {
+  const model = useMonacoModel(filename);
   if (filename === "") {
     return null;
   }
@@ -40,7 +41,6 @@ export const OutputEditor: FunctionComponent<{
       enabled: false,
     },
   };
-  const model = useMonacoModel(filename);
   model.setValue(value);
   return <Editor model={model} options={options}></Editor>;
 };
diff --git a/packages/playground/src/react/viewers/react-wrapper.tsx b/packages/playground/src/react/viewers/react-wrapper.tsx
@@ -16,7 +16,7 @@ export default (props: { spec: string }) => {
       });
     }
     uiInstance.current.specActions.updateSpec(props.spec);
-  }, [uiRef.current, props.spec]);
+  }, [props.spec]);
 
   return <div ref={uiRef}></div>;
 };
diff --git a/packages/react-components/src/split-pane/split-pane.tsx b/packages/react-components/src/split-pane/split-pane.tsx
@@ -122,12 +122,12 @@ export const SplitPane: FunctionComponent<SplitPaneProps> = ({
 
       return res;
     },
-    [...resolvedPropSize, children.length, wrapSize]
+    [children, resolvedPropSize, wrapSize]
   );
 
   const sashPosSizes = useMemo(
     () => sizes.reduce((a, b) => [...a, a[a.length - 1] + b], [0]),
-    [...sizes]
+    [sizes]
   );
 
   const dragStart = useCallback(
@@ -185,7 +185,7 @@ export const SplitPane: FunctionComponent<SplitPaneProps> = ({
 
       updateSizes(nextSizes);
     },
-    [paneLimitSizes, onChange]
+    [splitAxis, sizes, paneLimitSizes, updateSizes]
   );
 
   const paneFollow = !(performanceMode && isDragging);