Fix ListView and add plugin-react-hooks.

wirmar · Feb 1, 2020 · 4e28042 · 4e28042
1 parent 17832cd
commit 4e28042
Show file tree

Hide file tree

Showing 3 changed files with 58 additions and 32 deletions.
diff --git a/.eslintignore b/.eslintignore
@@ -44,7 +44,6 @@ src/components/FoilOverlay.js
 src/components/GroupModal.js
 src/components/GroupModalContext.js
 src/components/ImageFallback.js
-src/components/ListView.js
 src/components/LoadingButton.js
 src/components/MassBuyButton.js
 src/components/MulticoloredAnalysis.js

diff --git a/.eslintrc.yml b/.eslintrc.yml
@@ -8,6 +8,7 @@ extends:
 plugins:
   - jest
   - prettier
+  - react-hooks
 settings:
   import/resolver:
     webpack:
@@ -40,6 +41,8 @@ rules:
   react/no-unescaped-entities:
     - error
     - forbid: ['>', '"', '}']
+  react-hooks/rules-of-hooks: error
+  react-hooks/exhaustive-deps: error
 overrides:
   - files: ['src/**']
     rules:

diff --git a/src/components/ListView.js b/src/components/ListView.js
@@ -1,4 +1,5 @@
 import React, { useCallback, useContext, useState, useEffect } from 'react';
+import PropTypes from 'prop-types';
 
 import { Form, Input } from 'reactstrap';
 
@@ -98,29 +99,29 @@ const ListViewRow = ({ card, versions, checked, onCheck, addAlert }) => {
         const json = await response.json();
         if (json.success === 'true') {
           const oldCardID = card.cardID;
-          card = { ...card, ...updated };
-          updateCubeCard(card.index, card);
+          const newCard = { ...card, ...updated };
+          updateCubeCard(card.index, newCard);
           if (updated.cardID !== oldCardID) {
             // changed version
             const getResponse = await fetch(`/cube/api/getcardfromid/${updated.cardID}`);
             const getJson = await getResponse.json();
-            updateCubeCard(card.index, { ...card, details: getJson.card });
+            updateCubeCard(card.index, { ...newCard, details: getJson.card });
           }
         }
       } catch (err) {
         addAlert('danger', 'Failed to send update request.');
         throw err;
       }
     },
