refactor: break setState to separate handlers

to be more consistent with common patterns?
determined-ai · Aug 4, 2020 · 132ed48 · 132ed48
1 parent 4a7b47b
commit 132ed48
Show file tree

Hide file tree

Showing 3 changed files with 33 additions and 36 deletions.
diff --git a/webui/react/src/components/CreateExperimentModal.tsx b/webui/react/src/components/CreateExperimentModal.tsx
@@ -1,43 +1,39 @@
 import { Alert, Modal } from 'antd';
 import yaml from 'js-yaml';
-import React, { SetStateAction, useCallback, useState } from 'react';
+import React, { useCallback, useState } from 'react';
 import MonacoEditor from 'react-monaco-editor';
 
 import { routeAll } from 'routes';
 import { forkExperiment } from 'services/api';
 
 import css from './CreateExperimentModal.module.scss';
 
-export interface CreateExpModalState {
-  visible: boolean;
-  config: string;
-}
-
 interface Props {
   title: string;
   okText: string;
   parentId: number;
   visible: boolean;
   config: string;
-  setState: (arg0: SetStateAction<CreateExpModalState>) => void;
+  onVisibleChange: (arg0: boolean) => void;
+  onConfigChange: (arg0: string) => void;
 }
 
 const CreateExperimentModal: React.FC<Props> = (
-  { visible, config, setState, parentId, ...props }: Props,
+  { visible, config, onVisibleChange, onConfigChange, parentId, ...props }: Props,
 ) => {
   const [ configError, setConfigError ] = useState<string>();
 
   const editorOnChange = useCallback((newValue: string) => {
-    setState((existingState: CreateExpModalState) => ({ ...existingState, config: newValue }));
+    onConfigChange(newValue);
     setConfigError(undefined);
-  }, [ setState, setConfigError ]);
+  }, [ onConfigChange, setConfigError ]);
 
   const handleOk = async (): Promise<void> => {
     try {
       // Validate the yaml syntax by attempting to load it.
       yaml.safeLoad(config);
       const configId = await forkExperiment({ experimentConfig: config, parentId });
-      setState(existingState => ({ ...existingState, visible: false }));
+      onVisibleChange(false);
       routeAll(`/det/experiments/${configId}`);
     } catch (e) {
       let errorMessage = 'Failed to config using the provided config.';
@@ -51,7 +47,7 @@ const CreateExperimentModal: React.FC<Props> = (
   };
 
   const handleCancel = (): void => {
-    setState(existingState => ({ ...existingState, visible: false }));
+    onVisibleChange(false);
   };
   return <Modal
     bodyStyle={{

diff --git a/webui/react/src/pages/ExperimentDetails.tsx b/webui/react/src/pages/ExperimentDetails.tsx
@@ -48,29 +48,30 @@ const ExperimentDetailsComp: React.FC = () => {
   }, [ id, triggerExperimentRequest ]);
 
   usePolling(pollExperimentDetails);
-  const [ forkModalState, setForkModalState ] = useState({ config: 'Loading', visible: false });
+  const [ forkModalVisible, setForkModalVisible ] = useState(false);
+  const [ forkModalConfig, setForkModalConfig ] = useState('Loading');
 
   useEffect(() => {
     if (experiment && experiment.config) {
       try {
         const prefix = 'Fork of ';
         const rawConfig = clone(experiment.configRaw);
         rawConfig.description = prefix + rawConfig.description;
-        setForkModalState(state => ({ ...state, config: yaml.safeDump(rawConfig) }));
+        setForkModalConfig(yaml.safeDump(rawConfig));
       } catch (e) {
         handleError({
           error: e,
           message: 'failed to load experiment config',
           type: ErrorType.ApiBadResponse,
         });
-        setForkModalState(state => ({ ...state, config: 'failed to load experiment config' }));
+        setForkModalConfig('failed to load experiment config');
       }
     }
   }, [ experiment ]);
 
   const showForkModal = useCallback((): void => {
-    setForkModalState(state => ({ ...state, visible: true }));
-  }, [ setForkModalState ]);
+    setForkModalVisible(true);
+  }, [ setForkModalVisible ]);
 
   const handleTableRow = useCallback((record: TrialItem) => ({
     onClick: makeClickHandler(record.url as string),
@@ -218,12 +219,13 @@ const ExperimentDetailsComp: React.FC = () => {
         title={`Best Checkpoint for Trial ${activeCheckpoint.trialId}`}
         onHide={handleCheckpointDismiss} />}
       <CreateExperimentModal
-        config={forkModalState.config}
+        config={forkModalConfig}
         okText="Fork"
         parentId={id}
-        setState={setForkModalState}
         title={`Fork Experiment ${id}`}
-        visible={forkModalState.visible}
+        visible={forkModalVisible}
+        onConfigChange={setForkModalConfig}
+        onVisibleChange={setForkModalVisible}
       />
     </Page>
   );

diff --git a/webui/react/src/pages/TrialDetails.tsx b/webui/react/src/pages/TrialDetails.tsx
@@ -3,7 +3,7 @@ import yaml from 'js-yaml';
 import React, { useCallback, useEffect, useState } from 'react';
 import { useParams } from 'react-router';
 
-import CreateExperimentModal, { CreateExpModalState } from 'components/CreateExperimentModal';
+import CreateExperimentModal from 'components/CreateExperimentModal';
 import Icon from 'components/Icon';
 import Link from 'components/Link';
 import Message from 'components/Message';
@@ -29,9 +29,8 @@ const TrialDetailsComp: React.FC = () => {
   const trialId = parseInt(trialIdParam);
   const [ trial, triggerTrialRequest ] =
     useRestApi<TrialDetailsParams, TrialDetails>(getTrialDetails, { id: trialId });
-  const [ ContModalState, setContModalState ] = useState<CreateExpModalState>(
-    { config: 'Loading', visible: false },
-  );
+  const [ contModalVisible, setContModalVisible ] = useState(false);
+  const [ contModalConfig, setContModalConfig ] = useState('Loading');
 
   const pollTrialDetails = useCallback(
     () => triggerTrialRequest({ id: trialId }),
@@ -59,15 +58,14 @@ const TrialDetailsComp: React.FC = () => {
           };
           const newHyperparameters = trialHParamsToExperimentHParams(hparams);
 
-          setContModalState(state => ({ ...state,
-            config: yaml.safeDump({
-              ...rawConfig,
-              description: newDescription,
-              hyperparameters: newHyperparameters,
-              searcher: newSearcher,
-            }) }));
+          setContModalConfig(yaml.safeDump({
+            ...rawConfig,
+            description: newDescription,
+            hyperparameters: newHyperparameters,
+            searcher: newSearcher,
+          }));
         } catch (e) {
-          setContModalState(state => ({ ...state, config: 'failed to load experiment config' }));
+          setContModalConfig('failed to load experiment config');
           handleError({
             error: e,
             message: 'failed to load experiment config',
@@ -81,7 +79,7 @@ const TrialDetailsComp: React.FC = () => {
   const handleActionClick = useCallback((action: TrialAction) => (): void => {
     switch (action) {
       case TrialAction.Continue:
-        setContModalState(state => ({ ...state, visible: true }));
+        setContModalVisible(true);
         break;
     }
   }, [ ]);
@@ -128,12 +126,13 @@ const TrialDetailsComp: React.FC = () => {
       <Section title="Chart" />
       <Section title="Steps" />
       <CreateExperimentModal
-        config={ContModalState.config}
+        config={contModalConfig}
         okText="Create"
         parentId={experimentId}
-        setState={setContModalState}
         title={`Continue Trial ${trialId}`}
-        visible={ContModalState.visible}
+        visible={contModalVisible}
+        onConfigChange={setContModalConfig}
+        onVisibleChange={setContModalVisible}
       />
     </Page>
   );