Skip to content

Commit c73ee9e

Browse files
committed
improvement(workflows): remove dead complexity, fix mutation edge cases
- Throw on state PUT failure in useCreateWorkflow instead of swallowing - Use Map for O(1) lookups in duplicate/export loops (3 hooks) - Broaden invalidation scope in update/delete mutations to lists() - Switch workflow-block to useWorkflowMap for direct ID lookup - Consolidate use-workflow-operations to single useWorkflowMap hook - Remove workspace transition guard (sync body, unreachable timeout) - Make switchToWorkspace synchronous (remove async/try-catch/finally)
1 parent 0183e24 commit c73ee9e

9 files changed

Lines changed: 39 additions & 96 deletions

File tree

apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/workflow-block/workflow-block.tsx

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ import { useCredentialName } from '@/hooks/queries/oauth/oauth-credentials'
4646
import { useReactivateSchedule, useScheduleInfo } from '@/hooks/queries/schedules'
4747
import { useSkills } from '@/hooks/queries/skills'
4848
import { useTablesList } from '@/hooks/queries/tables'
49-
import { useWorkflows } from '@/hooks/queries/workflows'
49+
import { useWorkflowMap } from '@/hooks/queries/workflows'
5050
import { useSelectorDisplayName } from '@/hooks/use-selector-display-name'
5151
import { useVariablesStore } from '@/stores/panel'
5252
import { useSubBlockStore } from '@/stores/workflows/subblock/store'
@@ -600,11 +600,11 @@ const SubBlockRow = memo(function SubBlockRow({
600600
)
601601
const knowledgeBaseDisplayName = kbForDisplayName?.name ?? null
602602

603-
const { data: workflowListForLookup } = useWorkflows(workspaceId)
603+
const { data: workflowMapForLookup = {} } = useWorkflowMap(workspaceId)
604604
const workflowSelectionName = useMemo(() => {
605605
if (subBlock?.id !== 'workflowId' || typeof rawValue !== 'string') return null
606-
return (workflowListForLookup ?? []).find((w) => w.id === rawValue)?.name ?? null
607-
}, [workflowListForLookup, subBlock?.id, rawValue])
606+
return workflowMapForLookup[rawValue]?.name ?? null
607+
}, [workflowMapForLookup, subBlock?.id, rawValue])
608608

