Fix misuse of Hooks with useRef (#24)

Reviewed-on: https://git.siklos-chaneru.duckdns.org/Siklos/svg-layout-designer-react/pulls/24

.eslintrc.cjs

@@ -19,6 +19,7 @@ module.exports = {
   plugins: [
     'only-warn',
     'react',
+    'react-hooks',
     '@typescript-eslint'
   ],
   rules: {
@@ -29,5 +30,7 @@ module.exports = {
     '@typescript-eslint/semi': ['warn', 'always'],
     'no-unused-vars': 'off',
     '@typescript-eslint/no-unused-vars': 'error',
+    'react-hooks/rules-of-hooks': 'error', // Checks rules of Hooks
+    'react-hooks/exhaustive-deps': 'warn' // Checks effect dependencies
   }
 };

package-lock.json

@@ -15,6 +15,7 @@
         "react-svg-pan-zoom": "^3.11.0"
       },
       "devDependencies": {
+        "@testing-library/dom": "^8.16.1",
         "@testing-library/jest-dom": "^5.16.4",
         "@testing-library/react": "^13.3.0",
         "@testing-library/user-event": "^14.4.1",
@@ -31,8 +32,10 @@
         "eslint-config-standard-with-typescript": "^22.0.0",
         "eslint-plugin-import": "^2.26.0",
         "eslint-plugin-n": "^15.2.4",
+        "eslint-plugin-only-warn": "^1.0.3",
         "eslint-plugin-promise": "^6.0.0",
         "eslint-plugin-react": "^7.30.1",
+        "eslint-plugin-react-hooks": "^4.6.0",
         "jsdom": "^20.0.0",
         "postcss": "^8.4.14",
         "sass": "^1.54.0",
@@ -745,9 +748,9 @@
       "dev": true
     },
     "node_modules/@testing-library/dom": {
-      "version": "8.16.0",
+      "version": "8.17.1",
-      "resolved": "https://registry.npmjs.org/@testing-library/dom/-/dom-8.16.0.tgz",
+      "resolved": "https://registry.npmjs.org/@testing-library/dom/-/dom-8.17.1.tgz",
-      "integrity": "sha512-uxF4zmnLHHDlmW4l+0WDjcgLVwCvH+OVLpD8Dfp+Bjfz85prwxWGbwXgJdLtkgjD0qfOzkJF9SmA6YZPsMYX4w==",
+      "integrity": "sha512-KnH2MnJUzmFNPW6RIKfd+zf2Wue8mEKX0M3cpX6aKl5ZXrJM1/c/Pc8c2xDNYQCnJO48Sm5ITbMXgqTr3h4jxQ==",
       "dev": true,
       "dependencies": {
         "@babel/code-frame": "^7.10.4",
@@ -2879,6 +2882,15 @@
         "eslint": ">=7.0.0"
       }
     },
+    "node_modules/eslint-plugin-only-warn": {
+      "version": "1.0.3",
+      "resolved": "https://registry.npmjs.org/eslint-plugin-only-warn/-/eslint-plugin-only-warn-1.0.3.tgz",
+      "integrity": "sha512-XQOX/TfLoLw6h8ky51d29uUjXRTQHqBGXPylDEmy5fe/w7LIOnp8MA24b1OSMEn9BQoKow1q3g1kLe5/9uBTvw==",
+      "dev": true,
+      "engines": {
+        "node": ">=6"
+      }
+    },
     "node_modules/eslint-plugin-promise": {
       "version": "6.0.0",
       "resolved": "https://registry.npmjs.org/eslint-plugin-promise/-/eslint-plugin-promise-6.0.0.tgz",
@@ -2919,6 +2931,18 @@
         "eslint": "^3 || ^4 || ^5 || ^6 || ^7 || ^8"
       }
     },
