mirror of
https://github.com/snapotter-hq/SnapOtter.git
synced 2026-08-03 07:46:42 +02:00
fix: resolve pipeline step race condition and infinite re-render loop
Two issues caused intermittent step addition failures in the automation pipeline: 1. RemoveBgControls had onChange in its useEffect deps. Since onChange is a new function reference on every parent render, this created an infinite re-render loop (effect -> setState -> render -> effect). Fixed by using the onChangeRef pattern matching other settings components. 2. All step mutation callbacks read from stepsRef.current and passed values to setSteps. Concurrent callbacks (e.g. addStep + a settings effect) would overwrite each other. Fixed by switching to functional state updates (setSteps(prev => ...)) and removing stepsRef. Also rewrites automate e2e tests to use manual step addition instead of referencing templates that no longer exist in the UI.
This commit is contained in:
@@ -13,7 +13,7 @@ import {
|
||||
Upload,
|
||||
X,
|
||||
} from "lucide-react";
|
||||
import { useCallback, useEffect, useMemo, useRef, useState } from "react";
|
||||
import { type SetStateAction, useCallback, useEffect, useMemo, useState } from "react";
|
||||
import { SearchBar } from "@/components/common/search-bar";
|
||||
import { apiGet } from "@/lib/api";
|
||||
import { cn } from "@/lib/utils";
|
||||
@@ -32,7 +32,7 @@ export interface PipelineStep {
|
||||
|
||||
interface PipelineBuilderProps {
|
||||
steps: PipelineStep[];
|
||||
onStepsChange: (steps: PipelineStep[]) => void;
|
||||
onStepsChange: (action: SetStateAction<PipelineStep[]>) => void;
|
||||
onSave: (name: string, description: string) => void;
|
||||
onExecute: (file: File) => void;
|
||||
saving?: boolean;
|
||||
@@ -67,12 +67,6 @@ export function PipelineBuilder({
|
||||
const [pipelineToolIds, setPipelineToolIds] = useState<string[] | null>(null);
|
||||
const [toolSearch, setToolSearch] = useState("");
|
||||
|
||||
// Keep a ref to steps so callbacks always read the latest value
|
||||
// without needing steps in their dependency arrays (prevents stale closures).
|
||||
// Assigned during render (not useEffect) so it's current before child effects fire.
|
||||
const stepsRef = useRef(steps);
|
||||
stepsRef.current = steps;
|
||||
|
||||
useEffect(() => {
|
||||
apiGet<{ settings: Record<string, string> }>("/v1/settings")
|
||||
.then((data) => {
|
||||
@@ -111,7 +105,7 @@ export function PipelineBuilder({
|
||||
toolId,
|
||||
settings: {},
|
||||
};
|
||||
onStepsChange([...stepsRef.current, step]);
|
||||
onStepsChange((prev) => [...prev, step]);
|
||||
setShowToolPicker(false);
|
||||
setToolSearch("");
|
||||
setExpandedStep(step.id);
|
||||
@@ -121,7 +115,7 @@ export function PipelineBuilder({
|
||||
|
||||
const removeStep = useCallback(
|
||||
(id: string) => {
|
||||
onStepsChange(stepsRef.current.filter((s) => s.id !== id));
|
||||
onStepsChange((prev) => prev.filter((s) => s.id !== id));
|
||||
setExpandedStep((prev) => (prev === id ? null : prev));
|
||||
},
|
||||
[onStepsChange],
|
||||
@@ -129,23 +123,22 @@ export function PipelineBuilder({
|
||||
|
||||
const moveStep = useCallback(
|
||||
(id: string, direction: "up" | "down") => {
|
||||
const current = stepsRef.current;
|
||||
const idx = current.findIndex((s) => s.id === id);
|
||||
if (idx < 0) return;
|
||||
const newIdx = direction === "up" ? idx - 1 : idx + 1;
|
||||
if (newIdx < 0 || newIdx >= current.length) return;
|
||||
const newSteps = [...current];
|
||||
[newSteps[idx], newSteps[newIdx]] = [newSteps[newIdx], newSteps[idx]];
|
||||
onStepsChange(newSteps);
|
||||
onStepsChange((prev) => {
|
||||
const idx = prev.findIndex((s) => s.id === id);
|
||||
if (idx < 0) return prev;
|
||||
const newIdx = direction === "up" ? idx - 1 : idx + 1;
|
||||
if (newIdx < 0 || newIdx >= prev.length) return prev;
|
||||
const newSteps = [...prev];
|
||||
[newSteps[idx], newSteps[newIdx]] = [newSteps[newIdx], newSteps[idx]];
|
||||
return newSteps;
|
||||
});
|
||||
},
|
||||
[onStepsChange],
|
||||
);
|
||||
|
||||
const updateStepSettings = useCallback(
|
||||
(id: string, newSettings: Record<string, unknown>) => {
|
||||
onStepsChange(
|
||||
stepsRef.current.map((s) => (s.id === id ? { ...s, settings: newSettings } : s)),
|
||||
);
|
||||
onStepsChange((prev) => prev.map((s) => (s.id === id ? { ...s, settings: newSettings } : s)));
|
||||
},
|
||||
[onStepsChange],
|
||||
);
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { Download, ImageIcon, Package, User } from "lucide-react";
|
||||
import { useEffect, useState } from "react";
|
||||
import { useEffect, useRef, useState } from "react";
|
||||
import { ProgressCard } from "@/components/common/progress-card";
|
||||
import { useToolProcessor } from "@/hooks/use-tool-processor";
|
||||
import { useFileStore } from "@/stores/file-store";
|
||||
@@ -56,12 +56,17 @@ export function RemoveBgControls({ settings, onChange }: RemoveBgControlsProps)
|
||||
|
||||
const model = isPassport ? "birefnet-portrait" : MODEL_MAP[subject][quality];
|
||||
|
||||
const onChangeRef = useRef(onChange);
|
||||
useEffect(() => {
|
||||
onChangeRef.current = onChange;
|
||||
});
|
||||
|
||||
// Sync settings on every control change
|
||||
useEffect(() => {
|
||||
const next: Record<string, unknown> = { model };
|
||||
if (bgColor) next.backgroundColor = bgColor;
|
||||
onChange(next);
|
||||
}, [model, bgColor, onChange]);
|
||||
onChangeRef.current(next);
|
||||
}, [model, bgColor]);
|
||||
|
||||
return (
|
||||
<div className="space-y-4">
|
||||
|
||||
Reference in New Issue
Block a user