Skip to content

Commit 6744043

Browse files
authored
fix(ops): fix subflow resizing on exit (#2760)
* fix(sockets): broadcast handles and enabled/disabled state * made all ops batched, removed all individual ops * fix subflow resizing on exit * removed unused custom event * fix failing tests, update testing * fix test mock
1 parent 47eb060 commit 6744043

File tree

16 files changed

+759
-808
lines changed

16 files changed

+759
-808
lines changed

apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/condition-input/condition-input.tsx

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,7 @@ export function ConditionInput({
156156
[key: string]: number[]
157157
}>({})
158158
const updateNodeInternals = useUpdateNodeInternals()
159-
const removeEdge = useWorkflowStore((state) => state.removeEdge)
159+
const batchRemoveEdges = useWorkflowStore((state) => state.batchRemoveEdges)
160160
const edges = useWorkflowStore((state) => state.edges)
161161

162162
const prevStoreValueRef = useRef<string | null>(null)
@@ -657,11 +657,12 @@ export function ConditionInput({
657657
if (isPreview || disabled || conditionalBlocks.length <= 2) return
658658

659659
// Remove any associated edges before removing the block
660-
edges.forEach((edge) => {
661-
if (edge.sourceHandle?.startsWith(`condition-${id}`)) {
662-
removeEdge(edge.id)
663-
}
664-
})
660+
const edgeIdsToRemove = edges
661+
.filter((edge) => edge.sourceHandle?.startsWith(`condition-${id}`))
662+
.map((edge) => edge.id)
663+
if (edgeIdsToRemove.length > 0) {
664+
batchRemoveEdges(edgeIdsToRemove)
665+
}
665666

666667
if (conditionalBlocks.length === 1) return
667668
shouldPersistRef.current = true

apps/sim/app/workspace/[workspaceId]/w/[workflowId]/hooks/use-node-utilities.ts

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -373,16 +373,20 @@ export function useNodeUtilities(blocks: Record<string, any>) {
373373
* Updates a node's parent with proper position calculation
374374
* @param nodeId ID of the node being reparented
375375
* @param newParentId ID of the new parent (or null to remove parent)
376-
* @param updateBlockPosition Function to update the position of a block
377-
* @param updateParentId Function to update the parent ID of a block
376+
* @param batchUpdatePositions Function to batch update positions of blocks
377+
* @param batchUpdateBlocksWithParent Function to batch update blocks with parent info
378378
* @param resizeCallback Function to resize loop nodes after parent update
379379
*/
380380
const updateNodeParent = useCallback(
381381
(
382382
nodeId: string,
383383
newParentId: string | null,
384-
updateBlockPosition: (id: string, position: { x: number; y: number }) => void,
385-
updateParentId: (id: string, parentId: string, extent: 'parent') => void,
384+
batchUpdatePositions: (
385+
updates: Array<{ id: string; position: { x: number; y: number } }>
386+
) => void,
387+
batchUpdateBlocksWithParent: (
388+
updates: Array<{ id: string; position: { x: number; y: number }; parentId?: string }>
389+
) => void,
386390
resizeCallback: () => void
387391
) => {
388392
const node = getNodes().find((n) => n.id === nodeId)
@@ -394,15 +398,15 @@ export function useNodeUtilities(blocks: Record<string, any>) {
394398
if (newParentId) {
395399
const relativePosition = calculateRelativePosition(nodeId, newParentId)
396400

397-
updateBlockPosition(nodeId, relativePosition)
398-
updateParentId(nodeId, newParentId, 'parent')
401+
batchUpdatePositions([{ id: nodeId, position: relativePosition }])
402+
batchUpdateBlocksWithParent([
403+
{ id: nodeId, position: relativePosition, parentId: newParentId },
404+
])
399405
} else if (currentParentId) {
400406
const absolutePosition = getNodeAbsolutePosition(nodeId)
401407

402-
// First set the absolute position so the node visually stays in place
403-
updateBlockPosition(nodeId, absolutePosition)
404-
// Then clear the parent relationship in the store (empty string removes parentId/extent)
405-
updateParentId(nodeId, '', 'parent')
408+
batchUpdatePositions([{ id: nodeId, position: absolutePosition }])
409+
batchUpdateBlocksWithParent([{ id: nodeId, position: absolutePosition, parentId: '' }])
406410
}
407411

408412
resizeCallback()

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

Lines changed: 85 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -443,11 +443,9 @@ const WorkflowContent = React.memo(() => {
443443
}, [userPermissions, currentWorkflow.isSnapshotView])
444444

445445
const {
446-
collaborativeAddEdge: addEdge,
447-
collaborativeRemoveEdge: removeEdge,
446+
collaborativeBatchAddEdges,
448447
collaborativeBatchRemoveEdges,
449448
collaborativeBatchUpdatePositions,
450-
collaborativeUpdateParentId: updateParentId,
451449
collaborativeBatchUpdateParent,
452450
collaborativeBatchAddBlocks,
453451
collaborativeBatchRemoveBlocks,
@@ -464,6 +462,34 @@ const WorkflowContent = React.memo(() => {
464462
[collaborativeBatchUpdatePositions]
465463
)
466464

465+
const addEdge = useCallback(
466+
(edge: Edge) => {
467+
collaborativeBatchAddEdges([edge])
468+
},
469+
[collaborativeBatchAddEdges]
470+
)
471+
472+
const removeEdge = useCallback(
473+
(edgeId: string) => {
474+
collaborativeBatchRemoveEdges([edgeId])
475+
},
476+
[collaborativeBatchRemoveEdges]
477+
)
478+
479+
const batchUpdateBlocksWithParent = useCallback(
480+
(updates: Array<{ id: string; position: { x: number; y: number }; parentId?: string }>) => {
481+
collaborativeBatchUpdateParent(
482+
updates.map((u) => ({
483+
blockId: u.id,
484+
newParentId: u.parentId || null,
485+
newPosition: u.position,
486+
affectedEdges: [],
487+
}))
488+
)
489+
},
490+
[collaborativeBatchUpdateParent]
491+
)
492+
467493
const addBlock = useCallback(
468494
(
469495
id: string,
@@ -570,8 +596,8 @@ const WorkflowContent = React.memo(() => {
570596
const result = updateNodeParentUtil(
571597
nodeId,
572598
newParentId,
573-
updateBlockPosition,
574-
updateParentId,
599+
collaborativeBatchUpdatePositions,
600+
batchUpdateBlocksWithParent,
575601
() => resizeLoopNodesWrapper()
576602
)
577603

@@ -594,8 +620,8 @@ const WorkflowContent = React.memo(() => {
594620
},
595621
[
596622
getNodes,
597-
updateBlockPosition,
598-
updateParentId,
623+
collaborativeBatchUpdatePositions,
624+
batchUpdateBlocksWithParent,
599625
blocks,
600626
edgesForDisplay,
601627
getNodeAbsolutePosition,
@@ -903,22 +929,15 @@ const WorkflowContent = React.memo(() => {
903929
(blockId: string, edgesToRemove: Edge[]): void => {
904930
if (edgesToRemove.length === 0) return
905931

906-
window.dispatchEvent(new CustomEvent('skip-edge-recording', { detail: { skip: true } }))
907-
908-
try {
909-
edgesToRemove.forEach((edge) => {
910-
removeEdge(edge.id)
911-
})
932+
const edgeIds = edgesToRemove.map((edge) => edge.id)
933+
collaborativeBatchRemoveEdges(edgeIds, { skipUndoRedo: true })
912934

913-
logger.debug('Removed edges for node', {
914-
blockId,
915-
edgeCount: edgesToRemove.length,
916-
})
917-
} finally {
918-
window.dispatchEvent(new CustomEvent('skip-edge-recording', { detail: { skip: false } }))
919-
}
935+
logger.debug('Removed edges for node', {
936+
blockId,
937+
edgeCount: edgesToRemove.length,
938+
})
920939
},
921-
[removeEdge]
940+
[collaborativeBatchRemoveEdges]
922941
)
923942

924943
/** Finds the closest block to a position for auto-connect. */
@@ -1942,27 +1961,37 @@ const WorkflowContent = React.memo(() => {
19421961

19431962
const movingNodeIds = new Set(validBlockIds)
19441963

1964+
// Find boundary edges (edges that cross the subflow boundary)
19451965
const boundaryEdges = edgesForDisplay.filter((e) => {
19461966
const sourceInSelection = movingNodeIds.has(e.source)
19471967
const targetInSelection = movingNodeIds.has(e.target)
19481968
return sourceInSelection !== targetInSelection
19491969
})
19501970

1951-
// Collect absolute positions BEFORE updating parents
1971+
// Collect absolute positions BEFORE any mutations
19521972
const absolutePositions = new Map<string, { x: number; y: number }>()
19531973
for (const blockId of validBlockIds) {
19541974
absolutePositions.set(blockId, getNodeAbsolutePosition(blockId))
19551975
}
19561976

1957-
for (const blockId of validBlockIds) {
1977+
// Build batch update with all blocks and their affected edges
1978+
const updates = validBlockIds.map((blockId) => {
1979+
const absolutePosition = absolutePositions.get(blockId)!
19581980
const edgesForThisNode = boundaryEdges.filter(
19591981
(e) => e.source === blockId || e.target === blockId
19601982
)
1961-
removeEdgesForNode(blockId, edgesForThisNode)
1962-
updateNodeParent(blockId, null, edgesForThisNode)
1963-
}
1983+
return {
1984+
blockId,
1985+
newParentId: null,
1986+
newPosition: absolutePosition,
1987+
affectedEdges: edgesForThisNode,
1988+
}
1989+
})
19641990

1965-
// Immediately update displayNodes to prevent React Flow from using stale parent data
1991+
// Single atomic batch update (handles edge removal + parent update + undo/redo)
1992+
collaborativeBatchUpdateParent(updates)
1993+
1994+
// Update displayNodes once to prevent React Flow from using stale parent data
19661995
setDisplayNodes((nodes) =>
19671996
nodes.map((n) => {
19681997
const absPos = absolutePositions.get(n.id)
@@ -1977,6 +2006,8 @@ const WorkflowContent = React.memo(() => {
19772006
return n
19782007
})
19792008
)
2009+
2010+
// Note: Container resize happens automatically via the derivedNodes effect
19802011
} catch (err) {
19812012
logger.error('Failed to remove from subflow', { err })
19822013
}
@@ -1985,7 +2016,7 @@ const WorkflowContent = React.memo(() => {
19852016
window.addEventListener('remove-from-subflow', handleRemoveFromSubflow as EventListener)
19862017
return () =>
19872018
window.removeEventListener('remove-from-subflow', handleRemoveFromSubflow as EventListener)
1988-
}, [blocks, edgesForDisplay, removeEdgesForNode, updateNodeParent, getNodeAbsolutePosition])
2019+
}, [blocks, edgesForDisplay, getNodeAbsolutePosition, collaborativeBatchUpdateParent])
19892020

19902021
/** Handles node position changes - updates local state for smooth drag, syncs to store only on drag end. */
19912022
const onNodesChange = useCallback((changes: NodeChange[]) => {
@@ -2072,7 +2103,12 @@ const WorkflowContent = React.memo(() => {
20722103
// Create a mapping of node IDs to check for missing parent references
20732104
const nodeIds = new Set(Object.keys(blocks))
20742105

2075-
// Check for nodes with invalid parent references
2106+
// Check for nodes with invalid parent references and collect updates
2107+
const orphanedUpdates: Array<{
2108+
id: string
2109+
position: { x: number; y: number }
2110+
parentId: string
2111+
}> = []
20762112
Object.entries(blocks).forEach(([id, block]) => {
20772113
const parentId = block.data?.parentId
20782114

@@ -2084,22 +2120,28 @@ const WorkflowContent = React.memo(() => {
20842120
})
20852121

20862122
const absolutePosition = getNodeAbsolutePosition(id)
2087-
updateBlockPosition(id, absolutePosition)
2088-
updateParentId(id, '', 'parent')
2123+
orphanedUpdates.push({ id, position: absolutePosition, parentId: '' })
20892124
}
20902125
})
2091-
}, [blocks, updateBlockPosition, updateParentId, getNodeAbsolutePosition, isWorkflowReady])
2126+
2127+
// Batch update all orphaned nodes at once
2128+
if (orphanedUpdates.length > 0) {
2129+
batchUpdateBlocksWithParent(orphanedUpdates)
2130+
}
2131+
}, [blocks, batchUpdateBlocksWithParent, getNodeAbsolutePosition, isWorkflowReady])
20922132

20932133
/** Handles edge removal changes. */
20942134
const onEdgesChange = useCallback(
20952135
(changes: any) => {
2096-
changes.forEach((change: any) => {
2097-
if (change.type === 'remove') {
2098-
removeEdge(change.id)
2099-
}
2100-
})
2136+
const edgeIdsToRemove = changes
2137+
.filter((change: any) => change.type === 'remove')
2138+
.map((change: any) => change.id)
2139+
2140+
if (edgeIdsToRemove.length > 0) {
2141+
collaborativeBatchRemoveEdges(edgeIdsToRemove)
2142+
}
21012143
},
2102-
[removeEdge]
2144+
[collaborativeBatchRemoveEdges]
21032145
)
21042146

21052147
/**
@@ -2683,9 +2725,6 @@ const WorkflowContent = React.memo(() => {
26832725

26842726
const edgesToAdd: Edge[] = autoConnectEdge ? [autoConnectEdge] : []
26852727

2686-
// Skip recording these edges separately since they're part of the parent update
2687-
window.dispatchEvent(new CustomEvent('skip-edge-recording', { detail: { skip: true } }))
2688-
26892728
// Moving to a new parent container - pass both removed and added edges for undo/redo
26902729
const affectedEdges = [...edgesToRemove, ...edgesToAdd]
26912730
updateNodeParent(node.id, potentialParentId, affectedEdges)
@@ -2704,10 +2743,10 @@ const WorkflowContent = React.memo(() => {
27042743
})
27052744
)
27062745

2707-
// Now add the edges after parent update
2708-
edgesToAdd.forEach((edge) => addEdge(edge))
2709-
2710-
window.dispatchEvent(new CustomEvent('skip-edge-recording', { detail: { skip: false } }))
2746+
// Add edges after parent update (skip undo recording - it's part of parent update)
2747+
if (edgesToAdd.length > 0) {
2748+
collaborativeBatchAddEdges(edgesToAdd, { skipUndoRedo: true })
2749+
}
27112750
} else if (!potentialParentId && dragStartParentId) {
27122751
// Moving OUT of a subflow to canvas
27132752
// Get absolute position BEFORE removing from parent
@@ -2761,7 +2800,7 @@ const WorkflowContent = React.memo(() => {
27612800
potentialParentId,
27622801
updateNodeParent,
27632802
updateBlockPosition,
2764-
addEdge,
2803+
collaborativeBatchAddEdges,
27652804
tryCreateAutoConnectEdge,
27662805
blocks,
27672806
edgesForDisplay,

apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/settings-modal/components/access-control/access-control.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,8 @@ import { Input as BaseInput, Skeleton } from '@/components/ui'
2525
import { useSession } from '@/lib/auth/auth-client'
2626
import { getSubscriptionStatus } from '@/lib/billing/client'
2727
import type { PermissionGroupConfig } from '@/lib/permission-groups/types'
28+
import { getUserColor } from '@/lib/workspaces/colors'
2829
import { getUserRole } from '@/lib/workspaces/organization'
29-
import { getUserColor } from '@/app/workspace/[workspaceId]/w/utils/get-user-color'
3030
import { getAllBlocks } from '@/blocks'
3131
import { useOrganization, useOrganizations } from '@/hooks/queries/organization'
3232
import {

0 commit comments

Comments
 (0)