+    "node_modules/eslint-plugin-react-hooks": {
+      "version": "4.6.0",
+      "resolved": "https://registry.npmjs.org/eslint-plugin-react-hooks/-/eslint-plugin-react-hooks-4.6.0.tgz",
+      "integrity": "sha512-oFc7Itz9Qxh2x4gNHStv3BqJq54ExXmfC+a1NjAta66IAN87Wu0R/QArgIS9qKzX3dXKPI9H5crl9QchNMY9+g==",
+      "dev": true,
+      "engines": {
+        "node": ">=10"
+      },
+      "peerDependencies": {
+        "eslint": "^3.0.0 || ^4.0.0 || ^5.0.0 || ^6.0.0 || ^7.0.0 || ^8.0.0-0"
+      }
+    },
     "node_modules/eslint-plugin-react/node_modules/doctrine": {
       "version": "2.1.0",
       "resolved": "https://registry.npmjs.org/doctrine/-/doctrine-2.1.0.tgz",
@@ -6708,9 +6732,9 @@
       "dev": true
     },
     "@testing-library/dom": {
-      "version": "8.16.0",
+      "version": "8.17.1",
-      "resolved": "https://registry.npmjs.org/@testing-library/dom/-/dom-8.16.0.tgz",
+      "resolved": "https://registry.npmjs.org/@testing-library/dom/-/dom-8.17.1.tgz",
-      "integrity": "sha512-uxF4zmnLHHDlmW4l+0WDjcgLVwCvH+OVLpD8Dfp+Bjfz85prwxWGbwXgJdLtkgjD0qfOzkJF9SmA6YZPsMYX4w==",
+      "integrity": "sha512-KnH2MnJUzmFNPW6RIKfd+zf2Wue8mEKX0M3cpX6aKl5ZXrJM1/c/Pc8c2xDNYQCnJO48Sm5ITbMXgqTr3h4jxQ==",
       "dev": true,
       "requires": {
         "@babel/code-frame": "^7.10.4",
@@ -8254,6 +8278,12 @@
         "semver": "^7.3.7"
       }
     },
+    "eslint-plugin-only-warn": {
+      "version": "1.0.3",
+      "resolved": "https://registry.npmjs.org/eslint-plugin-only-warn/-/eslint-plugin-only-warn-1.0.3.tgz",
+      "integrity": "sha512-XQOX/TfLoLw6h8ky51d29uUjXRTQHqBGXPylDEmy5fe/w7LIOnp8MA24b1OSMEn9BQoKow1q3g1kLe5/9uBTvw==",
+      "dev": true
+    },
     "eslint-plugin-promise": {
       "version": "6.0.0",
       "resolved": "https://registry.npmjs.org/eslint-plugin-promise/-/eslint-plugin-promise-6.0.0.tgz",
@@ -8311,6 +8341,13 @@
         }
       }
     },