609609
const { data: mcpServers = [] } = useMcpServers(workspaceId || '')
610610
const mcpServerDisplayName = useMemo(() => {

apps/sim/app/workspace/[workspaceId]/w/components/sidebar/hooks/use-workflow-operations.ts

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { useCallback, useMemo } from 'react'
22
import { createLogger } from '@sim/logger'
33
import { useRouter } from 'next/navigation'
44
import { getNextWorkflowColor } from '@/lib/workflows/colors'
5-
import { useCreateWorkflow, useWorkflowMap, useWorkflows } from '@/hooks/queries/workflows'
5+
import { useCreateWorkflow, useWorkflowMap } from '@/hooks/queries/workflows'
66
import { useWorkflowDiffStore } from '@/stores/workflow-diff/store'
77
import { generateCreativeWorkflowName } from '@/stores/workflows/registry/utils'
88

@@ -14,19 +14,15 @@ interface UseWorkflowOperationsProps {
1414

1515
export function useWorkflowOperations({ workspaceId }: UseWorkflowOperationsProps) {
1616
const router = useRouter()
17-
const workflowsQuery = useWorkflows(workspaceId)
18-
const { data: workflowList = [] } = workflowsQuery
19-
const { data: workflows = {} } = useWorkflowMap(workspaceId)
17+
const { data: workflows = {}, isLoading: workflowsLoading } = useWorkflowMap(workspaceId)
2018
const createWorkflowMutation = useCreateWorkflow()
2119

2220
const regularWorkflows = useMemo(
2321
() =>
24-
workflowList
22+
Object.values(workflows)
2523
.filter((workflow) => workflow.workspaceId === workspaceId)
26-
.sort((a, b) => {
27-
return b.createdAt.getTime() - a.createdAt.getTime()
28-
}),
29-
[workflowList, workspaceId]
24+
.sort((a, b) => b.createdAt.getTime() - a.createdAt.getTime()),
25+
[workflows, workspaceId]
3026
)
3127

3228
const handleCreateWorkflow = useCallback(async (): Promise<string | null> => {
@@ -58,7 +54,7 @@ export function useWorkflowOperations({ workspaceId }: UseWorkflowOperationsProp
5854
return {
5955
workflows,
6056
regularWorkflows,
61-
workflowsLoading: workflowsQuery.isLoading,
57+
workflowsLoading,
6258
isCreatingWorkflow: createWorkflowMutation.isPending,
6359

6460
handleCreateWorkflow,

apps/sim/app/workspace/[workspaceId]/w/components/sidebar/hooks/use-workspace-management.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ export function useWorkspaceManagement({
110110
}
111111

112112
try {
113-
await switchToWorkspace(workspace.id)
113+
switchToWorkspace(workspace.id)
114114
routerRef.current?.push(`/workspace/${workspace.id}/home`)
115115
logger.info(`Switched to workspace: ${workspace.name} (${workspace.id})`)
116116
} catch (error) {

apps/sim/app/workspace/[workspaceId]/w/hooks/use-duplicate-selection.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ export function useDuplicateSelection({ workspaceId, onSuccess }: UseDuplicateSe
6161

6262
setIsDuplicating(true)
6363
try {
64-
const workflows = getWorkflows(workspaceIdRef.current)
64+
const workflowMap = new Map(getWorkflows(workspaceIdRef.current).map((w) => [w.id, w]))
6565
const folderStore = useFolderStore.getState()
6666

6767
const duplicatedWorkflowIds: string[] = []
@@ -96,7 +96,7 @@ export function useDuplicateSelection({ workspaceId, onSuccess }: UseDuplicateSe
9696
}
9797

9898
for (const workflowId of workflowIds) {
99-
const workflow = workflows.find((w) => w.id === workflowId)
99+
const workflow = workflowMap.get(workflowId)
100100
if (!workflow) {
101101
logger.warn(`Workflow ${workflowId} not found, skipping`)
102102
continue

apps/sim/app/workspace/[workspaceId]/w/hooks/use-duplicate-workflow.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,10 +60,10 @@ export function useDuplicateWorkflow({ workspaceId, onSuccess }: UseDuplicateWor
6060
const duplicatedIds: string[] = []
6161

6262
try {
63-
const workflows = getWorkflows(workspaceIdRef.current)
63+
const workflowMap = new Map(getWorkflows(workspaceIdRef.current).map((w) => [w.id, w]))
6464

6565
for (const sourceId of workflowIdsToDuplicate) {
66-
const sourceWorkflow = workflows.find((w) => w.id === sourceId)
66+
const sourceWorkflow = workflowMap.get(sourceId)
6767
if (!sourceWorkflow) {
6868
logger.warn(`Workflow ${sourceId} not found, skipping`)
6969
continue

apps/sim/app/workspace/[workspaceId]/w/hooks/use-export-workflow.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,11 +58,11 @@ export function useExportWorkflow({ onSuccess }: UseExportWorkflowProps = {}) {
5858
count: workflowIdsToExport.length,
5959
})
6060

61-
const workflows = getWorkflows(workspaceIdRef.current)
61+
const workflowMap = new Map(getWorkflows(workspaceIdRef.current).map((w) => [w.id, w]))
6262
const exportedWorkflows = []
6363

6464
for (const workflowId of workflowIdsToExport) {
65-
const workflowMeta = workflows.find((w) => w.id === workflowId)
65+
const workflowMeta = workflowMap.get(workflowId)
6666
if (!workflowMeta) {
6767
logger.warn(`Workflow ${workflowId} not found in registry`)
6868
continue

apps/sim/hooks/queries/workflows.ts

Lines changed: 6 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -61,9 +61,7 @@ async function fetchWorkflowState(
6161
export function useWorkflowState(workflowId: string | undefined) {
6262
return useQuery({
6363
queryKey: workflowKeys.state(workflowId),
64-
queryFn: workflowId
65-
? ({ signal }) => fetchWorkflowState(workflowId, signal)
66-
: skipToken,
64+
queryFn: workflowId ? ({ signal }) => fetchWorkflowState(workflowId, signal) : skipToken,
6765
staleTime: 30 * 1000,
6866
})
6967
}
@@ -118,9 +116,7 @@ export function useWorkflows(workspaceId?: string, options?: { scope?: WorkflowQ
118116

119117
return useQuery({
120118
queryKey: workflowKeys.list(workspaceId, scope),
121-
queryFn: workspaceId
122-
? ({ signal }) => fetchWorkflows(workspaceId, scope, signal)
123-
: skipToken,
119+
queryFn: workspaceId ? ({ signal }) => fetchWorkflows(workspaceId, scope, signal) : skipToken,
124120
placeholderData: keepPreviousData,
125121
staleTime: 60 * 1000,
126122
})
@@ -136,9 +132,7 @@ export function useWorkflowMap(workspaceId?: string, options?: { scope?: Workflo
136132

137133
return useQuery({
138134
queryKey: workflowKeys.list(workspaceId, scope),
139-
queryFn: workspaceId
140-
? ({ signal }) => fetchWorkflows(workspaceId, scope, signal)
141-
: skipToken,
135+
queryFn: workspaceId ? ({ signal }) => fetchWorkflows(workspaceId, scope, signal) : skipToken,
142136
placeholderData: keepPreviousData,
143137
staleTime: 60 * 1000,
144138
select: (data) => Object.fromEntries(data.map((w) => [w.id, w])),
@@ -213,9 +207,7 @@ export function useCreateWorkflow() {
213207

214208
if (!stateResponse.ok) {
215209
const text = await stateResponse.text()
216-
logger.error('Failed to persist default Start block:', text)
217-
} else {
218-
logger.info('Successfully persisted default Start block')
210+
throw new Error(`Failed to persist default workflow state: ${text}`)
219211
}
220212

221213
return {
@@ -570,7 +562,7 @@ export function useUpdateWorkflow() {
570562
},
571563
onSettled: (_data, _error, variables) => {
572564
queryClient.invalidateQueries({
573-
queryKey: workflowKeys.list(variables.workspaceId, 'active'),
565+
queryKey: workflowKeys.lists(),
574566
})
575567
},
576568
})
@@ -623,7 +615,7 @@ export function useDeleteWorkflowMutation() {
623615
},
624616
onSettled: (_data, _error, variables) => {
625617
queryClient.invalidateQueries({
626-
queryKey: workflowKeys.list(variables.workspaceId, 'active'),
618+
queryKey: workflowKeys.lists(),
627619
})
628620
},
629621
})

apps/sim/stores/workflows/registry/store.ts

Lines changed: 15 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,6 @@ const initialHydration: HydrationState = {
2727

2828
const createRequestId = () => `${Date.now()}-${Math.random().toString(16).slice(2)}`
2929

30-
let isWorkspaceTransitioning = false
31-
const TRANSITION_TIMEOUT = 5000
32-
3330
function resetWorkflowStores() {
3431
useWorkflowStore.setState({
3532
blocks: {},
@@ -45,19 +42,6 @@ function resetWorkflowStores() {
4542
})
4643
}
4744

48-
function setWorkspaceTransitioning(isTransitioning: boolean): void {
49-
isWorkspaceTransitioning = isTransitioning
50-
51-
if (isTransitioning) {
52-
setTimeout(() => {
53-
if (isWorkspaceTransitioning) {
54-
logger.warn('Forcing workspace transition to complete due to timeout')
55-
isWorkspaceTransitioning = false
56-
}
57-
}, TRANSITION_TIMEOUT)
58-
}
59-
}
60-
6145
export const useWorkflowRegistry = create<WorkflowRegistry>()(
6246
devtools(
6347
(set, get) => ({
@@ -68,53 +52,24 @@ export const useWorkflowRegistry = create<WorkflowRegistry>()(
6852
clipboard: null,
6953
pendingSelection: null,
7054

71-
switchToWorkspace: async (workspaceId: string) => {
72-
if (isWorkspaceTransitioning) {
73-
logger.warn(
74-
`Ignoring workspace switch to ${workspaceId} - transition already in progress`
75-
)
76-
return
77-
}
78-
79-
setWorkspaceTransitioning(true)
55+
switchToWorkspace: (workspaceId: string) => {
56+
logger.info(`Switching to workspace: ${workspaceId}`)
8057

81-
try {
82-
logger.info(`Switching to workspace: ${workspaceId}`)
83-
84-
resetWorkflowStores()
85-
86-
// Invalidate the old workspace workflow cache so a fresh fetch happens
87-
getQueryClient().invalidateQueries({ queryKey: workflowKeys.lists() })
58+
resetWorkflowStores()
59+
getQueryClient().invalidateQueries({ queryKey: workflowKeys.lists() })
8860

89-
set({
90-
activeWorkflowId: null,
91-
deploymentStatuses: {},
61+
set({
62+
activeWorkflowId: null,
63+
deploymentStatuses: {},
64+
error: null,
65+
hydration: {
66+
phase: 'idle',
67+
workspaceId,
68+
workflowId: null,
69+
requestId: null,
9270
error: null,
93-
hydration: {
94-
phase: 'idle',
95-
workspaceId,
96-
workflowId: null,
97-
requestId: null,
98-
error: null,
99-
},
100-
})
101-
102-
logger.info(`Successfully switched to workspace: ${workspaceId}`)
103-
} catch (error) {
104-
logger.error(`Error switching to workspace ${workspaceId}:`, { error })
105-
set({
106-
error: `Failed to switch workspace: ${error instanceof Error ? error.message : 'Unknown error'}`,
107-
hydration: {
108-
phase: 'error',
109-
workspaceId,
110-
workflowId: null,
111-
requestId: null,
112-
error: error instanceof Error ? error.message : 'Unknown error',
113-
},
114-
})
115-
} finally {
116-
setWorkspaceTransitioning(false)
117-
}
71+
},
72+
})
11873
},
11974

12075
getWorkflowDeploymentStatus: (workflowId: string | null): DeploymentStatus | null => {

apps/sim/stores/workflows/registry/types.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ export interface WorkflowRegistryState {
5454
export interface WorkflowRegistryActions {
5555
setActiveWorkflow: (id: string) => Promise<void>
5656
loadWorkflowState: (workflowId: string) => Promise<void>
57-
switchToWorkspace: (id: string) => Promise<void>
57+
switchToWorkspace: (id: string) => void
5858
getWorkflowDeploymentStatus: (workflowId: string | null) => DeploymentStatus | null
5959
setDeploymentStatus: (
6060
workflowId: string | null,

0 commit comments

Comments
 (0)