Merge pull request #758 from mozilla/feature/eslint-hooks

eslint react hooks
Hubs-Foundation · Sep 16, 2019 · 6363cfc · 6363cfc
2 parents ac65be2 + c3e2244
commit 6363cfc
Show file tree

Hide file tree

Showing 10 changed files with 87 additions and 43 deletions.
diff --git a/.eslintrc.js b/.eslintrc.js
@@ -6,7 +6,7 @@ module.exports = {
     node: true,
     mocha: true
   },
-  plugins: ["prettier", "react"],
+  plugins: ["prettier", "react", "react-hooks"],
   rules: {
     "prettier/prettier": "error",
     "prefer-const": "error",
@@ -28,7 +28,30 @@ module.exports = {
       { blankLine: "always", prev: "import", next: "function" },
       { blankLine: "always", prev: "import", next: "class" }
     ],
-    "no-unused-vars": ["error", { varsIgnorePattern: "^_", argsIgnorePattern: "^_", ignoreRestSiblings: true }]
+    "no-unused-vars": ["error", { varsIgnorePattern: "^_", argsIgnorePattern: "^_", ignoreRestSiblings: true }],
+    "react-hooks/rules-of-hooks": "error",
+    "react-hooks/exhaustive-deps": "warn",
+    "require-atomic-updates": "warn",
+    "no-prototype-builtins": "warn"
   },
-  extends: ["prettier", "plugin:react/recommended", "eslint:recommended"]
+  extends: ["prettier", "plugin:react/recommended", "eslint:recommended"],
+  settings: {
+    react: {
+      createClass: "createReactClass", // Regex for Component Factory to use,
+      // default to "createReactClass"
+      pragma: "React", // Pragma to use, default to "React"
+      version: "detect", // React version. "detect" automatically picks the version you have installed.
+      // You can also use `16.0`, `16.3`, etc, if you want to override the detected value.
+      // default to latest and warns if missing
+      // It will default to "detect" in the future
+      flowVersion: "0.53" // Flow version
+    },
+    propWrapperFunctions: [
+      // The names of any function used to wrap propTypes, e.g. `forbidExtraProps`. If this isn't set, any propTypes wrapped in a function will be skipped.
+    ],
+    linkComponents: [
+      // Components used as alternatives to <a> for linking, eg. <Link to={ url } />
+      { name: "Link", linkAttribute: "to" }
+    ]
+  }
 };
diff --git a/package.json b/package.json
@@ -25,6 +25,7 @@
     "lint": "yarn lint:js && yarn lint:css",
     "lint:js": "eslint ./src",
     "lint:css": "stylelint './src/**/*.js'",
+    "pretest": "yarn lint",
     "test": "concurrently --success \"first\" --kill-others \"yarn test-all\" \"yarn test-server\"",
     "test-server": "cross-env NODE_ENV=test webpack-dev-server --mode development --port 9091 --content-base ./test/fixtures",
     "test-all": "ava",
@@ -113,6 +114,7 @@
     "eslint-config-prettier": "^6.3.0",
     "eslint-plugin-prettier": "^3.1.0",
     "eslint-plugin-react": "^7.12.4",
+    "eslint-plugin-react-hooks": "^2.0.1",
     "esm": "^3.2.25",
     "file-loader": "^4.2.0",
     "get-chrome": "0.0.2",
@@ -136,4 +138,4 @@
     "webpack-dev-server": "^3.8.0",
     "worker-loader": "^2.0.0"
   }
-}
+}
diff --git a/src/api/Api.js b/src/api/Api.js
@@ -872,8 +872,6 @@ export default class Project extends EventEmitter {
           hideDialog();
         }
       });