-    [cubeID, card],
+    [cubeID, card, updateCubeCard, addAlert],
   );
 
   const addTag = useCallback(
     async (tag) => {
       const newTags = [...tags, tag];
       setTags(newTags);
       try {
-        await syncCard({ tags: newTags.map((tag) => tag.text) });
+        await syncCard({ tags: newTags.map((newTag) => newTag.text) });
       } catch (err) {
         setTags(tags);
       }
@@ -129,11 +130,11 @@ const ListViewRow = ({ card, versions, checked, onCheck, addAlert }) => {
   );
 
   const deleteTag = useCallback(
-    async (tagIndex) => {
-      const newTags = tags.filter((tag, index) => index !== tagIndex);
+    async (deleteIndex) => {
+      const newTags = tags.filter((tag, tagIndex) => tagIndex !== deleteIndex);
       setTags(newTags);
       try {
-        await syncCard({ tags: newTags.map((tag) => tag.text) });
+        await syncCard({ tags: newTags.map((newTag) => newTag.text) });
       } catch (err) {
         setTags(tags);
       }
@@ -148,7 +149,7 @@ const ListViewRow = ({ card, versions, checked, onCheck, addAlert }) => {
       newTags.splice(newIndex, 0, tag);
       setTags(newTags);
       try {
-        await syncCard({ tags: newTags.map((tag) => tag.text) });
+        await syncCard({ tags: newTags.map((newTag) => newTag.text) });
       } catch (err) {
         setTags(tags);
       }
@@ -170,16 +171,16 @@ const ListViewRow = ({ card, versions, checked, onCheck, addAlert }) => {
 
   const handleChange = useCallback(
     async (event) => {
-      const target = event.target;
+      const { target } = event;
       const value = target.type === 'checkbox' ? target.checked : target.value;
-      const name = target.name;
+      const { name, tagName } = target;
 
-      const updateF = (values) => ({
-        ...values,
+      const updateF = (currentValues) => ({
+        ...currentValues,
         [name]: value,
       });
 
-      if (target.tagName.toLowerCase() === 'select') {
+      if (tagName.toLowerCase() === 'select') {
         try {
           const updatedCard = {};
           if (name === 'colors') {
@@ -204,14 +205,12 @@ const ListViewRow = ({ card, versions, checked, onCheck, addAlert }) => {
 
   const handleBlur = useCallback(
     async (event) => {
-      const target = event.target;
-      const name = target.name;
-      const value = target.value;
+      const { target } = event;
+      const { name, value, tagName } = target;
 
       // <select>s handled in handleChange above.
-      if (target.tagName.toLowerCase() !== 'select') {
+      if (tagName.toLowerCase() !== 'select') {
         try {
-          // TODO: Apply some kind of loading indicator to the element.
           // Note: We can use this logic on all but the colors field, which is a select anyway so this path is irrelevant.
           await syncCard({
             [name]: value,
@@ -308,6 +307,25 @@ const ListViewRow = ({ card, versions, checked, onCheck, addAlert }) => {
   );
 };
 
+ListViewRow.propTypes = {
+  card: PropTypes.shape({
+    index: PropTypes.number.isRequired,
+    cardID: PropTypes.string.isRequired,
+    colors: PropTypes.arrayOf(PropTypes.oneOf([...'WUBRG'])).isRequired,
+    tags: PropTypes.arrayOf(PropTypes.string).isRequired,
+    details: PropTypes.shape({
+      name: PropTypes.string.isRequired,
+    }).isRequired,
+  }).isRequired,
+  versions: PropTypes.arrayOf(PropTypes.shape({
+    id: PropTypes.string.isRequired,
+    version: PropTypes.string.isRequired,
+  })).isRequired,
+  checked: PropTypes.bool.isRequired,
+  onCheck: PropTypes.func.isRequired,
+  addAlert: PropTypes.func.isRequired,
+};
+
 const ListView = ({ cards }) => {
   const [versionDict, setVersionDict] = useState({});
   const [checked, setChecked] = useState([]);
@@ -331,19 +349,19 @@ const ListView = ({ cards }) => {
           },
         });
         if (!response.ok) {
-          return console.error(response);
+          console.error(response);
+          return;
         }
 
         const json = await response.json();
-        setVersionDict((versionDict) => ({ ...versionDict, ...json.dict }));
+        setVersionDict((current) => ({ ...current, ...json.dict }));
       }
     };
     wrapper();
   }, [cards, versionDict]);
 
   const handleCheckAll = useCallback((event) => {
-    const target = event.target;
-    const value = target.checked;
+    const value = event.target.checked;
 
     if (value) {
       setChecked(cards.map(({ index }) => index));
@@ -352,13 +370,13 @@ const ListView = ({ cards }) => {
       setChecked([]);
       setGroupModalCards([]);
     }
-  }, []);
+  }, [cards, setGroupModalCards]);
 
   const handleCheck = useCallback(
     (event) => {
       const value = event.target.checked;
-      const index = parseInt(event.target.getAttribute('data-index'));
-      if (!isNaN(value)) {
+      const index = parseInt(event.target.getAttribute('data-index'), 10);
+      if (Number.isInteger(value)) {
         let newChecked = checked;
         if (value) {
           if (!newChecked.includes(index)) {
@@ -368,16 +386,16 @@ const ListView = ({ cards }) => {
           newChecked = checked.filter((testIndex) => testIndex !== index);
         }
         setChecked(newChecked);
-        setGroupModalCards(newChecked.map((index) => cards.find((card) => card.index === index)).filter((x) => x));
+        setGroupModalCards(newChecked.map((cardIndex) => cards.find((card) => card.index === cardIndex)).filter((x) => x));
       }
     },
-    [checked],
+    [checked, cards, setGroupModalCards],
   );
 
   const sorted = sortDeep(cards, primary, secondary);
 
-  const rows = sorted.map(([label1, group1]) =>
-    group1.map(([label2, group2]) =>
+  const rows = sorted.map(([, group1]) =>
+    group1.map(([, group2]) =>
       group2.map((card) => (
         <ListViewRow
           key={card._id}
@@ -391,7 +409,7 @@ const ListView = ({ cards }) => {
     ),
   );
 
-  const rowsFlat = [].concat.apply([], [].concat.apply([], rows));
+  const rowsFlat = [].concat(...[].concat(...rows));
 
   return (
     <>
@@ -419,4 +437,10 @@ const ListView = ({ cards }) => {
   );
 };
 
+ListView.propTypes = {
+  cards: PropTypes.arrayOf(PropTypes.shape({
+    _id: PropTypes.string.isRequired,
+  })).isRequired,
+};
+
 export default ListView;