fix(ui): improve editor performance and fix Submit button. Fixes argo…

…proj#1382 The `<ObjectEditor>` component used by all the edit pages encapsulates the logic for serializing/deserializing the object being edited, but it's extremely inefficient. For example, if you're editing a `WorkflowTemplate`, every single keystroke will cause `parse()` to be called twice and `stringify()` to be called 4 times. For large documents, this serializatino/deserialization overhead adds up quickly. Additionally, the "Submit" button is sometimes incorrectly grayed out, e.g. after clicking the "Update" button. This is happening because `useEditableObject()` is using reference comparisons on the object being edited, since it doesn't have access to the serialized string version encapsulated by `<ObjectEditor>`. This decouples `<ObjectEditor>` from serialization/deseralization and lifts the state involved up to the `useEditableObject()` hook, as discussed in argoproj#13593 (comment). Doing so eliminates unnecessary serialization/deseralization: now, if you're editing a `WorkflowTemplate`, each keystroke will only call `parse()` once and won't call `stringify()` at all. This also fixes issues with the Submit button not reflecting the current page state, since `useEditableObject()` now uses string comparisons instead of ref comparisons. Signed-off-by: Mason Malone <[email protected]>
MasonM · Nov 17, 2024 · 60b91ba · 60b91ba
1 parent 6da0ff8
commit 60b91ba
Show file tree

Hide file tree

Showing 27 changed files with 386 additions and 244 deletions.
diff --git a/ui/src/cluster-workflow-templates/cluster-workflow-template-creator.tsx b/ui/src/cluster-workflow-templates/cluster-workflow-template-creator.tsx
@@ -8,10 +8,11 @@ import {UploadButton} from '../shared/components/upload-button';
 import {exampleClusterWorkflowTemplate} from '../shared/examples';
 import {ClusterWorkflowTemplate} from '../shared/models';
 import {services} from '../shared/services';
