fix(ui): Submit button grayed out on details page. Fixes argoproj#13453

The React 18 upgrade in argoproj#13069 introduced a bug on the details page where the "Submit" button is disabled immediately on page load. Specifically, I believe this is happening due to batching changes that affect the following two `useEffect()` calls, which are common to all the details pages modified in this PR: ``` useEffect(() => { (async () => { try { const newEventSource = await services.eventSource.get(name, namespace); setEventSource(newEventSource); setEdited(false); // set back to false setError(null); } catch (err) { setError(err); } })(); }, [name, namespace]); useEffect(() => setEdited(true), [eventSource]); ``` The first runs immediately and invokes `setEventSource(newEventSource)`, which causes the second `useEffect()` call to be fired, since the `eventSource` dependency has changed. Since both are invoking `setEdited()`, this is basically a race condition, since the state of `edited` is going to depend on whether these calls are batched together are fired separately. This PR fixes that by removing the second `useEffect()` call, which eliminates the race condition. Instead, we only call `setEdited(true)` when the editor is modified. Signed-off-by: Mason Malone <[email protected]>
MasonM · Sep 12, 2024 · 77d896c · 77d896c
1 parent c551304
commit 77d896c
Show file tree

Hide file tree

Showing 5 changed files with 25 additions and 10 deletions.
diff --git a/ui/src/app/cluster-workflow-templates/cluster-workflow-template-details.tsx b/ui/src/app/cluster-workflow-templates/cluster-workflow-template-details.tsx
@@ -42,7 +42,10 @@ export function ClusterWorkflowTemplateDetails({history, location, match}: Route
         [history]
     );
 
-    useEffect(() => setEdited(true), [template]);
+    const editTemplate = (template: ClusterWorkflowTemplate) => {
+        setEdited(true);
+        setTemplate(template);
+    };
     useEffect(() => {
         history.push(historyUrl('cluster-workflow-templates/{name}', {name, sidePanel, tab}));
     }, [name, sidePanel, tab]);
@@ -132,7 +135,7 @@ export function ClusterWorkflowTemplateDetails({history, location, match}: Route
                 {!template ? (
                     <Loading />
                 ) : (
-                    <ClusterWorkflowTemplateEditor template={template} onChange={setTemplate} onError={setError} onTabSelected={setTab} selectedTabKey={tab} />
+                    <ClusterWorkflowTemplateEditor template={template} onChange={editTemplate} onError={setError} onTabSelected={setTab} selectedTabKey={tab} />
                 )}
             </>
             {template && (

diff --git a/ui/src/app/cron-workflows/cron-workflow-details.tsx b/ui/src/app/cron-workflows/cron-workflow-details.tsx
@@ -70,7 +70,10 @@ export function CronWorkflowDetails({match, location, history}: RouteComponentPr
             .catch(setError);
     }, [namespace, name]);
 
-    useEffect(() => setEdited(true), [cronWorkflow]);
+    const editCronWorkflow = (cronWorkflow: CronWorkflow) => {
+        setEdited(true);
+        setCronWorkflow(cronWorkflow);
+    };
 
     useEffect(() => {
         (async () => {
@@ -214,7 +217,7 @@ export function CronWorkflowDetails({match, location, history}: RouteComponentPr
                 {!cronWorkflow ? (
                     <Loading />
                 ) : (
-                    <CronWorkflowEditor cronWorkflow={cronWorkflow} onChange={setCronWorkflow} onError={setError} selectedTabKey={tab} onTabSelected={setTab} />
+                    <CronWorkflowEditor cronWorkflow={cronWorkflow} onChange={editCronWorkflow} onError={setError} selectedTabKey={tab} onTabSelected={setTab} />
                 )}
                 <SlidingPanel isShown={!!sidePanel} onClose={() => setSidePanel(null)}>
                     {sidePanel === 'share' && <WidgetGallery namespace={namespace} label={'workflows.argoproj.io/cron-workflow=' + name} />}

diff --git a/ui/src/app/event-sources/event-source-details.tsx b/ui/src/app/event-sources/event-source-details.tsx
@@ -78,7 +78,10 @@ export function EventSourceDetails({history, location, match}: RouteComponentPro
         })();
     }, [name, namespace]);
 
-    useEffect(() => setEdited(true), [eventSource]);
+    const editEventSource = (eventSource: EventSource) => {
+        setEventSource(eventSource);
+        setEdited(true);
+    };
 
     useCollectEvent('openedEventSourceDetails');
 
@@ -143,7 +146,7 @@ export function EventSourceDetails({history, location, match}: RouteComponentPro
                 {!eventSource ? (
                     <Loading />
                 ) : (
-                    <EventSourceEditor eventSource={eventSource} onChange={setEventSource} onError={setError} onTabSelected={setTab} selectedTabKey={tab} />
+                    <EventSourceEditor eventSource={eventSource} onChange={editEventSource} onError={setError} onTabSelected={setTab} selectedTabKey={tab} />
                 )}
             </>
             <SlidingPanel isShown={!!selected} onClose={() => setSelectedNode(null)}>

diff --git a/ui/src/app/sensors/sensor-details.tsx b/ui/src/app/sensors/sensor-details.tsx
@@ -64,7 +64,10 @@ export function SensorDetails({match, location, history}: RouteComponentProps<an
             .catch(setError);
     }, [namespace, name]);
 
-    useEffect(() => setEdited(true), [sensor]);
+    const editSensor = (sensor: Sensor) => {
+        setEdited(true);
+        setSensor(sensor);
+    };
 
     useCollectEvent('openedSensorDetails');
 
@@ -129,7 +132,7 @@ export function SensorDetails({match, location, history}: RouteComponentProps<an
             }}>
             <>
                 <ErrorNotice error={error} />
-                {!sensor ? <Loading /> : <SensorEditor sensor={sensor} onChange={setSensor} onError={setError} selectedTabKey={tab} onTabSelected={setTab} />}
+                {!sensor ? <Loading /> : <SensorEditor sensor={sensor} onChange={editSensor} onError={setError} selectedTabKey={tab} onTabSelected={setTab} />}
             </>
             {!!selectedLogNode && (
                 <SensorSidePanel

diff --git a/ui/src/app/workflow-templates/workflow-template-details.tsx b/ui/src/app/workflow-templates/workflow-template-details.tsx
@@ -54,7 +54,10 @@ export function WorkflowTemplateDetails({history, location, match}: RouteCompone
     const [template, setTemplate] = useState<WorkflowTemplate>();
     const [edited, setEdited] = useState(false);
 
-    useEffect(() => setEdited(true), [template]);
+    const editTemplate = (template: WorkflowTemplate) => {
+        setTemplate(template);
+        setEdited(true);
+    };
 
     useEffect(() => {
         services.workflowTemplate
@@ -122,7 +125,7 @@ export function WorkflowTemplateDetails({history, location, match}: RouteCompone
             }}>
             <>
                 <ErrorNotice error={error} />
-                {!template ? <Loading /> : <WorkflowTemplateEditor template={template} onChange={setTemplate} onError={setError} onTabSelected={setTab} selectedTabKey={tab} />}
+                {!template ? <Loading /> : <WorkflowTemplateEditor template={template} onChange={editTemplate} onError={setError} onTabSelected={setTab} selectedTabKey={tab} />}
             </>
             {template && (
                 <SlidingPanel isShown={!!sidePanel} onClose={() => setSidePanel(null)} isMiddle={sidePanel === 'submit'}>