Fixed lots of bugs where useEffect were being triggered repeatedly.

This commit is contained in:
Colin Dawson 2026-02-14 16:40:24 +00:00
parent 291a391faf
commit 1e1293f005
4 changed files with 179 additions and 54 deletions

View File

@ -15,6 +15,7 @@ export enum InputType {
hidden = "hidden", hidden = "hidden",
image = "image", image = "image",
month = "month", month = "month",
multiselect = "multiselect",
number = "number", number = "number",
password = "password", password = "password",
radio = "radio", radio = "radio",
@ -45,8 +46,13 @@ export interface InputProps {
step?: number; step?: number;
hidden?: boolean; hidden?: boolean;
autoComplete?: string; autoComplete?: string;
onChange?: (e: React.ChangeEvent<HTMLInputElement>) => void; onChange?: (
e: React.ChangeEvent<
HTMLInputElement | HTMLSelectElement | HTMLTextAreaElement
>,
) => void;
maxLength?: number; maxLength?: number;
options?: { value: string; label: string }[];
} }
function Input(props: InputProps) { function Input(props: InputProps) {
@ -64,6 +70,7 @@ function Input(props: InputProps) {
hidden, hidden,
autoComplete, autoComplete,
onChange, onChange,
options,
...rest ...rest
} = props; } = props;
@ -94,6 +101,7 @@ function Input(props: InputProps) {
type === InputType.password && showPasswordIcon === faEye type === InputType.password && showPasswordIcon === faEye
? InputType.text ? InputType.text
: type; : type;
const divEyeIconClassName = readOnly const divEyeIconClassName = readOnly
? "fullHeight disabledIcon" ? "fullHeight disabledIcon"
: "fullHeight"; : "fullHeight";
@ -109,7 +117,9 @@ function Input(props: InputProps) {
{label} {label}
</label> </label>
)} )}
<div className={flexClassName}> <div className={flexClassName}>
{/* TEXTAREA */}
{type === InputType.textarea && ( {type === InputType.textarea && (
<textarea <textarea
id={name} id={name}
@ -119,9 +129,41 @@ function Input(props: InputProps) {
disabled={readOnly} disabled={readOnly}
value={showValue ?? ""} value={showValue ?? ""}
autoComplete={autoComplete} autoComplete={autoComplete}
></textarea> />
)} )}
{type !== InputType.textarea && (
{/* MULTISELECT */}
{type === InputType.multiselect && (
<select
id={name}
className={className}
name={name}
multiple
disabled={readOnly}
value={(value as string[]) ?? []}
onChange={(e) => {
const selected = Array.from(e.target.selectedOptions).map(
(opt) => opt.value,
);
onChange?.({
...e,
target: {
...e.target,
value: selected,
},
} as any);
}}
>
{options?.map((opt) => (
<option key={opt.value} value={opt.value}>
{opt.label}
</option>
))}
</select>
)}
{/* ALL OTHER INPUT TYPES */}
{type !== InputType.textarea && type !== InputType.multiselect && (
<input <input
{...rest} {...rest}
id={name} id={name}
@ -138,6 +180,8 @@ function Input(props: InputProps) {
autoComplete={autoComplete} autoComplete={autoComplete}
/> />
)} )}
{/* PASSWORD TOGGLE */}
{type === InputType.password && ( {type === InputType.password && (
<div className={divEyeIconClassName}> <div className={divEyeIconClassName}>
<FontAwesomeIcon <FontAwesomeIcon
@ -151,6 +195,7 @@ function Input(props: InputProps) {
</div> </div>
)} )}
</div> </div>
<ErrorBlock error={error}></ErrorBlock> <ErrorBlock error={error}></ErrorBlock>
</div> </div>
); );

View File

@ -27,6 +27,7 @@ export const TaskCoreEditor: React.FC<TaskCoreEditorProps> = ({
const { t: tTaskType } = useTranslation(Namespaces.TaskTypes); const { t: tTaskType } = useTranslation(Namespaces.TaskTypes);
const [fieldErrors, setFieldErrors] = useState<Record<string, string>>({}); const [fieldErrors, setFieldErrors] = useState<Record<string, string>>({});
const prevErrorsRef = useRef<Record<string, string>>({}); const prevErrorsRef = useRef<Record<string, string>>({});
const hasAssignedDefaultName = useRef(false);
// Generate a unique default name // Generate a unique default name
const nameExists = (tasks: TaskDefinition[], candidate: string): boolean => { const nameExists = (tasks: TaskDefinition[], candidate: string): boolean => {
@ -57,27 +58,33 @@ export const TaskCoreEditor: React.FC<TaskCoreEditorProps> = ({
[allowedTasks, task.type, tTaskType], [allowedTasks, task.type, tTaskType],
); );
useEffect(() => {
if (!hasAssignedDefaultName.current && !task.config.name) {
hasAssignedDefaultName.current = true;
const displayName = allowedTasks.find(
(t) => t.taskType === task.type,
)?.displayName;
if (displayName) {
const newName = formatNewTaskName(allTasks);
onChange({
...task,
config: {
...task.config,
name: newName,
},
});
}
}
}, [allTasks, allowedTasks, formatNewTaskName, onChange, task]);
const runValidation = useCallback(() => { const runValidation = useCallback(() => {
const errors: Record<string, string> = {}; const errors: Record<string, string> = {};
// If the task doesn't have a name, generate a default one via onChange
if (!task.config.name) {
const newName = formatNewTaskName(allTasks);
onChange({
...task,
config: {
...task.config,
name: newName,
},
});
// Stop here — next render will validate again
return { errors: {}, isValid: true };
}
// Name required // Name required
if (!(task.config.name as string).trim()) { if (!(task.config.name as string)?.trim()) {
errors["name"] = "Name cannot be empty"; errors["name"] = "Name cannot be empty";
} }
@ -85,8 +92,8 @@ export const TaskCoreEditor: React.FC<TaskCoreEditorProps> = ({
const duplicate = allTasks.find( const duplicate = allTasks.find(
(t) => (t) =>
t.config.guid !== task.config.guid && t.config.guid !== task.config.guid &&
(t.config.name as string).trim().toLowerCase() === (t.config.name as string)?.trim().toLowerCase() ===
(task.config.name as string).trim().toLowerCase(), (task.config.name as string)?.trim().toLowerCase(),
); );
if (duplicate) { if (duplicate) {
@ -103,35 +110,58 @@ export const TaskCoreEditor: React.FC<TaskCoreEditorProps> = ({
`Description can be up to ${descriptionMaxLength} characters long.`; `Description can be up to ${descriptionMaxLength} characters long.`;
} }
// Predecessors must not include self
if (
(task.config.predecessors as string[])?.includes(
task.config.guid as string,
)
) {
errors["predecessors"] = "A task cannot depend on itself.";
}
// Predecessors must be unique
if (task.config.predecessors) {
const unique = new Set(task.config.predecessors as string[]);
if (unique.size !== (task.config.predecessors as string[]).length) {
errors["predecessors"] = "Duplicate predecessors are not allowed.";
}
}
const isValid = Object.keys(errors).length === 0; const isValid = Object.keys(errors).length === 0;
return { errors, isValid }; return { errors, isValid };
}, [task, allTasks, formatNewTaskName, onChange]); }, [task, allTasks]);
const prevInitialValidationRef = useRef<{
isValid: boolean;
errors: Record<string, string>;
} | null>(null);
// Validate when task changes (new task selected / created) // Validate when task changes (new task selected / created)
useEffect(() => { useEffect(() => {
const { errors, isValid } = runValidation(); const result = runValidation();
setFieldErrors(errors); const prev = prevInitialValidationRef.current;
prevErrorsRef.current = errors;
onValidate({ isValid, errors }); const changed =
}, [task.config.guid, runValidation, onValidate]); !prev ||
prev.isValid !== result.isValid ||
Object.keys(prev.errors).length !== Object.keys(result.errors).length ||
Object.entries(result.errors).some(([k, v]) => prev.errors[k] !== v);
// Validate when fields change if (changed) {
useEffect(() => { setFieldErrors(result.errors);
const { errors, isValid } = runValidation(); prevErrorsRef.current = result.errors;
onValidate(result);
const prevErrors = prevErrorsRef.current; prevInitialValidationRef.current = result;
const errorsChanged =
Object.keys(prevErrors).length !== Object.keys(errors).length ||
Object.entries(errors).some(([key, value]) => prevErrors[key] !== value);
if (errorsChanged) {
setFieldErrors(errors);
onValidate({ isValid, errors });
prevErrorsRef.current = errors;
} }
}, [task.config.name, task.config.description, runValidation, onValidate]); }, [
task.config.guid,
task.config.name,
task.config.description,
task.config.predecessors,
runValidation,
onValidate,
]);
return ( return (
<div> <div>
@ -152,6 +182,25 @@ export const TaskCoreEditor: React.FC<TaskCoreEditorProps> = ({
InputType.textarea, InputType.textarea,
fieldErrors["description"], fieldErrors["description"],
)} )}
{renderTaskField(
task,
onChange,
"predecessors",
"Predecessors",
InputType.multiselect,
fieldErrors["predecessors"],
"",
0,
{
options: allTasks
.filter((t) => t.config.guid !== task.config.guid)
.map((t) => ({
value: t.config.guid as string,
label: t.config.name as string,
})),
},
)}
</div> </div>
); );
}; };

View File

@ -1,5 +1,8 @@
import { useState } from "react"; import { useEffect, useRef, useState } from "react";
import { TaskDefinition } from "../services/WorkflowTemplateService"; import {
TaskDefinition,
TaskMetadata,
} from "../services/WorkflowTemplateService";
import { TaskCoreEditor } from "./CapabilityEditors/TaskCoreEditor"; import { TaskCoreEditor } from "./CapabilityEditors/TaskCoreEditor";
export interface TaskValidationResult { export interface TaskValidationResult {
@ -33,15 +36,22 @@ export const TaskEditor: React.FC<TaskEditorProps> = ({
) => { ) => {
setValidationMap((prev) => { setValidationMap((prev) => {
const updated = { ...prev, [capabilityName]: result }; const updated = { ...prev, [capabilityName]: result };
const allValid = Object.values(updated).every((r) => r.isValid);
onValidate(task.config.guid as string, allValid);
return updated; return updated;
}); });
}; };
const prevAllValidRef = useRef<boolean | null>(null);
useEffect(() => {
const allValid = Object.values(validationMap).every((r) => r.isValid);
// Only notify parent when the value actually changes
if (prevAllValidRef.current !== allValid) {
prevAllValidRef.current = allValid;
onValidate(task.config.guid as string, allValid);
}
}, [validationMap, task.config.guid, onValidate]);
return ( return (
<> <>
<TaskCoreEditor <TaskCoreEditor

View File

@ -10,13 +10,22 @@ export const renderTaskField = (
error?: string, error?: string,
placeholder?: string, placeholder?: string,
maxLength?: number, maxLength?: number,
extraProps?: {
options?: { value: string; label: string }[];
},
) => { ) => {
const handleChange = (e: React.ChangeEvent<HTMLInputElement>) => { const handleChange = (
e: React.ChangeEvent<
HTMLInputElement | HTMLSelectElement | HTMLTextAreaElement
>,
) => {
const newValue = e.target.value;
onChange({ onChange({
...task, ...task,
config: { config: {
...task.config, ...task.config,
[field]: e.target.value, [field]: newValue,
}, },
}); });
}; };
@ -31,32 +40,44 @@ export const renderTaskField = (
false, false,
placeholder ?? "", placeholder ?? "",
maxLength ?? 0, maxLength ?? 0,
extraProps,
); );
}; };
export const renderTaskInput = ( export const renderTaskInput = (
name: string, name: string,
label: string, label: string,
value: string | number | undefined, value: string | number | readonly string[] | undefined,
error: string | undefined, error: string | undefined,
type: InputType = InputType.text, type: InputType = InputType.text,
onChange: (e: React.ChangeEvent<HTMLInputElement>) => void, onChange: (
e: React.ChangeEvent<HTMLInputElement | HTMLSelectElement>,
) => void,
readOnly: boolean = false, readOnly: boolean = false,
placeholder: string = "", placeholder: string = "",
maxLength: number = 0, maxLength: number = 0,
extraProps?: {
options?: { value: string; label: string }[];
},
) => { ) => {
const normalisedValue =
type === InputType.multiselect
? ((value as string[]) ?? [])
: (value ?? "");
return ( return (
<Input <Input
includeLabel={true} includeLabel={true}
type={type} type={type}
name={name} name={name}
label={label} label={label}
value={value ?? ""} value={normalisedValue}
error={error} error={error}
maxLength={maxLength} maxLength={maxLength}
onChange={onChange} onChange={onChange}
readOnly={readOnly} readOnly={readOnly}
placeHolder={placeholder} placeHolder={placeholder}
{...extraProps}
/> />
); );
}; };