+    "eslint-plugin-react-hooks": {
+      "version": "4.6.0",
+      "resolved": "https://registry.npmjs.org/eslint-plugin-react-hooks/-/eslint-plugin-react-hooks-4.6.0.tgz",
+      "integrity": "sha512-oFc7Itz9Qxh2x4gNHStv3BqJq54ExXmfC+a1NjAta66IAN87Wu0R/QArgIS9qKzX3dXKPI9H5crl9QchNMY9+g==",
+      "dev": true,
+      "requires": {}
+    },
     "eslint-scope": {
       "version": "5.1.1",
       "resolved": "https://registry.npmjs.org/eslint-scope/-/eslint-scope-5.1.1.tgz",

package.json

@@ -40,6 +40,7 @@
     "eslint-plugin-only-warn": "^1.0.3",
     "eslint-plugin-promise": "^6.0.0",
     "eslint-plugin-react": "^7.30.1",
+    "eslint-plugin-react-hooks": "^4.6.0",
     "jsdom": "^20.0.0",
     "postcss": "^8.4.14",
     "sass": "^1.54.0",

pnpm-lock.yaml

@@ -22,6 +22,7 @@ specifiers:
   eslint-plugin-only-warn: ^1.0.3
   eslint-plugin-promise: ^6.0.0
   eslint-plugin-react: ^7.30.1
+  eslint-plugin-react-hooks: ^4.6.0
   framer-motion: ^6.5.1
   jsdom: ^20.0.0
   postcss: ^8.4.14
@@ -62,6 +63,7 @@ devDependencies:
   eslint-plugin-only-warn: 1.0.3
   eslint-plugin-promise: 6.0.0_eslint@8.21.0
   eslint-plugin-react: 7.30.1_eslint@8.21.0
+  eslint-plugin-react-hooks: 4.6.0_eslint@8.21.0
   jsdom: 20.0.0
   postcss: 8.4.16
   sass: 1.54.3
@@ -1740,6 +1742,15 @@ packages:
       eslint: 8.21.0
     dev: true
 
+  /eslint-plugin-react-hooks/4.6.0_eslint@8.21.0:
+    resolution: {integrity: sha512-oFc7Itz9Qxh2x4gNHStv3BqJq54ExXmfC+a1NjAta66IAN87Wu0R/QArgIS9qKzX3dXKPI9H5crl9QchNMY9+g==}
+    engines: {node: '>=10'}
+    peerDependencies:
+      eslint: ^3.0.0 || ^4.0.0 || ^5.0.0 || ^6.0.0 || ^7.0.0 || ^8.0.0-0
+    dependencies:
+      eslint: 8.21.0
+    dev: true
+
   /eslint-plugin-react/7.30.1_eslint@8.21.0:
     resolution: {integrity: sha512-NbEvI9jtqO46yJA3wcRF9Mo0lF9T/jhdHqhCHXiXtD+Zcb98812wvokjWpU7Q4QH5edo6dmqrukxVvWWXHlsUg==}
     engines: {node: '>=4'}

src/Components/Editor/ContainerOperations.ts

@@ -5,6 +5,7 @@ import { ContainerModel, IContainerModel } from '../../Interfaces/ContainerModel
 import { findContainerById } from '../../utils/itertools';
 import { getCurrentHistory } from './Editor';
 import { SizePointer } from '../../Interfaces/SizePointer';
+import Properties from '../../Interfaces/Properties';
 
 /**
  * Select a container
@@ -274,7 +275,7 @@ export function OnPropertyChange(
   */
 export function OnPropertiesSubmit(
   event: React.SyntheticEvent<HTMLFormElement>,
-  refs: Array<React.RefObject<HTMLInputElement>>,
+  properties: Properties,
   fullHistory: HistoryState[],
   historyCurrentStep: number,
   setHistory: Dispatch<SetStateAction<HistoryState[]>>,
@@ -291,10 +292,10 @@ export function OnPropertiesSubmit(
 
   if (parent === null) {
     const selectedContainerClone: IContainerModel = structuredClone(current.SelectedContainer);
-    for (const ref of refs) {
+    for (const property in properties) {
-      const input = ref.current;
+      const input = (event.target as HTMLFormElement).querySelector(`#${property}`);
       if (input instanceof HTMLInputElement) {
-        (selectedContainerClone.properties as any)[input.id] = input.value;
+        (selectedContainerClone.properties as any)[property] = input.value;
       }
     }
     setHistory(history.concat([{
@@ -315,10 +316,10 @@ export function OnPropertiesSubmit(
     throw new Error('[OnPropertyChange] Container model was not found among children of the main container!');
   }
 
-  for (const ref of refs) {
+  for (const property in properties) {
-    const input = ref.current;
+    const input = (event.target as HTMLFormElement).querySelector(`#${property}`);
     if (input instanceof HTMLInputElement) {
-      (container.properties as any)[input.id] = input.value;
+      (container.properties as any)[property] = input.value;
     }
   }
 

src/Components/Editor/Editor.tsx

@@ -72,9 +72,9 @@ const Editor: React.FunctionComponent<IEditorProps> = (props) => {
           setHistory,
           setHistoryCurrentStep
         )}
-        OnPropertiesSubmit={(event, refs) => OnPropertiesSubmit(
+        OnPropertiesSubmit={(event, properties) => OnPropertiesSubmit(
           event,
-          refs,
+          properties,
           history,
           historyCurrentStep,
           setHistory,

src/Components/ElementsSidebar/ElementsSidebar.tsx

@@ -1,6 +1,7 @@
 import * as React from 'react';
 import { motion } from 'framer-motion';
 import { Properties } from '../Properties/Properties';
+import ContainerProperties from '../../Interfaces/Properties';
 import { IContainerModel } from '../../Interfaces/ContainerModel';
 import { getDepth, MakeIterator } from '../../utils/itertools';
 import { Menu } from '../Menu/Menu';
@@ -8,13 +9,14 @@ import { MenuItem } from '../Menu/MenuItem';
 import { handleDragLeave, handleDragOver, handleLeftClick, handleOnDrop, handleRightClick } from './MouseEventHandlers';
 import { Point } from '../../Interfaces/Point';
 
+
 interface IElementsSidebarProps {
   MainContainer: IContainerModel
   isOpen: boolean
   isHistoryOpen: boolean
   SelectedContainer: IContainerModel | null
   OnPropertyChange: (key: string, value: string | number | boolean) => void
-  OnPropertiesSubmit: (event: React.FormEvent<HTMLFormElement>, refs: Array<React.RefObject<HTMLInputElement>>) => void
+  OnPropertiesSubmit: (event: React.FormEvent<HTMLFormElement>, properties: ContainerProperties) => void
   SelectContainer: (container: IContainerModel) => void
   DeleteContainer: (containerid: string) => void
   AddContainer: (index: number, type: string, parent: string) => void

src/Components/Properties/Properties.tsx

@@ -6,7 +6,7 @@ import { INPUT_TYPES } from './PropertiesInputTypes';
 interface IPropertiesProps {
   properties?: ContainerProperties
   onChange: (key: string, value: string | number | boolean) => void
-  onSubmit: (event: React.FormEvent<HTMLFormElement>, refs: Array<React.RefObject<HTMLInputElement>>) => void
+  onSubmit: (event: React.FormEvent<HTMLFormElement>, properties: ContainerProperties) => void
 }
 
 export const Properties: React.FC<IPropertiesProps> = (props: IPropertiesProps) => {
@@ -17,10 +17,9 @@ export const Properties: React.FC<IPropertiesProps> = (props: IPropertiesProps)
   }
 
   const groupInput: React.ReactNode[] = [];
-  const refs: Array<React.RefObject<HTMLInputElement>> = [];
   Object
     .entries(props.properties)
-    .forEach((pair) => handleProperties(pair, groupInput, refs, isDynamicInput, props.onChange));
+    .forEach((pair) => handleProperties(pair, groupInput, isDynamicInput, props.onChange));
 
   const form = isDynamicInput
     ? <div>
@@ -28,7 +27,7 @@ export const Properties: React.FC<IPropertiesProps> = (props: IPropertiesProps)
     </div>
     : <form
       key={props.properties.id}
-      onSubmit={(event) => props.onSubmit(event, refs)}
+      onSubmit={(event) => props.onSubmit(event, props.properties as ContainerProperties)}
     >
       <input type='submit' className='normal-btn block mx-auto' value='Submit'/>
       { groupInput }
@@ -52,7 +51,6 @@ export const Properties: React.FC<IPropertiesProps> = (props: IPropertiesProps)
 const handleProperties = (
   [key, value]: [string, string | number],
   groupInput: React.ReactNode[],
-  refs: Array<React.RefObject<HTMLInputElement>>,
   isDynamicInput: boolean,
   onChange: (key: string, value: string | number | boolean) => void
 ): void => {
@@ -69,9 +67,6 @@ const handleProperties = (
     type = INPUT_TYPES[key];
   }
 
-  const ref: React.RefObject<HTMLInputElement> = React.useRef<HTMLInputElement>(null);
-  refs.push(ref);
-
   const isDisabled = ['id', 'parentId'].includes(key);
   const input = isDynamicInput
     ? <input
@@ -82,7 +77,6 @@ const handleProperties = (
          '
       type={type}
       id={key}
-      ref={ref}
       value={value}
       checked={checked}
       onChange={(event) => {
@@ -102,7 +96,6 @@ const handleProperties = (
          '
       type={type}
       id={key}
-      ref={ref}
       defaultValue={value}
       defaultChecked={checked}
       disabled={isDisabled}

src/Components/UI/UI.tsx

@@ -8,6 +8,7 @@ import { HistoryState } from '../../Interfaces/HistoryState';
 import { PhotographIcon, UploadIcon } from '@heroicons/react/outline';
 import { FloatingButton } from '../FloatingButton/FloatingButton';
 import { Bar } from '../Bar/Bar';
+import Properties from '../../Interfaces/Properties';
 
 interface IUIProps {
   current: HistoryState
@@ -17,7 +18,7 @@ interface IUIProps {
   SelectContainer: (container: ContainerModel) => void
   DeleteContainer: (containerId: string) => void
   OnPropertyChange: (key: string, value: string | number | boolean) => void
-  OnPropertiesSubmit: (event: React.FormEvent<HTMLFormElement>, refs: Array<React.RefObject<HTMLInputElement>>) => void
+  OnPropertiesSubmit: (event: React.FormEvent<HTMLFormElement>, properties: Properties) => void
   AddContainerToSelectedContainer: (type: string) => void
   AddContainer: (index: number, type: string, parentId: string) => void
   SaveEditorAsJSON: () => void