Use eslint. (#133)

Fixes #128.

A number of hook-related warnings were brought up, which this PR also fixes.  Note that from now on, if there are any warnings raised by the eslint, CI will fail.

.eslintrc

@@ -0,0 +1,14 @@
+{
+  "root": true,
+  "parser": "@typescript-eslint/parser",
+  "plugins": [
+    "@typescript-eslint",
+    "react-hooks"
+  ],
+  "extends": [
+    "plugin:react-hooks/recommended"
+  ],
+  "parserOptions": {
+    "sourceType": "module"
+  }
+}

.github/workflows/test.yml

@@ -22,7 +22,6 @@ jobs:
       with:
         node-version: ${{ matrix.node-version }}
     - run: npm ci
-    - run: npm run build -- --no-minify
-    - run: npm run prettier:check
-    - run: npm run typecheck
+    - run: npm run build
+    - run: npm run lint
     - run: npm test

lib/auto-sizing-svg.tsx

@@ -1,4 +1,4 @@
-import React, { useEffect, useRef, useState } from "react";
+import React, { useCallback, useEffect, useRef, useState } from "react";
 
 type AutoSizingSvgProps = {
   padding?: number;
@@ -31,7 +31,7 @@ export const AutoSizingSvg = React.forwardRef(
     const [width, setWidth] = useState(1);
     const [height, setHeight] = useState(1);
     const gRef = useRef<SVGGElement>(null);
-    const resizeToElement = () => {
+    const resizeToElement = useCallback(() => {
       if (sizeToElement?.current) {
         const bbox = sizeToElement.current.getBoundingClientRect();
         setX(-bbox.width / 2);
@@ -41,10 +41,14 @@ export const AutoSizingSvg = React.forwardRef(
         return true;
       }
       return false;
-    };
+    }, [sizeToElement]);
 
     useResizeHandler(resizeToElement);
 
+    // Note that we're passing `props.children` in as a dependency; it's not
+    // used anywhere in the effect, but since any change to the
+    // children may result in a dimension change in the SVG element, we
+    // want it to trigger the effect.
     useEffect(() => {
       if (!resizeToElement()) {
         const svgEl = gRef.current;
@@ -57,7 +61,7 @@ export const AutoSizingSvg = React.forwardRef(
           setHeight(bbox.height + padding * 2);
         }
       }
-    });
+    }, [props.padding, resizeToElement, props.children]);
 
     return (
       <svg

lib/browser-main.tsx

@@ -1,4 +1,4 @@
-import React, { useEffect, useState } from "react";
+import React, { useCallback, useEffect, useState } from "react";
 import ReactDOM from "react-dom";
 import { PageContext, PAGE_QUERY_ARG } from "./page";
 import { pageNames, Pages, toPageName, DEFAULT_PAGE } from "./pages";
@@ -30,15 +30,21 @@ function usePushState(onPushOrPopState: () => void) {
     };
   }, [onPushOrPopState]);
 
-  return function pushState(href: string) {
-    window.history.pushState(null, "", href);
-    onPushOrPopState();
-  };
+  return useCallback(
+    function pushState(href: string) {
+      window.history.pushState(null, "", href);
+      onPushOrPopState();
+    },
+    [onPushOrPopState]
+  );
 }
 
 const App: React.FC<{}> = (props) => {
   const [search, setSearch] = useState(getWindowSearch());
-  const updateSearchFromWindow = () => setSearch(getWindowSearch());
+  const updateSearchFromWindow = useCallback(
+    () => setSearch(getWindowSearch()),
+    []
+  );
   const currPage = toPageName(search.get(PAGE_QUERY_ARG) || "", DEFAULT_PAGE);
   const PageComponent = Pages[currPage];
   const pushState = usePushState(updateSearchFromWindow);

lib/page-with-shareable-state.tsx

@@ -1,4 +1,4 @@
-import React, { useContext, useEffect, useMemo, useState } from "react";
+import React, { useCallback, useContext, useEffect, useState } from "react";
 import { PageContext, PAGE_QUERY_ARG } from "./page";
 
 export type ComponentWithShareableStateProps<T> = {
@@ -82,8 +82,8 @@ export function createPageWithShareableState<T>({
       console.log(`Error deserializing state: ${e}`);
     }
 
-    const onChange = useMemo(
-      () => (value: T) => {
+    const onChange = useCallback(
+      (value: T) => {
         const newState = serialize(value);
         if (state !== newState) {
           const newSearch = new URLSearchParams();
@@ -95,7 +95,7 @@ export function createPageWithShareableState<T>({
           setIsInOnChange(false);
         }
       },
-      [state, currPage]
+      [state, currPage, pushState]
     );
 
     useEffect(() => {
@@ -104,7 +104,7 @@ export function createPageWithShareableState<T>({
         setLatestState(state);
         setKey(key + 1);
       }
-    });
+    }, [isInOnChange, state, latestState, key]);
 
     return <Component key={key} defaults={defaults} onChange={onChange} />;
   };

lib/pages/mandala-page/core.tsx

@@ -1,4 +1,4 @@
-import React, { useMemo, useRef, useState } from "react";
+import React, { useCallback, useMemo, useRef, useState } from "react";
 import { AutoSizingSvg } from "../../auto-sizing-svg";
 import { AnimationRenderer, ExportWidget } from "../../export-svg";
 import { HoverDebugHelper } from "../../hover-debug-helper";
@@ -336,7 +336,10 @@ export const MandalaPageWithDefaults: React.FC<{
   const isAnimated = isDesignAnimated(design);
   const render = useMemo(() => createAnimationRenderer(design), [design]);
 
-  useDebouncedEffect(250, () => onChange(design), [onChange, design]);
+  useDebouncedEffect(
+    250,
+    useCallback(() => onChange(design), [onChange, design])
+  );
 
   return (
     <Page title="Mandala!">

lib/unique-id.tsx

@@ -21,7 +21,7 @@ function useUniqueIds(count: number): string[] {
     }
 
     return result;
-  }, [count]);
+  }, [count, ctx]);
 
   return result;
 }
@@ -83,6 +83,6 @@ export function useUniqueIdMap(originalIds: string[]): UniqueIdMap {
 
   return useMemo(
     () => new UniqueIdMap(originalIds.map((id, i) => [id, uniqueIds[i]])),
-    [originalIds]
+    [originalIds, uniqueIds]
   );
 }

lib/use-debounced-effect.ts

@@ -2,17 +2,17 @@ import { useEffect } from "react";
 
 /**
  * Like useEffect(), but ensures that the effect is only
- * called when its dependencies haven't changed for the
+ * called when the callback hasn't changed for the
  * given number of milliseconds.
+ *
+ * Note that this means that the callback itself needs
+ * to be wrapped in something like `useCallback()`, or
+ * else it may never be called!
  */
-export function useDebouncedEffect(
-  ms: number,
-  effect: React.EffectCallback,
-  deps: React.DependencyList
-) {
+export function useDebouncedEffect(ms: number, effect: React.EffectCallback) {
   useEffect(() => {
     const timeout = setTimeout(effect, ms);
 
     return () => clearTimeout(timeout);
-  }, [...deps, ms]);
+  }, [effect, ms]);
 }

package.json

@@ -8,6 +8,8 @@
     "prettier:check": "prettier --check lib/**/*.ts lib/**/*.ts",
     "prettier:fix": "prettier --write   lib/**/*.ts lib/**/*.ts",
     "typecheck": "tsc --noemit",
+    "eslint": "eslint lib --ext .ts,.tsx --max-warnings 1",
+    "lint": "npm run prettier:check && npm run typecheck && npm run eslint",
     "test:watch": "jest --watch",
     "test": "jest",
     "esbuild": "esbuild lib/browser-main.tsx --bundle --outfile=dist/browser-main.js --external:stylus --define:global=window --inject:./lib/esbuild-shims.js --target=es2018 --sourcemap",
@@ -54,5 +56,10 @@
     "typescript": "^4.1.3",
     "util": "^0.12.3"
   },
-  "devDependencies": {}
+  "devDependencies": {
+    "@typescript-eslint/eslint-plugin": "^4.25.0",
+    "@typescript-eslint/parser": "^4.25.0",
+    "eslint": "^7.27.0",
+    "eslint-plugin-react-hooks": "^4.2.0"
+  }
 }