Skip to content

Commit b6a2255

Browse files
committed
fix(deploy): address PR review feedback
- Remove redundant isLoading check (subset of isPending in RQ v5) - Make deployedState prop nullable to avoid non-null assertion - Destructure mutateAsync to eliminate ESLint suppression - Guard debounced invalidation against stale workflowId on switch
1 parent 1fcff2b commit b6a2255

File tree

6 files changed

+31
-21
lines changed

6 files changed

+31
-21
lines changed

apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/components/deploy-modal/components/general/general.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ const logger = createLogger('GeneralDeploy')
2626

2727
interface GeneralDeployProps {
2828
workflowId: string | null
29-
deployedState: WorkflowState
29+
deployedState?: WorkflowState | null
3030
isLoadingDeployedState: boolean
3131
versions: WorkflowDeploymentVersionResponse[]
3232
versionsLoading: boolean

apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/components/deploy-modal/deploy-modal.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ interface DeployModalProps {
5959
workflowId: string | null
6060
isDeployed: boolean
6161
needsRedeployment: boolean
62-
deployedState: WorkflowState
62+
deployedState?: WorkflowState | null
6363
isLoadingDeployedState: boolean
6464
}
6565

apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/deploy.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ export function Deploy({ activeWorkflowId, userPermissions, className }: DeployP
128128
workflowId={activeWorkflowId}
129129
isDeployed={isDeployed}
130130
needsRedeployment={changeDetected}
131-
deployedState={deployedState!}
131+
deployedState={deployedState}
132132
isLoadingDeployedState={isLoadingDeployedState}
133133
/>
134134
</>

apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/hooks/use-change-detection.ts

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,14 @@ export function useChangeDetection({
5858
// Track initial lastSaved to detect saves after load.
5959
// Debounced to avoid redundant API calls during rapid auto-saves.
6060
const initialLastSavedRef = useRef<number | undefined>(undefined)
61+
const workflowIdRef = useRef(workflowId)
62+
63+
// Reset tracking when workflow changes — must run before the lastSaved effect
64+
// to prevent spurious invalidation with a stale ref during workflow switches.
65+
useEffect(() => {
66+
workflowIdRef.current = workflowId
67+
initialLastSavedRef.current = undefined
68+
}, [workflowId])
6169

6270
useEffect(() => {
6371
if (lastSaved !== undefined && initialLastSavedRef.current === undefined) {
@@ -76,20 +84,17 @@ export function useChangeDetection({
7684

7785
initialLastSavedRef.current = lastSaved
7886

87+
const capturedWorkflowId = workflowId
7988
const timer = setTimeout(() => {
89+
if (workflowIdRef.current !== capturedWorkflowId) return
8090
queryClient.invalidateQueries({
81-
queryKey: deploymentKeys.info(workflowId),
91+
queryKey: deploymentKeys.info(capturedWorkflowId),
8292
})
8393
}, 500)
8494

8595
return () => clearTimeout(timer)
8696
}, [lastSaved, workflowId, queryClient])
8797

88-
// Reset tracking when workflow changes
89-
useEffect(() => {
90-
initialLastSavedRef.current = undefined
91-
}, [workflowId])
92-
9398
// Skip expensive state merge when server result is available (the common path).
9499
// Only build currentState for the client-side fallback comparison.
95100
const needsClientFallback = serverNeedsRedeployment === undefined && !isServerLoading
@@ -106,7 +111,16 @@ export function useChangeDetection({
106111
parallels,
107112
variables: workflowVariables,
108113
} as WorkflowState & { variables: Record<string, any> }
109-
}, [needsClientFallback, workflowId, blocks, edges, loops, parallels, subBlockValues, workflowVariables])
114+
}, [
115+
needsClientFallback,
116+
workflowId,
117+
blocks,
118+
edges,
119+
loops,
120+
parallels,
121+
subBlockValues,
122+
workflowVariables,
123+
])
110124

111125
const changeDetected = useMemo(() => {
112126
if (isServerLoading) return false

apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/deploy/hooks/use-deployment.ts

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,8 @@ interface UseDeploymentProps {
1515
* First deploy: runs pre-deploy checks, then deploys via mutation and opens modal.
1616
* Already deployed: opens modal directly (validation happens on Update in modal).
1717
*/
18-
export function useDeployment({
19-
workflowId,
20-
isDeployed,
21-
}: UseDeploymentProps) {
22-
const deployMutation = useDeployWorkflow()
18+
export function useDeployment({ workflowId, isDeployed }: UseDeploymentProps) {
19+
const { mutateAsync, isPending: isDeploying } = useDeployWorkflow()
2320
const addNotification = useNotificationStore((state) => state.addNotification)
2421

2522
const handleDeployClick = useCallback(async () => {
@@ -48,7 +45,7 @@ export function useDeployment({
4845
}
4946

5047
try {
51-
await deployMutation.mutateAsync({ workflowId, deployChatEnabled: false })
48+
await mutateAsync({ workflowId, deployChatEnabled: false })
5249
return { success: true, shouldOpenModal: true }
5350
} catch (error) {
5451
const errorMessage = error instanceof Error ? error.message : 'Failed to deploy workflow'
@@ -59,11 +56,10 @@ export function useDeployment({
5956
})
6057
return { success: false, shouldOpenModal: false }
6158
}
62-
// eslint-disable-next-line react-hooks/exhaustive-deps -- mutateAsync is a stable reference
63-
}, [workflowId, isDeployed, addNotification])
59+
}, [workflowId, isDeployed, addNotification, mutateAsync])
6460

6561
return {
66-
isDeploying: deployMutation.isPending,
62+
isDeploying,
6763
handleDeployClick,
6864
}
6965
}

apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/workflow-block/hooks/use-child-workflow.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,13 +51,13 @@ export function useChildWorkflow(
5151
}
5252
}
5353

54-
const { data, isLoading, isPending } = useDeploymentInfo(
54+
const { data, isPending } = useDeploymentInfo(
5555
isWorkflowSelector ? (childWorkflowId ?? null) : null
5656
)
5757

5858
const childIsDeployed = data?.isDeployed ?? null
5959
const childNeedsRedeploy = data?.needsRedeployment ?? false
60-
const isLoadingChildVersion = isLoading || isPending
60+
const isLoadingChildVersion = isPending
6161

6262
return {
6363
childWorkflowId,

0 commit comments

Comments
 (0)