fix(orchestrator): endless onChange() loop and turning ui:allowNewItems from string to boolean type (#1720)

Signed-off-by: Marek Libra <marek.libra@gmail.com>

Author: mareklibra

workspaces/orchestrator/.changeset/clean-cooks-fetch.md

@@ -0,0 +1,6 @@
+---
+'@red-hat-developer-hub/backstage-plugin-orchestrator-form-widgets': patch
+'@red-hat-developer-hub/backstage-plugin-orchestrator-form-react': patch
+---
+
+Fixing endless onChange() loop and turning "ui:allowNewItems" from string to boolean type.

workspaces/orchestrator/plugins/orchestrator-form-react/src/components/OrchestratorForm.tsx

@@ -159,18 +159,17 @@ const OrchestratorForm = ({
     return generateUiSchema(schema, isMultiStep);
   }, [schema, isMultiStep]);
 
-  const reviewStep = useMemo(
-    () => (
+  const reviewStep = useMemo(() => {
+    return (
       <ReviewStep
         data={prunedFormData}
         schema={schema}
         busy={isExecuting}
         handleExecute={_handleExecute}
         // no schema update here
       />
-    ),
-    [prunedFormData, schema, isExecuting, _handleExecute],
-  );
+    );
+  }, [prunedFormData, schema, isExecuting, _handleExecute]);
 
   return (
     <StepperContextProvider reviewStep={reviewStep} t={t}>

workspaces/orchestrator/plugins/orchestrator-form-react/src/components/OrchestratorFormWrapper.tsx

@@ -20,7 +20,7 @@ import { ErrorPanel } from '@backstage/core-components';
 import { JsonObject } from '@backstage/types';
 
 import Grid from '@mui/material/Grid';
-import { withTheme } from '@rjsf/core';
+import { IChangeEvent, withTheme } from '@rjsf/core';
 import { Theme as MuiTheme } from '@rjsf/material-ui';
 import { ErrorSchema } from '@rjsf/utils';
 import type { JSONSchema7 } from 'json-schema';
@@ -47,7 +47,6 @@ const MuiForm = withTheme<
 
 const FormComponent = (decoratorProps: FormDecoratorProps) => {
   const formContext = decoratorProps.formContext;
-
   const [extraErrors, setExtraErrors] = useState<
     ErrorSchema<JsonObject> | undefined
   >();
@@ -111,6 +110,15 @@ const FormComponent = (decoratorProps: FormDecoratorProps) => {
     }
   };
 
+  const onChange = (
+    e: IChangeEvent<JsonObject, JSONSchema7, OrchestratorFormContextProps>,
+  ) => {
+    setFormData(e.formData || {});
+    if (decoratorProps.onChange) {
+      decoratorProps.onChange(e);
+    }
+  };
+
   return (
     <Grid container spacing={2} direction="column" wrap="nowrap">
       {validationError && (
@@ -131,12 +139,7 @@ const FormComponent = (decoratorProps: FormDecoratorProps) => {
           noHtml5Validate
           extraErrors={extraErrors}
           onSubmit={e => onSubmit(e.formData || {})}
-          onChange={e => {
-            setFormData(e.formData || {});
-            if (decoratorProps.onChange) {
-              decoratorProps.onChange(e);
-            }
-          }}
+          onChange={onChange}
         >
           {children}
         </MuiForm>

workspaces/orchestrator/plugins/orchestrator-form-widgets/src/uiPropTypes.ts

@@ -19,7 +19,7 @@ import { TypographyVariant } from '@mui/material/styles';
 export type UiProps = {
   'ui:variant'?: TypographyVariant;
   'ui:text'?: string;
-  'ui:allowNewItems'?: 'true' | 'false';
+  'ui:allowNewItems'?: boolean;
   'fetch:url'?: string;
   'fetch:method'?: 'GET' | 'POST';
   'fetch:headers'?: Record<string, string>;

workspaces/orchestrator/plugins/orchestrator-form-widgets/src/widgets/ActiveDropdown.tsx

@@ -70,7 +70,7 @@ export const ActiveDropdown: Widget<
     () => (props.options?.props ?? {}) as UiProps,
     [props.options?.props],
   );
-  const isReadOnly = !!props.readonly;
+  const isReadOnly = !!props?.schema.readOnly;
 
   const labelSelector = uiProps['fetch:response:label']?.toString();
   const valueSelector = uiProps['fetch:response:value']?.toString();
@@ -140,7 +140,7 @@ export const ActiveDropdown: Widget<
 
   // set default value to the first one
   useEffect(() => {
-    if (!isChangedByUser && values && values.length > 0) {
+    if (!isChangedByUser && !value && values && values.length > 0) {
       handleChange(values[0], false);
     }
   }, [handleChange, value, values, isChangedByUser]);

workspaces/orchestrator/plugins/orchestrator-form-widgets/src/widgets/ActiveMultiSelect.tsx

@@ -66,14 +66,13 @@ export const ActiveMultiSelect: Widget<
     () => (props.options.props ?? {}) as UiProps,
     [props.options.props],
   );
-  const isReadOnly = !!props.readonly;
+  const isReadOnly = !!props?.schema.readOnly;
 
   const autocompleteSelector =
     uiProps['fetch:response:autocomplete']?.toString();
   const mandatorySelector = uiProps['fetch:response:mandatory']?.toString();
   const defaultValueSelector = uiProps['fetch:response:value']?.toString();
-  const allowNewItems = uiProps['ui:allowNewItems'] === 'true';
-
+  const allowNewItems = uiProps['ui:allowNewItems'] === true;
   const [localError] = useState<string | undefined>(
     autocompleteSelector
       ? undefined
@@ -124,7 +123,17 @@ export const ActiveMultiSelect: Widget<
             true,
             true,
           );
-          setAutocompleteOptions(autocompleteValues);
+
+          // Only update if arrays differ (by item or count).
+          const arraysAreEqual =
+            Array.isArray(autocompleteOptions) &&
+            autocompleteValues.length === autocompleteOptions.length &&
+            [...autocompleteValues].sort().join(',') ===
+              [...autocompleteOptions].sort().join(',');
+
+          if (!arraysAreEqual) {
+            setAutocompleteOptions(autocompleteValues);
+          }
         }
 
         let defaults: string[] = [];
@@ -144,7 +153,17 @@ export const ActiveMultiSelect: Widget<
         let mandatory: string[] = [];
         if (mandatorySelector) {
           mandatory = await applySelectorArray(data, mandatorySelector, true);
-          setMandatoryValues(mandatory);
+
+          // Only update if arrays differ (by item or count).
+          const arraysAreEqual =
+            Array.isArray(mandatoryValues) &&
+            mandatory.length === mandatoryValues.length &&
+            [...mandatory].sort().join(',') ===
+              [...mandatoryValues].sort().join(',');
+
+          if (!arraysAreEqual) {
+            setMandatoryValues(mandatory);
+          }
         }
 
         if (
@@ -161,6 +180,8 @@ export const ActiveMultiSelect: Widget<
     autocompleteSelector,
     mandatorySelector,
     defaultValueSelector,
+    autocompleteOptions,
+    mandatoryValues,
     isChangedByUser,
     data,
     props.id,

workspaces/orchestrator/plugins/orchestrator-form-widgets/src/widgets/ActiveTextInput.tsx

@@ -64,7 +64,7 @@ export const ActiveTextInput: Widget<
     () => (props.options?.props ?? {}) as UiProps,
     [props.options?.props],
   );
-  const isReadOnly = !!props.readonly;
+  const isReadOnly = !!props?.schema.readOnly;
 
   const defaultValueSelector = uiProps['fetch:response:value']?.toString();
   const autocompleteSelector =
@@ -208,6 +208,7 @@ export const ActiveTextInput: Widget<
         data-testid={`${id}-textfield`}
         onChange={event => handleChange(event.target.value, true)}
         label={label}
+        disabled={isReadOnly}
       />
     </FormControl>
   );