-import {ClusterWorkflowTemplateEditor} from './cluster-workflow-template-editor';
+import {useEditableObject} from '../shared/use-editable-object';
+import {WorkflowTemplateEditor} from '../workflow-templates/workflow-template-editor';
 
 export function ClusterWorkflowTemplateCreator({onCreate}: {onCreate: (workflow: ClusterWorkflowTemplate) => void}) {
-    const [template, setTemplate] = useState<ClusterWorkflowTemplate>(exampleClusterWorkflowTemplate());
+    const {object: template, setObject: setTemplate, serialization, lang, setLang} = useEditableObject(exampleClusterWorkflowTemplate());
     const [error, setError] = useState<Error>();
     return (
         <>
@@ -31,7 +32,7 @@ export function ClusterWorkflowTemplateCreator({onCreate}: {onCreate: (workflow:
                 </Button>
             </div>
             <ErrorNotice error={error} />
-            <ClusterWorkflowTemplateEditor template={template} onChange={setTemplate} onError={setError} />
+            <WorkflowTemplateEditor template={template} serialization={serialization} lang={lang} onLangChange={setLang} onChange={setTemplate} onError={setError} />
             <div>
                 <ExampleManifests />.
             </div>

diff --git a/ui/src/cluster-workflow-templates/cluster-workflow-template-details.tsx b/ui/src/cluster-workflow-templates/cluster-workflow-template-details.tsx
@@ -18,9 +18,9 @@ import {services} from '../shared/services';
 import {useCollectEvent} from '../shared/use-collect-event';
 import {useEditableObject} from '../shared/use-editable-object';
 import {useQueryParams} from '../shared/use-query-params';
+import {WorkflowTemplateEditor} from '../workflow-templates/workflow-template-editor';
 import {SubmitWorkflowPanel} from '../workflows/components/submit-workflow-panel';
 import {WorkflowDetailsList} from '../workflows/components/workflow-details-list/workflow-details-list';
-import {ClusterWorkflowTemplateEditor} from './cluster-workflow-template-editor';
 
 import '../workflows/components/workflow-details/workflow-details.scss';
 
@@ -37,7 +37,7 @@ export function ClusterWorkflowTemplateDetails({history, location, match}: Route
     const [columns, setColumns] = useState<models.Column[]>([]);
 
     const [error, setError] = useState<Error>();
-    const [template, edited, setTemplate, resetTemplate] = useEditableObject<ClusterWorkflowTemplate>();
+    const {object: template, setObject: setTemplate, resetObject: resetTemplate, serialization, edited, lang, setLang} = useEditableObject<ClusterWorkflowTemplate>();
 
     useEffect(
         useQueryParams(history, p => {
@@ -138,7 +138,16 @@ export function ClusterWorkflowTemplateDetails({history, location, match}: Route
                 {!template ? (
                     <Loading />
                 ) : (
-                    <ClusterWorkflowTemplateEditor template={template} onChange={setTemplate} onError={setError} onTabSelected={setTab} selectedTabKey={tab} />
+                    <WorkflowTemplateEditor
+                        template={template}
+                        serialization={serialization}
+                        lang={lang}
+                        onLangChange={setLang}
+                        onChange={setTemplate}
+                        onError={setError}
+                        onTabSelected={setTab}
+                        selectedTabKey={tab}
+                    />
                 )}
             </>
             {template && (

diff --git a/ui/src/cluster-workflow-templates/cluster-workflow-template-editor.tsx b/ui/src/cluster-workflow-templates/cluster-workflow-template-editor.tsx
diff --git a/ui/src/cron-workflows/cron-workflow-creator.tsx b/ui/src/cron-workflows/cron-workflow-creator.tsx
@@ -9,10 +9,11 @@ import {exampleCronWorkflow} from '../shared/examples';
 import {CronWorkflow} from '../shared/models';
 import * as nsUtils from '../shared/namespaces';
 import {services} from '../shared/services';
+import {useEditableObject} from '../shared/use-editable-object';
 import {CronWorkflowEditor} from './cron-workflow-editor';
 
 export function CronWorkflowCreator({onCreate, namespace}: {namespace: string; onCreate: (cronWorkflow: CronWorkflow) => void}) {
-    const [cronWorkflow, setCronWorkflow] = useState<CronWorkflow>(exampleCronWorkflow(nsUtils.getNamespaceWithDefault(namespace)));
+    const {object: cronWorkflow, setObject: setCronWorkflow, serialization, lang, setLang} = useEditableObject(exampleCronWorkflow(nsUtils.getNamespaceWithDefault(namespace)));
     const [error, setError] = useState<Error>();
     return (
         <>
@@ -32,7 +33,7 @@ export function CronWorkflowCreator({onCreate, namespace}: {namespace: string; o
                 </Button>
             </div>
             <ErrorNotice error={error} />
-            <CronWorkflowEditor cronWorkflow={cronWorkflow} onChange={setCronWorkflow} onError={setError} />
+            <CronWorkflowEditor cronWorkflow={cronWorkflow} serialization={serialization} lang={lang} onLangChange={setLang} onChange={setCronWorkflow} onError={setError} />
             <p>
                 <ExampleManifests />.
             </p>

diff --git a/ui/src/cron-workflows/cron-workflow-details.tsx b/ui/src/cron-workflows/cron-workflow-details.tsx
@@ -36,7 +36,7 @@ export function CronWorkflowDetails({match, location, history}: RouteComponentPr
     const [workflows, setWorkflows] = useState<Workflow[]>([]);
     const [columns, setColumns] = useState<models.Column[]>([]);
 
-    const [cronWorkflow, edited, setCronWorkflow, resetCronWorkflow] = useEditableObject<CronWorkflow>();
+    const {object: cronWorkflow, setObject: setCronWorkflow, resetObject: resetCronWorkflow, serialization, edited, lang, setLang} = useEditableObject<CronWorkflow>();
     const [error, setError] = useState<Error>();
 
     useEffect(
@@ -207,7 +207,16 @@ export function CronWorkflowDetails({match, location, history}: RouteComponentPr
                 {!cronWorkflow ? (
                     <Loading />
                 ) : (
-                    <CronWorkflowEditor cronWorkflow={cronWorkflow} onChange={setCronWorkflow} onError={setError} selectedTabKey={tab} onTabSelected={setTab} />
+                    <CronWorkflowEditor
+                        cronWorkflow={cronWorkflow}
+                        serialization={serialization}
+                        lang={lang}
+                        onLangChange={setLang}
+                        onChange={setCronWorkflow}
+                        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/cron-workflows/cron-workflow-editor.tsx b/ui/src/cron-workflows/cron-workflow-editor.tsx
@@ -5,6 +5,7 @@ import {LabelsAndAnnotationsEditor} from '../shared/components/editors/labels-an
 import {MetadataEditor} from '../shared/components/editors/metadata-editor';
 import {WorkflowParametersEditor} from '../shared/components/editors/workflow-parameters-editor';
 import {ObjectEditor} from '../shared/components/object-editor';
+import type {Lang} from '../shared/components/object-parser';
 import {CronWorkflow} from '../shared/models';
 import {CronWorkflowSpecEditor} from './cron-workflow-spec-editior';
 import {CronWorkflowStatusViewer} from './cron-workflow-status-viewer';
@@ -14,10 +15,16 @@ export function CronWorkflowEditor({
     onTabSelected,
     onError,
     onChange,
-    cronWorkflow
+    onLangChange,
+    cronWorkflow,
+    serialization,
+    lang
 }: {
     cronWorkflow: CronWorkflow;
-    onChange: (cronWorkflow: CronWorkflow) => void;
+    serialization: string;
+    lang: Lang;
+    onChange: (cronWorkflow: string | CronWorkflow) => void;
+    onLangChange: (lang: Lang) => void;
     onError: (error: Error) => void;
     onTabSelected?: (tab: string) => void;
     selectedTabKey?: string;
@@ -41,7 +48,16 @@ export function CronWorkflowEditor({
                 {
                     key: 'manifest',
                     title: 'Manifest',
-                    content: <ObjectEditor type='io.argoproj.workflow.v1alpha1.CronWorkflow' value={cronWorkflow} onChange={x => onChange({...x})} />
+                    content: (
+                        <ObjectEditor
+                            type='io.argoproj.workflow.v1alpha1.CronWorkflow'
+                            value={cronWorkflow}
+                            text={serialization}
+                            lang={lang}
+                            onLangChange={onLangChange}
+                            onChange={onChange}
+                        />
+                    )
                 },
                 {
                     key: 'cron',

diff --git a/ui/src/event-flow/event-flow-page.tsx b/ui/src/event-flow/event-flow-page.tsx
@@ -15,7 +15,7 @@ import {GraphPanel} from '../shared/components/graph/graph-panel';
 import {Node} from '../shared/components/graph/types';
 import {Links} from '../shared/components/links';
 import {NamespaceFilter} from '../shared/components/namespace-filter';
-import {ResourceEditor} from '../shared/components/resource-editor/resource-editor';
+import {SerializingObjectEditor} from '../shared/components/object-editor';
 import {ZeroState} from '../shared/components/zero-state';
 import {Context} from '../shared/context';
 import {Footnote} from '../shared/footnote';
@@ -317,7 +317,7 @@ export function EventFlowPage({history, location, match}: RouteComponentProps<an
                                 {
                                     title: 'SUMMARY',
                                     key: 'summary',
-                                    content: <ResourceEditor kind={selected.kind} value={selected.value} />
+                                    content: <SerializingObjectEditor type={'io.argoproj.workflow.v1alpha1.' + selected.kind} value={selected.value} />
                                 },
                                 {
                                     title: 'LOGS',

diff --git a/ui/src/event-sources/event-source-creator.tsx b/ui/src/event-sources/event-source-creator.tsx
@@ -8,10 +8,11 @@ import {exampleEventSource} from '../shared/examples';
 import {EventSource} from '../shared/models';
 import * as nsUtils from '../shared/namespaces';
 import {services} from '../shared/services';
+import {useEditableObject} from '../shared/use-editable-object';
 import {EventSourceEditor} from './event-source-editor';
 
 export function EventSourceCreator({onCreate, namespace}: {namespace: string; onCreate: (eventSource: EventSource) => void}) {
-    const [eventSource, setEventSource] = useState<EventSource>(exampleEventSource(nsUtils.getNamespaceWithDefault(namespace)));
+    const {object: eventSource, setObject: setEventSource, serialization, lang, setLang} = useEditableObject(exampleEventSource(nsUtils.getNamespaceWithDefault(namespace)));
     const [error, setError] = useState<Error>();
     return (
         <>
@@ -31,7 +32,7 @@ export function EventSourceCreator({onCreate, namespace}: {namespace: string; on
                 </Button>
             </div>
             <ErrorNotice error={error} />
-            <EventSourceEditor eventSource={eventSource} onChange={setEventSource} onError={setError} />
+            <EventSourceEditor eventSource={eventSource} serialization={serialization} lang={lang} onChange={setEventSource} onLangChange={setLang} onError={setError} />
             <p>
                 <a href='https://github.com/argoproj/argo-events/tree/stable/examples/event-sources'>
                     Example event sources <i className='fa fa-external-link-alt' />

diff --git a/ui/src/event-sources/event-source-details.tsx b/ui/src/event-sources/event-source-details.tsx
@@ -54,7 +54,7 @@ export function EventSourceDetails({history, location, match}: RouteComponentPro
     );
 
     const [error, setError] = useState<Error>();
-    const [eventSource, edited, setEventSource, resetEventSource] = useEditableObject<EventSource>();
+    const {object: eventSource, setObject: setEventSource, resetObject: resetEventSource, serialization, edited, lang, setLang} = useEditableObject<EventSource>();
 
     const selected = (() => {
         if (!selectedNode) {
@@ -139,7 +139,16 @@ export function EventSourceDetails({history, location, match}: RouteComponentPro
                 {!eventSource ? (
                     <Loading />
                 ) : (
-                    <EventSourceEditor eventSource={eventSource} onChange={setEventSource} onError={setError} onTabSelected={setTab} selectedTabKey={tab} />
+                    <EventSourceEditor
+                        eventSource={eventSource}
+                        serialization={serialization}
+                        lang={lang}
+                        onChange={setEventSource}
+                        onLangChange={setLang}
+                        onError={setError}
+                        onTabSelected={setTab}
+                        selectedTabKey={tab}
+                    />
                 )}
             </>
             <SlidingPanel isShown={!!selected} onClose={() => setSelectedNode(null)}>

diff --git a/ui/src/event-sources/event-source-editor.tsx b/ui/src/event-sources/event-source-editor.tsx
@@ -3,16 +3,23 @@ import * as React from 'react';
 
 import {MetadataEditor} from '../shared/components/editors/metadata-editor';
 import {ObjectEditor} from '../shared/components/object-editor';
+import type {Lang} from '../shared/components/object-parser';
 import {EventSource} from '../shared/models';
 
 export function EventSourceEditor({
     onChange,
+    onLangChange,
     onTabSelected,
     selectedTabKey,
-    eventSource
+    eventSource,
+    serialization,
+    lang
 }: {
     eventSource: EventSource;
-    onChange: (template: EventSource) => void;
+    serialization: string;
+    lang: Lang;
+    onChange: (template: string | EventSource) => void;
+    onLangChange: (lang: Lang) => void;
     onError: (error: Error) => void;
     onTabSelected?: (tab: string) => void;
     selectedTabKey?: string;
@@ -27,7 +34,16 @@ export function EventSourceEditor({
                 {
                     key: 'manifest',
                     title: 'Manifest',
-                    content: <ObjectEditor type='io.argoproj.events.v1alpha1.EventSource' value={eventSource} onChange={x => onChange({...x})} />
+                    content: (
+                        <ObjectEditor
+                            type='io.argoproj.events.v1alpha1.EventSource'
+                            value={eventSource}
+                            text={serialization}
+                            lang={lang}
+                            onLangChange={onLangChange}
+                            onChange={onChange}
+                        />
+                    )
                 },
 
                 {

diff --git a/ui/src/sensors/sensor-creator.tsx b/ui/src/sensors/sensor-creator.tsx
@@ -8,10 +8,11 @@ import {exampleSensor} from '../shared/examples';
 import {Sensor} from '../shared/models';
 import * as nsUtils from '../shared/namespaces';
 import {services} from '../shared/services';
+import {useEditableObject} from '../shared/use-editable-object';
 import {SensorEditor} from './sensor-editor';
 
 export function SensorCreator({namespace, onCreate}: {namespace: string; onCreate: (sensor: Sensor) => void}) {
-    const [sensor, setSensor] = useState<Sensor>(exampleSensor(nsUtils.getNamespaceWithDefault(namespace)));
+    const {object: sensor, setObject: setSensor, serialization, lang, setLang} = useEditableObject(exampleSensor(nsUtils.getNamespaceWithDefault(namespace)));
     const [error, setError] = useState<Error>();
     return (
         <>
@@ -31,7 +32,7 @@ export function SensorCreator({namespace, onCreate}: {namespace: string; onCreat
                 </Button>
             </div>
             <ErrorNotice error={error} />
-            <SensorEditor sensor={sensor} onChange={setSensor} onError={setError} />
+            <SensorEditor sensor={sensor} serialization={serialization} lang={lang} onChange={setSensor} onLangChange={setLang} onError={setError} />
             <p>
                 <a href='https://github.com/argoproj/argo-events/tree/stable/examples/sensors'>
                     Example sensors <i className='fa fa-external-link-alt' />

diff --git a/ui/src/sensors/sensor-details.tsx b/ui/src/sensors/sensor-details.tsx
@@ -30,7 +30,7 @@ export function SensorDetails({match, location, history}: RouteComponentProps<an
     const [name] = useState(match.params.name);
     const [tab, setTab] = useState<string>(queryParams.get('tab'));
 
-    const [sensor, edited, setSensor, resetSensor] = useEditableObject<Sensor>();
+    const {object: sensor, setObject: setSensor, resetObject: resetSensor, serialization, edited, lang, setLang} = useEditableObject<Sensor>();
     const [selectedLogNode, setSelectedLogNode] = useState<Node>(queryParams.get('selectedLogNode'));
     const [error, setError] = useState<Error>();
 
@@ -125,7 +125,20 @@ 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}
+                        serialization={serialization}
+                        lang={lang}
+                        onChange={setSensor}
+                        onLangChange={setLang}
+                        onError={setError}
+                        selectedTabKey={tab}
+                        onTabSelected={setTab}
+                    />
+                )}
             </>
             {!!selectedLogNode && (
                 <SensorSidePanel