-    } catch (e) {
-      throw e;
     } finally {
       if (screenshotUrl) {
         URL.revokeObjectURL(screenshotUrl);

diff --git a/src/api/SketchfabZipLoader.js b/src/api/SketchfabZipLoader.js
@@ -1,7 +1,7 @@
 import SketchfabZipWorker from "./SketchfabZipLoader.worker.js";
 
 export async function getFilesFromSketchfabZip(src) {
-  return new Promise(async (resolve, reject) => {
+  return new Promise((resolve, reject) => {
     const worker = new SketchfabZipWorker();
     worker.onmessage = e => {
       const [success, fileMapOrError] = e.data;

diff --git a/src/ui/dialogs/SaveNewProjectDialog.js b/src/ui/dialogs/SaveNewProjectDialog.js
@@ -7,19 +7,28 @@ import PreviewDialog from "./PreviewDialog";
 export default function SaveNewProjectDialog({ thumbnailUrl, initialName, onConfirm, onCancel }) {
   const [name, setName] = useState(initialName);
 
-  const onChangeName = useCallback(value => {
-    setName(value);
-  });
+  const onChangeName = useCallback(
+    value => {
+      setName(value);
+    },
+    [setName]
+  );
 
-  const onConfirmCallback = useCallback(e => {
-    e.preventDefault();
-    onConfirm({ name });
-  });
+  const onConfirmCallback = useCallback(
+    e => {
+      e.preventDefault();
+      onConfirm({ name });
+    },
+    [name, onConfirm]
+  );
 
-  const onCancelCallback = useCallback(e => {
-    e.preventDefault();
-    onCancel();
-  });
+  const onCancelCallback = useCallback(
+    e => {
+      e.preventDefault();
+      onCancel();
+    },
+    [onCancel]
+  );
 
   return (
     <PreviewDialog

diff --git a/src/ui/hierarchy/HierarchyPanelContainer.js b/src/ui/hierarchy/HierarchyPanelContainer.js
@@ -169,40 +169,41 @@ function isAncestor(object, otherObject) {
 
 function TreeNode(props) {
   const { node, ...rest } = props;
+  const { onKeyDown, onToggle, onRenameSubmit } = props;
 
   const editor = useContext(EditorContext);
 
-  const onToggle = useCallback(
+  const onClickToggle = useCallback(
     e => {
       e.stopPropagation();
 
-      if (props.onToggle) {
-        props.onToggle(e, node);
+      if (onToggle) {
+        onToggle(e, node);
       }
     },
-    [props.onToggle, node]
+    [onToggle, node]
   );
 
-  const onKeyDown = useCallback(
+  const onNodeKeyDown = useCallback(
     e => {
       e.stopPropagation();
 
-      if (props.onKeyDown) {
-        props.onKeyDown(e, node);
+      if (onKeyDown) {
+        onKeyDown(e, node);
       }
     },
-    [props.onKeyDown, node]
+    [onKeyDown, node]
   );
 
   const onKeyDownNameInput = useCallback(
     e => {
       if (e.key === "Escape") {
-        props.onRenameSubmit(node, null);
+        onRenameSubmit(node, null);
       } else if (e.key === "Enter") {
-        props.onRenameSubmit(node, e.target.value);
+        onRenameSubmit(node, e.target.value);
       }
     },
-    [props.onRenameSubmit, node]
+    [onRenameSubmit, node]
   );
 
   const renaming = props.renamingNode && props.renamingNode.id === node.id;
@@ -224,7 +225,7 @@ function TreeNode(props) {
 
   useEffect(() => {
     preview(getEmptyImage(), { captureDraggingState: true });
-  }, []);
+  }, [preview]);
 
   const [{ canDropBefore, isOverBefore }, beforeDropTarget] = useDrop({
     accept: ItemTypes.Node,
@@ -305,7 +306,7 @@ function TreeNode(props) {
           onMouseDown={e => props.onMouseDown(e, node)}
           onClick={e => props.onClick(e, node)}
           tabIndex="0"
-          onKeyDown={onKeyDown}
+          onKeyDown={onNodeKeyDown}
           root={node.depth === 0}
           selected={node.selected}
           active={node.active}
@@ -321,7 +322,7 @@ function TreeNode(props) {
             {node.leaf ? (
               <TreeNodeLeafSpacer />
             ) : (
-              <TreeNodeToggle collapsed={collapsed} onClick={onToggle}>
+              <TreeNodeToggle collapsed={collapsed} onClick={onClickToggle}>
                 {collapsed ? <CaretRight size={12} /> : <CaretDown size={12} />}
               </TreeNodeToggle>
             )}
@@ -481,15 +482,15 @@ export default function HierarchyPanel() {
 
   const onExpandAllNodes = useCallback(() => {
     setCollapsedNodes({});
-  });
+  }, [setCollapsedNodes]);
 
   const onCollapseAllNodes = useCallback(() => {
     const newCollapsedNodes = {};
     traverse(sceneRootNode, child => {
       newCollapsedNodes[child.id] = true;
     });
     setCollapsedNodes(newCollapsedNodes);
-  });
+  }, [sceneRootNode, setCollapsedNodes]);
 
   const onObjectChanged = useCallback(
     (objects, propertyName) => {
@@ -510,7 +511,7 @@ export default function HierarchyPanel() {
       editor.removeListener("selectionChanged", updateNodeHierarchy);
       editor.removeListener("objectsChanged", onObjectChanged);
     };
-  }, [editor, updateNodeHierarchy]);
+  }, [editor, updateNodeHierarchy, onObjectChanged]);
 
   const onMouseDown = useCallback(
     (e, node) => {
@@ -544,7 +545,7 @@ export default function HierarchyPanel() {
         collapseNode(node);
       }
     },
-    [collapsedNodes]
+    [collapsedNodes, expandNode, collapseNode]
   );
 
   const onKeyDown = useCallback(

diff --git a/src/ui/layout/Popover.js b/src/ui/layout/Popover.js
@@ -22,15 +22,19 @@ export default function Popover({ children, padding, position, renderContent, ..
 
   const onPreventClose = useCallback(e => {
     e.stopPropagation();
-  });
+  }, []);
+
+  const getTargetRef = useCallback(() => {
+    return popoverTriggerRef;
+  }, [popoverTriggerRef]);
 
   return (
     <div ref={popoverTriggerRef} onClick={onOpen} {...rest}>
       {children}
       {isOpen && (
         <Portal>
           <Overlay onClick={onClose} />
-          <Positioner onClick={onPreventClose} targetRef={popoverTriggerRef} padding={padding} position={position}>
+          <Positioner onClick={onPreventClose} getTargetRef={getTargetRef} padding={padding} position={position}>
             {renderContent({ onClose })}
           </Positioner>
         </Portal>

diff --git a/src/ui/layout/Positioner.js b/src/ui/layout/Positioner.js
@@ -15,7 +15,7 @@ const PositionerContainer = styled.div.attrs(({ transform, transformOrigin, opac
   left: 0;
 `;
 
-export default function Positioner({ children, position, padding, targetRef, ...rest }) {
+export default function Positioner({ children, position, padding, getTargetRef, ...rest }) {
   const positionerContainerRef = useRef();
 
   const [transformProps, setTransformProps] = useState({
@@ -28,7 +28,7 @@ export default function Positioner({ children, position, padding, targetRef, ...
   useEffect(() => {
     const onReposition = () => {
       const positionerContainerRect = positionerContainerRef.current.getBoundingClientRect();
-      const targetRect = targetRef.current.getBoundingClientRect();
+      const targetRect = getTargetRef().current.getBoundingClientRect();
       const viewportHeight = document.documentElement.clientHeight;
       const viewportWidth = document.documentElement.clientWidth;
 
@@ -56,7 +56,7 @@ export default function Positioner({ children, position, padding, targetRef, ...
     return () => {
       window.removeEventListener("resize", onReposition);
     };
-  }, [position, padding, targetRef.current, setTransformProps]);
+  }, [position, padding, getTargetRef, setTransformProps]);
 
   const childrenWithProps = Children.map(children, child =>
     cloneElement(child, { position: transformProps.finalPosition })
@@ -73,7 +73,7 @@ Positioner.propTypes = {
   children: PropTypes.node,
   position: PropTypes.string,
   padding: PropTypes.number,
-  targetRef: PropTypes.shape({ current: PropTypes.object })
+  getTargetRef: PropTypes.func
 };
 
 Positioner.defaultProps = {

diff --git a/src/ui/library/LibrarySearchContainer.js b/src/ui/library/LibrarySearchContainer.js
@@ -49,7 +49,9 @@ class LibrarySearchContainer extends Component {
         label: PropTypes.string.isRequired,
         toolbar: PropTypes.elementType,
         toolbarProps: PropTypes.object,
-        onSearch: PropTypes.func
+        onSearch: PropTypes.func,
+        defaultFilter: PropTypes.string,
+        defaultType: PropTypes.string
       })
     ).isRequired,
     onSelect: PropTypes.func.isRequired,

diff --git a/yarn.lock b/yarn.lock
@@ -3742,6 +3742,11 @@ eslint-plugin-prettier@^3.1.0:
   dependencies:
     prettier-linter-helpers "^1.0.0"
 
+eslint-plugin-react-hooks@^2.0.1:
+  version "2.0.1"
+  resolved "https://registry.yarnpkg.com/eslint-plugin-react-hooks/-/eslint-plugin-react-hooks-2.0.1.tgz#e898ec26a0a335af6f7b0ad1f0bedda7143ed756"
+  integrity sha512-xir+3KHKo86AasxlCV8AHRtIZPHljqCRRUYgASkbatmt0fad4+5GgC7zkT7o/06hdKM6MIwp8giHVXqBPaarHQ==
+
 eslint-plugin-react@^7.12.4:
   version "7.14.3"
   resolved "https://registry.yarnpkg.com/eslint-plugin-react/-/eslint-plugin-react-7.14.3.tgz#911030dd7e98ba49e1b2208599571846a66bdf13"