fix(ui): handle React 18 batching for "Submit" button on details page. Fixes #13453

[MasonM]

Fixes #13453

Motivation

The React 18 upgrade in #13069 introduced a bug on the details page where the "Submit" button is disabled immediately on page load. I believe this is happening due to batching changes in React 18 that affect the following two useEffect() calls, which are common to all the components modified in this PR:

argo-workflows/ui/src/app/event-sources/event-source-details.tsx

Lines 68 to 81 in 729ac17

	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 useEffect() call 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 (the behavior in React 18) or fired separately (the old behavior).

Modifications

This PR turns the edited variable into derived state that dynamically compares objects using JSON.stringify(), which eliminates the need for useEffect(() => setEdited(true), [template]) call, and therefore fixes the race condition.

This will have a slight performance hit, which we could solve using useMemo(), but I wasn't sure if that was worth it.

Verification

Tested locally by editing a template at http://localhost:8081/workflow-templates?namespace=argo:

Screen.Recording.2024-09-12.at.4.53.59.PM.mp4

[agilgur5]

Hmm, so per #13410 (comment) and #13069 (comment), I was planning to revert the React upgrade on main (and did not cherry-pick it).

That being said, the difference in behavior should be handled regardless, but this may be lower priority as a result

[agilgur5]

Nice root cause analysis! We might want to double-check some other places for batching bugs too 🤔 seems like React didn't have any auto-detection for this unfortunately

I left some mostly stylistic comments below, with one behavior correction.

This will have a slight performance hit, which we could solve using useMemo(), but I wasn't sure if that was worth it.

Regarding useMemo, in this case I think it would be worth it, since these are all editor components, so they are latency sensitive to user input

[MasonM]

@agilgur5 Thanks for the quick and thorough review! I pushed a couple commits to fix the stylistic issues, switch to useMemo(), and fix the bug on sensor-details.tsx