fix(ui): skip history.push on initial page load. Fixes #13940 (#13995)

Signed-off-by: Tianchu Zhao <[email protected]>
argoproj · Dec 13, 2024 · f796449 · f796449
1 parent 43c6abd
commit f796449
Show file tree

Hide file tree

Showing 16 changed files with 197 additions and 139 deletions.
diff --git a/ui/src/cron-workflows/cron-workflow-details.tsx b/ui/src/cron-workflows/cron-workflow-details.tsx
@@ -2,7 +2,7 @@ import {NotificationType} from 'argo-ui/src/components/notifications/notificatio
 import {Page} from 'argo-ui/src/components/page/page';
 import {SlidingPanel} from 'argo-ui/src/components/sliding-panel/sliding-panel';
 import * as React from 'react';
-import {useContext, useEffect, useState} from 'react';
+import {useContext, useEffect, useRef, useState} from 'react';
 import {RouteComponentProps} from 'react-router';
 
 import {uiUrl} from '../shared/base';
@@ -29,6 +29,7 @@ export function CronWorkflowDetails({match, location, history}: RouteComponentPr
     const {navigation, notifications, popup} = useContext(Context);
     const queryParams = new URLSearchParams(location.search);
 
+    const isFirstRender = useRef(true);
     const [namespace] = useState(match.params.namespace);
     const [name] = useState(match.params.name);
     const [sidePanel, setSidePanel] = useState(queryParams.get('sidePanel'));
@@ -47,18 +48,20 @@ export function CronWorkflowDetails({match, location, history}: RouteComponentPr
         [history]
     );
 
-    useEffect(
-        () =>
-            history.push(
-                historyUrl('cron-workflows/{namespace}/{name}', {
-                    namespace,
-                    name,
-                    sidePanel,
-                    tab
-                })
-            ),
-        [namespace, name, sidePanel, tab]
-    );
+    useEffect(() => {
+        if (isFirstRender.current) {
+            isFirstRender.current = false;
+            return;
+        }
+        history.push(
+            historyUrl('cron-workflows/{namespace}/{name}', {
+                namespace,
+                name,
+                sidePanel,
+                tab
+            })
+        );
+    }, [namespace, name, sidePanel, tab]);
 
     useEffect(() => {
         services.cronWorkflows

diff --git a/ui/src/cron-workflows/cron-workflow-list.tsx b/ui/src/cron-workflows/cron-workflow-list.tsx
@@ -1,7 +1,7 @@
 import {Page} from 'argo-ui/src/components/page/page';
 import {SlidingPanel} from 'argo-ui/src/components/sliding-panel/sliding-panel';
 import * as React from 'react';
-import {useContext, useEffect, useState} from 'react';
+import {useContext, useEffect, useRef, useState} from 'react';
 import {RouteComponentProps} from 'react-router-dom';
 
 import {uiUrl} from '../shared/base';
@@ -33,6 +33,7 @@ export function CronWorkflowList({match, location, history}: RouteComponentProps
     const {navigation} = useContext(Context);
 
     // state for URL, query, and label parameters
+    const isFirstRender = useRef(true);
     const [namespace, setNamespace] = useState<string>(nsUtils.getNamespace(match.params.namespace) || '');
     const [sidePanel, setSidePanel] = useState(queryParams.get('sidePanel') === 'true');
     const [labels, setLabels] = useState<string[]>([]);
@@ -49,16 +50,18 @@ export function CronWorkflowList({match, location, history}: RouteComponentProps
     );
 
     // save history
-    useEffect(
-        () =>
-            history.push(
-                historyUrl('cron-workflows' + (nsUtils.getManagedNamespace() ? '' : '/{namespace}'), {
-                    namespace,
-                    sidePanel
-                })
-            ),
-        [namespace, sidePanel]
-    );
+    useEffect(() => {
+        if (isFirstRender.current) {
+            isFirstRender.current = false;
+            return;
+        }
+        history.push(
+            historyUrl('cron-workflows' + (nsUtils.getManagedNamespace() ? '' : '/{namespace}'), {
+                namespace,
+                sidePanel
+            })
+        );
+    }, [namespace, sidePanel]);
 
     // internal state
     const [error, setError] = useState<Error>();

diff --git a/ui/src/event-flow/event-flow-page.tsx b/ui/src/event-flow/event-flow-page.tsx
@@ -1,7 +1,7 @@
 import {Page} from 'argo-ui/src/components/page/page';
 import {SlidingPanel} from 'argo-ui/src/components/sliding-panel/sliding-panel';
 import {Tabs} from 'argo-ui/src/components/tabs/tabs';
-import {useContext, useEffect, useState} from 'react';
+import {useContext, useEffect, useRef, useState} from 'react';
 import * as React from 'react';
 import {RouteComponentProps} from 'react-router-dom';
 import {Observable} from 'rxjs';
@@ -43,6 +43,7 @@ export function EventFlowPage({history, location, match}: RouteComponentProps<an
     const queryParams = new URLSearchParams(location.search);
 
     // state for URL and query parameters
+    const isFirstRender = useRef(true);
     const [namespace, setNamespace] = useState(nsUtils.getNamespace(match.params.namespace) || '');
     const [showFlow, setShowFlow] = useState(queryParams.get('showFlow') === 'true');
     const [showWorkflows, setShowWorkflows] = useState(queryParams.get('showWorkflows') !== 'false');
@@ -61,20 +62,22 @@ export function EventFlowPage({history, location, match}: RouteComponentProps<an
         [history]
     );
 
-    useEffect(
-        () =>
-            history.push(
-                historyUrl('event-flow' + (nsUtils.getManagedNamespace() ? '' : '/{namespace}'), {
-                    namespace,
-                    showFlow,
-                    showWorkflows,
-                    expanded,
-                    selectedNode,
-                    tab
-                })
-            ),
-        [namespace, showFlow, showWorkflows, expanded, expanded, tab]
-    );
+    useEffect(() => {
+        if (isFirstRender.current) {
+            isFirstRender.current = false;
+            return;
+        }
+        history.push(
+            historyUrl('event-flow' + (nsUtils.getManagedNamespace() ? '' : '/{namespace}'), {
+                namespace,
+                showFlow,
+                showWorkflows,
+                expanded,
+                selectedNode,
+                tab
+            })
+        );
+    }, [namespace, showFlow, showWorkflows, expanded, expanded, tab]);
 
     // internal state
     const [error, setError] = useState<Error>();

diff --git a/ui/src/event-sources/event-source-details.tsx b/ui/src/event-sources/event-source-details.tsx
@@ -3,7 +3,7 @@ import {Page} from 'argo-ui/src/components/page/page';
 import {SlidingPanel} from 'argo-ui/src/components/sliding-panel/sliding-panel';
 import {Tabs} from 'argo-ui/src/components/tabs/tabs';
 import * as React from 'react';
-import {useContext, useEffect, useState} from 'react';
+import {useContext, useEffect, useRef, useState} from 'react';
 import {RouteComponentProps} from 'react-router';
 
 import {ID} from '../event-flow/id';
@@ -27,6 +27,7 @@ export function EventSourceDetails({history, location, match}: RouteComponentPro
     const queryParams = new URLSearchParams(location.search);
 
     // state for URL and query parameters
+    const isFirstRender = useRef(true);
     const namespace = match.params.namespace;
     const name = match.params.name;
     const [tab, setTab] = useState<string>(queryParams.get('tab'));
@@ -40,18 +41,20 @@ export function EventSourceDetails({history, location, match}: RouteComponentPro
         [history]
     );
 
-    useEffect(
-        () =>
-            history.push(
-                historyUrl('event-sources/{namespace}/{name}', {
-                    namespace,
-                    name,
-                    tab,
-                    selectedNode
-                })
-            ),
-        [namespace, name, tab, selectedNode]
-    );
+    useEffect(() => {
+        if (isFirstRender.current) {
+            isFirstRender.current = false;
+            return;
+        }
+        history.push(
+            historyUrl('event-sources/{namespace}/{name}', {
+                namespace,
+                name,
+                tab,
+                selectedNode
+            })
+        );
+    }, [namespace, name, tab, selectedNode]);
 
     const [error, setError] = useState<Error>();
     const {object: eventSource, setObject: setEventSource, resetObject: resetEventSource, serialization, edited, lang, setLang} = useEditableObject<EventSource>();

diff --git a/ui/src/event-sources/event-source-list.tsx b/ui/src/event-sources/event-source-list.tsx
@@ -3,7 +3,7 @@ import {SlidingPanel} from 'argo-ui/src/components/sliding-panel/sliding-panel';
 import {Tabs} from 'argo-ui/src/components/tabs/tabs';
 import classNames from 'classnames';
 import * as React from 'react';
-import {useContext, useEffect, useState} from 'react';
+import {useContext, useEffect, useRef, useState} from 'react';
 import {Link, RouteComponentProps} from 'react-router-dom';
 
 import {ID} from '../event-flow/id';
@@ -36,6 +36,7 @@ export function EventSourceList({match, location, history}: RouteComponentProps<
     const {navigation} = useContext(Context);
 
     // state for URL and query parameters
+    const isFirstRender = useRef(true);
     const [namespace, setNamespace] = useState(nsUtils.getNamespace(match.params.namespace) || '');
     const [sidePanel, setSidePanel] = useState(queryParams.get('sidePanel') === 'true');
     const [selectedNode, setSelectedNode] = useState<Node>(queryParams.get('selectedNode'));
@@ -50,18 +51,20 @@ export function EventSourceList({match, location, history}: RouteComponentProps<
         [history]
     );
 
-    useEffect(
-        () =>
-            history.push(
-                historyUrl('event-sources' + (nsUtils.getManagedNamespace() ? '' : '/{namespace}'), {
-                    namespace,
-                    sidePanel,
-                    selectedNode,
-                    tab
-                })
-            ),
-        [namespace, sidePanel, selectedNode, tab]
-    );
+    useEffect(() => {
+        if (isFirstRender.current) {
+            isFirstRender.current = false;
+            return;
+        }
+        history.push(
+            historyUrl('event-sources' + (nsUtils.getManagedNamespace() ? '' : '/{namespace}'), {
+                namespace,
+                sidePanel,
+                selectedNode,
+                tab
+            })
+        );
+    }, [namespace, sidePanel, selectedNode, tab]);
 
     // internal state
     const [error, setError] = useState<Error>();

diff --git a/ui/src/plugins/plugin-list.tsx b/ui/src/plugins/plugin-list.tsx
@@ -1,6 +1,6 @@
 import {Page} from 'argo-ui/src/components/page/page';
 import * as React from 'react';
-import {useEffect, useState} from 'react';
+import {useEffect, useRef, useState} from 'react';
 import {RouteComponentProps} from 'react-router-dom';
 
 import {uiUrl} from '../shared/base';
@@ -11,16 +11,19 @@ import {useCollectEvent} from '../shared/use-collect-event';
 
 export function PluginList({match, history}: RouteComponentProps<any>) {
     // state for URL and query parameters
+    const isFirstRender = useRef(true);
     const [namespace] = useState(nsUtils.getNamespace(match.params.namespace) || '');
-    useEffect(
-        () =>
-            history.push(
-                historyUrl('plugins' + (nsUtils.getManagedNamespace() ? '' : '/{namespace}'), {
-                    namespace
-                })
-            ),
-        [namespace]
-    );
+    useEffect(() => {
+        if (isFirstRender.current) {
+            isFirstRender.current = false;
+            return;
+        }
+        history.push(
+            historyUrl('plugins' + (nsUtils.getManagedNamespace() ? '' : '/{namespace}'), {
+                namespace
+            })
+        );
+    }, [namespace]);
     useCollectEvent('openedPlugins');
 
     return (

diff --git a/ui/src/reports/reports.tsx b/ui/src/reports/reports.tsx
@@ -4,7 +4,7 @@ import {ChartOptions} from 'chart.js';
 import 'chartjs-plugin-annotation';
 
 import * as React from 'react';
-import {useContext, useEffect, useState} from 'react';
+import {useContext, useEffect, useRef, useState} from 'react';
 import {Bar, ChartData} from 'react-chartjs-2';
 import {RouteComponentProps} from 'react-router-dom';
 
@@ -33,6 +33,7 @@ export function Reports({match, location, history}: RouteComponentProps<any>) {
     const {navigation} = useContext(Context);
 
     // state for URL, query, and label parameters
+    const isFirstRender = useRef(true);
     const [namespace, setNamespace] = useState<string>(nsUtils.getNamespace(match.params.namespace) || '');
     const [labels, setLabels] = useState((queryParams.get('labels') || '').split(',').filter(v => v !== ''));
     // internal state
@@ -41,6 +42,10 @@ export function Reports({match, location, history}: RouteComponentProps<any>) {
 
     // save history
     useEffect(() => {
+        if (isFirstRender.current) {
+            isFirstRender.current = false;
+            return;
+        }
         history.push(historyUrl('reports' + (nsUtils.getManagedNamespace() ? '' : '/{namespace}'), {namespace, labels: labels.join(',')}));
     }, [namespace, labels]);
 

diff --git a/ui/src/sensors/sensor-details.tsx b/ui/src/sensors/sensor-details.tsx
@@ -1,7 +1,7 @@
 import {NotificationType} from 'argo-ui/src/components/notifications/notifications';
 import {Page} from 'argo-ui/src/components/page/page';
 import * as React from 'react';
-import {useContext, useEffect, useState} from 'react';
+import {useContext, useEffect, useRef, useState} from 'react';
 import {RouteComponentProps} from 'react-router';
 
 import {ID} from '../event-flow/id';
@@ -23,6 +23,7 @@ import '../workflows/components/workflow-details/workflow-details.scss';
 
 export function SensorDetails({match, location, history}: RouteComponentProps<any>) {
     // boiler-plate
+    const isFirstRender = useRef(true);
     const {navigation, notifications, popup} = useContext(Context);
     const queryParams = new URLSearchParams(location.search);
 
@@ -42,18 +43,20 @@ export function SensorDetails({match, location, history}: RouteComponentProps<an
         [history]
     );
 
-    useEffect(
-        () =>
-            history.push(
-                historyUrl('sensors/{namespace}/{name}', {
-                    namespace,
-                    name,
-                    tab,
-                    selectedLogNode
-                })
-            ),
-        [namespace, name, tab, selectedLogNode]
-    );
+    useEffect(() => {
+        if (isFirstRender.current) {
+            isFirstRender.current = false;
+            return;
+        }
+        history.push(
+            historyUrl('sensors/{namespace}/{name}', {
+                namespace,
+                name,
+                tab,
+                selectedLogNode
+            })
+        );
+    }, [namespace, name, tab, selectedLogNode]);
 
     useEffect(() => {
         services.sensor