fix enable rules of hooks (#329)

This is a PR enabling eslint "rules-of-hooks".

This lint rule catches some very annoying bugs and enforces you to use correct naming for custom hooks (prepend with "use"), which is a mandatory react rule and important for a number of reasons.

I added eslint-disable statements wherever the rules are broken, and if this is merged, I would expect new code not to break the rules any longer.

I strongly suggest that the much better way to extract and reuse hooks and logic from components is the way the community does it and the React docs suggest. The new React docs are really fantastic in this regard and you can use the patterns found there very well to have an excellent application. https://react.dev/learn/reusing-logic-with-custom-hooks

.eslintrc.js

@@ -7,7 +7,7 @@ const config = createConfig('eslint', {
     'import/no-named-as-default-member': 'off',
     'import/no-self-import': 'off',
     'spaced-comment': ['error', 'always', { block: { exceptions: ['*'] } }],
-    'react-hooks/rules-of-hooks': 'off',
+    'react-hooks/rules-of-hooks': 2,
     'react-hooks/exhaustive-deps': 'off',
     'no-promise-executor-return': 'off',
     radix: 'off',

src/editors/containers/EditorContainer/components/TitleHeader/hooks.js

@@ -8,11 +8,13 @@ import * as module from './hooks';
 export const { navigateCallback } = textEditorHooks;
 
 export const state = {
+  // eslint-disable-next-line react-hooks/rules-of-hooks
   localTitle: (args) => React.useState(args),
 };
 
 export const hooks = {
   isEditing: () => {
+  // eslint-disable-next-line react-hooks/rules-of-hooks
     const [isEditing, setIsEditing] = React.useState(false);
     return {
       isEditing,
@@ -22,6 +24,7 @@ export const hooks = {
   },
 
   localTitle: ({ dispatch, stopEditing }) => {
+  // eslint-disable-next-line react-hooks/rules-of-hooks
     const title = useSelector(selectors.app.displayTitle);
     const [localTitle, setLocalTitle] = module.state.localTitle(title);
     return {

src/editors/containers/EditorContainer/components/TitleHeader/index.jsx

@@ -17,7 +17,9 @@ export const TitleHeader = ({
   intl,
 }) => {
   if (!isInitialized) { return intl.formatMessage(messages.loading); }
+  // eslint-disable-next-line react-hooks/rules-of-hooks
   const dispatch = useDispatch();
+  // eslint-disable-next-line react-hooks/rules-of-hooks
   const title = useSelector(selectors.app.displayTitle);
 
   const {

src/editors/containers/EditorContainer/hooks.js

@@ -16,6 +16,7 @@ export const {
 } = appHooks;
 
 export const state = StrictDict({
+  // eslint-disable-next-line react-hooks/rules-of-hooks
   isCancelConfirmModalOpen: (val) => useState(val),
 });
 
@@ -25,7 +26,9 @@ export const handleSaveClicked = ({
   validateEntry,
   returnFunction,
 }) => {
+  // eslint-disable-next-line react-hooks/rules-of-hooks
   const destination = useSelector(selectors.app.returnUrl);
+  // eslint-disable-next-line react-hooks/rules-of-hooks
   const analytics = useSelector(selectors.app.analytics);
 
   return () => saveBlock({
@@ -53,14 +56,18 @@ export const handleCancel = ({ onClose, returnFunction }) => {
   }
   return navigateCallback({
     returnFunction,
+    // eslint-disable-next-line react-hooks/rules-of-hooks
     destination: useSelector(selectors.app.returnUrl),
     analyticsEvent: analyticsEvt.editorCancelClick,
+    // eslint-disable-next-line react-hooks/rules-of-hooks
     analytics: useSelector(selectors.app.analytics),
   });
 };
 
+// eslint-disable-next-line react-hooks/rules-of-hooks
 export const isInitialized = () => useSelector(selectors.app.isInitialized);
 
+// eslint-disable-next-line react-hooks/rules-of-hooks
 export const saveFailed = () => useSelector((rootState) => (
   selectors.requests.isFailed(rootState, { requestKey: RequestKeys.saveBlock })
 ));

src/editors/containers/ProblemEditor/components/EditProblemView/AnswerWidget/hooks.js

@@ -6,6 +6,7 @@ import { ProblemTypeKeys } from '../../../../../data/constants/problem';
 import { fetchEditorContent } from '../hooks';
 
 export const state = StrictDict({
+  // eslint-disable-next-line react-hooks/rules-of-hooks
   isFeedbackVisible: (val) => useState(val),
 });
 

src/editors/containers/ProblemEditor/components/EditProblemView/SettingsWidget/hooks.js

@@ -12,10 +12,15 @@ import {
 import { fetchEditorContent } from '../hooks';
 
 export const state = {
+  // eslint-disable-next-line react-hooks/rules-of-hooks
   showAdvanced: (val) => useState(val),
+  // eslint-disable-next-line react-hooks/rules-of-hooks
   cardCollapsed: (val) => useState(val),
+  // eslint-disable-next-line react-hooks/rules-of-hooks
   summary: (val) => useState(val),
+  // eslint-disable-next-line react-hooks/rules-of-hooks
   showAttempts: (val) => useState(val),
+  // eslint-disable-next-line react-hooks/rules-of-hooks
   attemptDisplayValue: (val) => useState(val),
 };
 
@@ -44,6 +49,7 @@ export const showFullCard = (hasExpandableTextArea) => {
 export const hintsCardHooks = (hints, updateSettings) => {
   const [summary, setSummary] = module.state.summary({ message: messages.noHintSummary, values: {} });
 
+  // eslint-disable-next-line react-hooks/rules-of-hooks
   useEffect(() => {
     const hintsNumber = hints.length;
     if (hintsNumber === 0) {

src/editors/containers/ProblemEditor/components/EditProblemView/SettingsWidget/settingsComponents/GeneralFeedback/hooks.js

@@ -4,6 +4,7 @@ import messages from './messages';
 import * as module from './hooks';
 
 export const state = {
+  // eslint-disable-next-line react-hooks/rules-of-hooks
   summary: (val) => useState(val),
 };
 
@@ -12,6 +13,7 @@ export const generalFeedbackHooks = (generalFeedback, updateSettings) => {
     message: messages.noGeneralFeedbackSummary, values: {}, intl: true,
   });
 
+  // eslint-disable-next-line react-hooks/rules-of-hooks
   useEffect(() => {
     if (_.isEmpty(generalFeedback)) {
       setSummary({ message: messages.noGeneralFeedbackSummary, values: {}, intl: true });

src/editors/containers/ProblemEditor/components/EditProblemView/SettingsWidget/settingsComponents/GroupFeedback/hooks.js

@@ -4,12 +4,14 @@ import messages from './messages';
 import * as module from './hooks';
 
 export const state = {
+  // eslint-disable-next-line react-hooks/rules-of-hooks
   summary: (val) => useState(val),
 };
 
 export const groupFeedbackCardHooks = (groupFeedbacks, updateSettings, answerslist) => {
   const [summary, setSummary] = module.state.summary({ message: messages.noGroupFeedbackSummary, values: {} });
 
+  // eslint-disable-next-line react-hooks/rules-of-hooks
   useEffect(() => {
     if (groupFeedbacks.length === 0) {
       setSummary({ message: messages.noGroupFeedbackSummary, values: {} });

src/editors/containers/ProblemEditor/components/EditProblemView/SettingsWidget/settingsComponents/Randomization/hooks.js

@@ -3,6 +3,7 @@ import { RandomizationTypes, RandomizationTypesKeys } from '../../../../../../..
 import * as module from './hooks';
 
 export const state = {
+  // eslint-disable-next-line react-hooks/rules-of-hooks
   summary: (val) => useState(val),
 };
 

src/editors/containers/ProblemEditor/components/EditProblemView/hooks.js

@@ -7,6 +7,7 @@ import { setAssetToStaticUrl } from '../../../../sharedComponents/TinyMceWidget/
 import { ProblemTypeKeys } from '../../../../data/constants/problem';
 
 export const state = StrictDict({
+  // eslint-disable-next-line react-hooks/rules-of-hooks
   isSaveWarningModalOpen: (val) => useState(val),
 });
 

src/editors/containers/ProblemEditor/components/SelectTypeModal/hooks.js

@@ -7,6 +7,7 @@ import * as module from './hooks';
 import { getDataFromOlx } from '../../../../data/redux/thunkActions/problem';
 
 export const state = StrictDict({
+  // eslint-disable-next-line react-hooks/rules-of-hooks
   selected: (val) => useState(val),
 });
 

src/editors/containers/VideoEditor/components/SelectVideoModal.jsx

@@ -10,7 +10,9 @@ import * as module from './SelectVideoModal';
 
 export const hooks = {
   videoList: ({ fetchVideos }) => {
+  // eslint-disable-next-line react-hooks/rules-of-hooks
     const [videos, setVideos] = React.useState(null);
+    // eslint-disable-next-line react-hooks/rules-of-hooks
     React.useEffect(() => {
       fetchVideos({ onSuccess: setVideos });
     }, []);

src/editors/containers/VideoEditor/components/VideoEditorModal.jsx

@@ -13,12 +13,15 @@ export const {
 
 export const hooks = {
   initialize: (dispatch, selectedVideoId, selectedVideoUrl) => {
+    // eslint-disable-next-line react-hooks/rules-of-hooks
     React.useEffect(() => {
       dispatch(thunkActions.video.loadVideoData(selectedVideoId, selectedVideoUrl));
     }, []);
   },
   returnToGallery: () => {
+    // eslint-disable-next-line react-hooks/rules-of-hooks
     const learningContextId = useSelector(selectors.app.learningContextId);
+    // eslint-disable-next-line react-hooks/rules-of-hooks
     const blockId = useSelector(selectors.app.blockId);
     return () => (navigateTo(`/course/${learningContextId}/editor/course-videos/${blockId}`));
   },

src/editors/containers/VideoEditor/components/VideoSettingsModal/components/DurationWidget/hooks.js

@@ -9,8 +9,10 @@ const durationMatcher = /^(\d{0,2}):?(\d{0,2})?:?(\d{0,2})?$/i;
 export const durationWidget = ({ duration, updateField }) => {
   const setDuration = (val) => updateField({ duration: val });
   const initialState = module.durationString(duration);
+  // eslint-disable-next-line react-hooks/rules-of-hooks
   const [unsavedDuration, setUnsavedDuration] = useState(initialState);
 
+  // eslint-disable-next-line react-hooks/rules-of-hooks
   useEffect(() => {
     setUnsavedDuration(module.durationString(duration));
   }, [duration]);

src/editors/containers/VideoEditor/components/VideoSettingsModal/components/HandoutWidget/hooks.jsx

@@ -4,6 +4,7 @@ import { thunkActions } from '../../../../../../data/redux';
 import * as module from './hooks';
 
 export const state = {
+  // eslint-disable-next-line react-hooks/rules-of-hooks
   showSizeError: (args) => React.useState(args),
 };
 
@@ -28,7 +29,9 @@ export const checkValidFileSize = ({
 };
 
 export const fileInput = ({ fileSizeError }) => {
+  // eslint-disable-next-line react-hooks/rules-of-hooks
   const dispatch = useDispatch();
+  // eslint-disable-next-line react-hooks/rules-of-hooks
   const ref = React.useRef();
   const click = () => ref.current.click();
   const addFile = (e) => {

src/editors/containers/VideoEditor/components/VideoSettingsModal/components/ThumbnailWidget/hooks.js

@@ -5,6 +5,7 @@ import * as constants from './constants';
 import * as module from './hooks';
 
 export const state = {
+  // eslint-disable-next-line react-hooks/rules-of-hooks
   showSizeError: (args) => React.useState(args),
 };
 
@@ -85,7 +86,9 @@ export const checkValidSize = ({ file, onSizeFail }) => {
 };
 
 export const fileInput = ({ setThumbnailSrc, imgRef, fileSizeError }) => {
+  // eslint-disable-next-line react-hooks/rules-of-hooks
   const dispatch = useDispatch();
+  // eslint-disable-next-line react-hooks/rules-of-hooks
   const ref = React.useRef();
   const click = () => ref.current.click();
   const addFile = (e) => {

src/editors/containers/VideoEditor/components/VideoSettingsModal/components/TranscriptWidget/Transcript.jsx

@@ -25,6 +25,7 @@ import messages from './messages';
 
 export const hooks = {
   state: {
+  // eslint-disable-next-line react-hooks/rules-of-hooks
     inDeleteConfirmation: (args) => React.useState(args),
   },
   setUpDeleteConfirmation: () => {

src/editors/containers/VideoEditor/components/VideoSettingsModal/components/TranscriptWidget/index.jsx

@@ -33,6 +33,7 @@ import * as module from './index';
 
 export const hooks = {
   updateErrors: ({ isUploadError, isDeleteError }) => {
+  // eslint-disable-next-line react-hooks/rules-of-hooks
     const [error, setError] = React.useContext(ErrorContext).transcripts;
     if (isUploadError) {
       setError({ ...error, uploadError: messages.uploadTranscriptError.defaultMessage });

src/editors/containers/VideoEditor/components/VideoSettingsModal/components/VideoSourceWidget/hooks.jsx

@@ -4,6 +4,7 @@ import { parseYoutubeId } from '../../../../../../data/services/cms/api';
 import * as requests from '../../../../../../data/redux/thunkActions/requests';
 
 export const state = {
+  // eslint-disable-next-line react-hooks/rules-of-hooks
   showVideoIdChangeAlert: (args) => React.useState(args),
 };
 

src/editors/containers/VideoEditor/components/VideoSettingsModal/components/hooks.js

@@ -74,6 +74,7 @@ export const updatedObject = (obj, index, val) => ({ ...obj, [index]: val });
  * @param {string} key - form key
  * @return {func} - callback taking a value and updating the video redux field
  */
+// eslint-disable-next-line react-hooks/rules-of-hooks
 export const updateFormField = ({ dispatch, key }) => useCallback(
   (val) => dispatch(actions.video.updateField({ [key]: val })),
   [],
@@ -93,14 +94,17 @@ export const updateFormField = ({ dispatch, key }) => useCallback(
  *   setAll - sets form field in hook AND redux
  */
 export const valueHooks = ({ dispatch, key }) => {
+  // eslint-disable-next-line react-hooks/rules-of-hooks
   const formValue = useSelector(selectors.video[key]);
   const [local, setLocal] = module.state[key](formValue);
   const setFormValue = module.updateFormField({ dispatch, key });
 
+  // eslint-disable-next-line react-hooks/rules-of-hooks
   useEffect(() => {
     setLocal(formValue);
   }, [formValue]);
 
+  // eslint-disable-next-line react-hooks/rules-of-hooks
   const setAll = useCallback(
     (val) => {
       setLocal(val);

src/editors/containers/VideoEditor/hooks.js

@@ -7,12 +7,14 @@ import * as module from './hooks';
 export const ErrorContext = createContext();
 
 export const state = StrictDict({
+  /* eslint-disable react-hooks/rules-of-hooks */
   durationErrors: (val) => useState(val),
   handoutErrors: (val) => useState(val),
   licenseErrors: (val) => useState(val),
   thumbnailErrors: (val) => useState(val),
   transcriptsErrors: (val) => useState(val),
   videoSourceErrors: (val) => useState(val),
+  /* eslint-enable react-hooks/rules-of-hooks */
 });
 
 export const errorsHook = () => {

src/editors/containers/VideoGallery/hooks.js

@@ -19,12 +19,19 @@ export const {
 } = appHooks;
 
 export const state = {
+  // eslint-disable-next-line react-hooks/rules-of-hooks
   highlighted: (val) => React.useState(val),
+  // eslint-disable-next-line react-hooks/rules-of-hooks
   searchString: (val) => React.useState(val),
+  // eslint-disable-next-line react-hooks/rules-of-hooks
   showSelectVideoError: (val) => React.useState(val),
+  // eslint-disable-next-line react-hooks/rules-of-hooks
   showSizeError: (val) => React.useState(val),
+  // eslint-disable-next-line react-hooks/rules-of-hooks
   sortBy: (val) => React.useState(val),
+  // eslint-disable-next-line react-hooks/rules-of-hooks
   filertBy: (val) => React.useState(val),
+  // eslint-disable-next-line react-hooks/rules-of-hooks
   hideSelectedVideos: (val) => React.useState(val),
 };
 
@@ -100,7 +107,9 @@ export const videoListProps = ({ searchSortProps, videos }) => {
     setShowSizeError,
   ] = module.state.showSizeError(false);
   const filteredList = module.filterList({ ...searchSortProps, videos });
+  // eslint-disable-next-line react-hooks/rules-of-hooks
   const learningContextId = useSelector(selectors.app.learningContextId);
+  // eslint-disable-next-line react-hooks/rules-of-hooks
   const blockId = useSelector(selectors.app.blockId);
   return {
     galleryError: {
@@ -146,14 +155,18 @@ export const fileInputProps = () => {
 };
 
 export const handleVideoUpload = () => {
+  // eslint-disable-next-line react-hooks/rules-of-hooks
   const learningContextId = useSelector(selectors.app.learningContextId);
+  // eslint-disable-next-line react-hooks/rules-of-hooks
   const blockId = useSelector(selectors.app.blockId);
   return () => navigateTo(`/course/${learningContextId}/editor/video_upload/${blockId}`);
 };
 
 export const handleCancel = () => (
   navigateCallback({
+    // eslint-disable-next-line react-hooks/rules-of-hooks
     destination: useSelector(selectors.app.returnUrl),
+    // eslint-disable-next-line react-hooks/rules-of-hooks
     analytics: useSelector(selectors.app.analytics),
     analyticsEvent: analyticsEvt.videoGalleryCancelClick,
   })

src/editors/containers/VideoUploadEditor/hooks.js

@@ -14,8 +14,11 @@ export const {
 } = appHooks;
 
 export const state = {
+  // eslint-disable-next-line react-hooks/rules-of-hooks
   loading: (val) => React.useState(val),
+  // eslint-disable-next-line react-hooks/rules-of-hooks
   errorMessage: (val) => React.useState(val),
+  // eslint-disable-next-line react-hooks/rules-of-hooks
   textInputValue: (val) => React.useState(val),
 };
 

src/editors/hooks.js

@@ -7,6 +7,7 @@ import { actions, thunkActions } from './data/redux';
 import * as module from './hooks';
 import { RequestKeys } from './data/constants/requests';
 
+// eslint-disable-next-line react-hooks/rules-of-hooks
 export const initializeApp = ({ dispatch, data }) => useEffect(
   () => dispatch(thunkActions.app.initialize(data)),
   [data],

src/editors/sharedComponents/CodeEditor/hooks.js

@@ -13,6 +13,7 @@ import './index.scss';
 const CODEMIRROR_LANGUAGES = { HTML: 'html', XML: 'xml' };
 
 export const state = {
+  // eslint-disable-next-line react-hooks/rules-of-hooks
   showBtnEscapeHTML: (val) => React.useState(val),
 };
 
@@ -60,6 +61,7 @@ export const createCodeMirrorDomNode = ({
   upstreamRef,
   lang,
 }) => {
+  // eslint-disable-next-line react-hooks/rules-of-hooks
   useEffect(() => {
     const languageExtension = lang === CODEMIRROR_LANGUAGES.HTML ? html() : xml();
     const cleanText = cleanHTML({ initialText });