Skip to content

Commit 98493de

Browse files
committed
fix subflow issues
1 parent b085942 commit 98493de

File tree

3 files changed

+161
-76
lines changed

3 files changed

+161
-76
lines changed

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

Lines changed: 108 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -921,45 +921,6 @@ const WorkflowContent = React.memo(() => {
921921
[removeEdge]
922922
)
923923

924-
/** Handles ActionBar remove-from-subflow events. */
925-
useEffect(() => {
926-
const handleRemoveFromSubflow = (event: Event) => {
927-
const customEvent = event as CustomEvent<{ blockIds: string[] }>
928-
const blockIds = customEvent.detail?.blockIds
929-
if (!blockIds || blockIds.length === 0) return
930-
931-
try {
932-
const validBlockIds = blockIds.filter((id) => {
933-
const block = blocks[id]
934-
return block?.data?.parentId
935-
})
936-
if (validBlockIds.length === 0) return
937-
938-
const movingNodeIds = new Set(validBlockIds)
939-
940-
const boundaryEdges = edgesForDisplay.filter((e) => {
941-
const sourceInSelection = movingNodeIds.has(e.source)
942-
const targetInSelection = movingNodeIds.has(e.target)
943-
return sourceInSelection !== targetInSelection
944-
})
945-
946-
for (const blockId of validBlockIds) {
947-
const edgesForThisNode = boundaryEdges.filter(
948-
(e) => e.source === blockId || e.target === blockId
949-
)
950-
removeEdgesForNode(blockId, edgesForThisNode)
951-
updateNodeParent(blockId, null, edgesForThisNode)
952-
}
953-
} catch (err) {
954-
logger.error('Failed to remove from subflow', { err })
955-
}
956-
}
957-
958-
window.addEventListener('remove-from-subflow', handleRemoveFromSubflow as EventListener)
959-
return () =>
960-
window.removeEventListener('remove-from-subflow', handleRemoveFromSubflow as EventListener)
961-
}, [blocks, edgesForDisplay, removeEdgesForNode, updateNodeParent])
962-
963924
/** Finds the closest block to a position for auto-connect. */
964925
const findClosestOutput = useCallback(
965926
(newNodePosition: { x: number; y: number }): BlockData | null => {
@@ -1827,6 +1788,18 @@ const WorkflowContent = React.memo(() => {
18271788
return
18281789
}
18291790

1791+
// Recovery: detect and clear invalid parent references to prevent infinite recursion
1792+
if (block.data?.parentId) {
1793+
if (block.data.parentId === block.id) {
1794+
block.data = { ...block.data, parentId: undefined, extent: undefined }
1795+
} else {
1796+
const parentBlock = blocks[block.data.parentId]
1797+
if (parentBlock?.data?.parentId === block.id) {
1798+
block.data = { ...block.data, parentId: undefined, extent: undefined }
1799+
}
1800+
}
1801+
}
1802+
18301803
// Handle container nodes differently
18311804
if (block.type === 'loop' || block.type === 'parallel') {
18321805
nodeArray.push({
@@ -1965,6 +1938,67 @@ const WorkflowContent = React.memo(() => {
19651938
})
19661939
}, [derivedNodes])
19671940

1941+
/** Handles ActionBar remove-from-subflow events. */
1942+
useEffect(() => {
1943+
const handleRemoveFromSubflow = (event: Event) => {
1944+
const customEvent = event as CustomEvent<{ blockIds: string[] }>
1945+
const blockIds = customEvent.detail?.blockIds
1946+
if (!blockIds || blockIds.length === 0) return
1947+
1948+
try {
1949+
const validBlockIds = blockIds.filter((id) => {
1950+
const block = blocks[id]
1951+
return block?.data?.parentId
1952+
})
1953+
if (validBlockIds.length === 0) return
1954+
1955+
const movingNodeIds = new Set(validBlockIds)
1956+
1957+
const boundaryEdges = edgesForDisplay.filter((e) => {
1958+
const sourceInSelection = movingNodeIds.has(e.source)
1959+
const targetInSelection = movingNodeIds.has(e.target)
1960+
return sourceInSelection !== targetInSelection
1961+
})
1962+
1963+
// Collect absolute positions BEFORE updating parents
1964+
const absolutePositions = new Map<string, { x: number; y: number }>()
1965+
for (const blockId of validBlockIds) {
1966+
absolutePositions.set(blockId, getNodeAbsolutePosition(blockId))
1967+
}
1968+
1969+
for (const blockId of validBlockIds) {
1970+
const edgesForThisNode = boundaryEdges.filter(
1971+
(e) => e.source === blockId || e.target === blockId
1972+
)
1973+
removeEdgesForNode(blockId, edgesForThisNode)
1974+
updateNodeParent(blockId, null, edgesForThisNode)
1975+
}
1976+
1977+
// Immediately update displayNodes to prevent React Flow from using stale parent data
1978+
setDisplayNodes((nodes) =>
1979+
nodes.map((n) => {
1980+
const absPos = absolutePositions.get(n.id)
1981+
if (absPos) {
1982+
return {
1983+
...n,
1984+
position: absPos,
1985+
parentId: undefined,
1986+
extent: undefined,
1987+
}
1988+
}
1989+
return n
1990+
})
1991+
)
1992+
} catch (err) {
1993+
logger.error('Failed to remove from subflow', { err })
1994+
}
1995+
}
1996+
1997+
window.addEventListener('remove-from-subflow', handleRemoveFromSubflow as EventListener)
1998+
return () =>
1999+
window.removeEventListener('remove-from-subflow', handleRemoveFromSubflow as EventListener)
2000+
}, [blocks, edgesForDisplay, removeEdgesForNode, updateNodeParent, getNodeAbsolutePosition])
2001+
19682002
/** Handles node position changes - updates local state for smooth drag, syncs to store only on drag end. */
19692003
const onNodesChange = useCallback((changes: NodeChange[]) => {
19702004
setDisplayNodes((nds) => applyNodeChanges(changes, nds))
@@ -2409,6 +2443,8 @@ const WorkflowContent = React.memo(() => {
24092443
// Store the original parent ID when starting to drag
24102444
const currentParentId = blocks[node.id]?.data?.parentId || null
24112445
setDragStartParentId(currentParentId)
2446+
// Initialize potentialParentId to the current parent so a click without movement doesn't remove from subflow
2447+
setPotentialParentId(currentParentId)
24122448
// Store starting position for undo/redo move entry
24132449
setDragStartPosition({
24142450
id: node.id,
@@ -2432,7 +2468,7 @@ const WorkflowContent = React.memo(() => {
24322468
}
24332469
})
24342470
},
2435-
[blocks, setDragStartPosition, getNodes]
2471+
[blocks, setDragStartPosition, getNodes, potentialParentId, setPotentialParentId]
24362472
)
24372473

24382474
/** Handles node drag stop to establish parent-child relationships. */
@@ -2666,12 +2702,29 @@ const WorkflowContent = React.memo(() => {
26662702
const affectedEdges = [...edgesToRemove, ...edgesToAdd]
26672703
updateNodeParent(node.id, potentialParentId, affectedEdges)
26682704

2705+
setDisplayNodes((nodes) =>
2706+
nodes.map((n) => {
2707+
if (n.id === node.id) {
2708+
return {
2709+
...n,
2710+
position: relativePositionBefore,
2711+
parentId: potentialParentId,
2712+
extent: 'parent' as const,
2713+
}
2714+
}
2715+
return n
2716+
})
2717+
)
2718+
26692719
// Now add the edges after parent update
26702720
edgesToAdd.forEach((edge) => addEdge(edge))
26712721

26722722
window.dispatchEvent(new CustomEvent('skip-edge-recording', { detail: { skip: false } }))
26732723
} else if (!potentialParentId && dragStartParentId) {
26742724
// Moving OUT of a subflow to canvas
2725+
// Get absolute position BEFORE removing from parent
2726+
const absolutePosition = getNodeAbsolutePosition(node.id)
2727+
26752728
// Remove edges connected to this node since it's leaving its parent
26762729
const edgesToRemove = edgesForDisplay.filter(
26772730
(e) => e.source === node.id || e.target === node.id
@@ -2690,6 +2743,21 @@ const WorkflowContent = React.memo(() => {
26902743
// Clear the parent relationship
26912744
updateNodeParent(node.id, null, edgesToRemove)
26922745

2746+
// Immediately update displayNodes to prevent React Flow from using stale parent data
2747+
setDisplayNodes((nodes) =>
2748+
nodes.map((n) => {
2749+
if (n.id === node.id) {
2750+
return {
2751+
...n,
2752+
position: absolutePosition,
2753+
parentId: undefined,
2754+
extent: undefined,
2755+
}
2756+
}
2757+
return n
2758+
})
2759+
)
2760+
26932761
logger.info('Moved node out of subflow', {
26942762
blockId: node.id,
26952763
sourceParentId: dragStartParentId,

apps/sim/stores/workflows/utils.ts

Lines changed: 48 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -428,14 +428,8 @@ function updateBlockReferences(
428428
clearTriggerRuntimeValues = false
429429
): void {
430430
Object.entries(blocks).forEach(([_, block]) => {
431-
if (block.data?.parentId) {
432-
const newParentId = idMap.get(block.data.parentId)
433-
if (newParentId) {
434-
block.data = { ...block.data, parentId: newParentId }
435-
} else {
436-
block.data = { ...block.data, parentId: undefined, extent: undefined }
437-
}
438-
}
431+
// NOTE: parentId remapping is handled in regenerateBlockIds' second pass.
432+
// Do NOT remap parentId here as it would incorrectly clear already-mapped IDs.
439433

440434
if (block.subBlocks) {
441435
Object.entries(block.subBlocks).forEach(([subBlockId, subBlock]) => {
@@ -465,6 +459,7 @@ export function regenerateWorkflowIds(
465459
const nameMap = new Map<string, string>()
466460
const newBlocks: Record<string, BlockState> = {}
467461

462+
// First pass: generate new IDs
468463
Object.entries(workflowState.blocks).forEach(([oldId, block]) => {
469464
const newId = uuidv4()
470465
blockIdMap.set(oldId, newId)
@@ -473,6 +468,19 @@ export function regenerateWorkflowIds(
473468
newBlocks[newId] = { ...block, id: newId }
474469
})
475470

471+
// Second pass: update parentId references
472+
Object.values(newBlocks).forEach((block) => {
473+
if (block.data?.parentId) {
474+
const newParentId = blockIdMap.get(block.data.parentId)
475+
if (newParentId) {
476+
block.data = { ...block.data, parentId: newParentId }
477+
} else {
478+
// Parent not in the workflow, clear the relationship
479+
block.data = { ...block.data, parentId: undefined, extent: undefined }
480+
}
481+
}
482+
})
483+
476484
const newEdges = workflowState.edges.map((edge) => ({
477485
...edge,
478486
id: uuidv4(),
@@ -535,6 +543,7 @@ export function regenerateBlockIds(
535543
// Track all blocks for name uniqueness (existing + newly processed)
536544
const allBlocksForNaming = { ...existingBlockNames }
537545

546+
// First pass: generate new IDs and names for all blocks
538547
Object.entries(blocks).forEach(([oldId, block]) => {
539548
const newId = uuidv4()
540549
blockIdMap.set(oldId, newId)
@@ -544,16 +553,22 @@ export function regenerateBlockIds(
544553
const newNormalizedName = normalizeName(newName)
545554
nameMap.set(oldNormalizedName, newNormalizedName)
546555

547-
// Always apply position offset and clear parentId since we paste to canvas level
556+
// Check if this block has a parent that's also being copied
557+
// If so, it's a nested block and should keep its relative position (no offset)
558+
// Only top-level blocks (no parent in the paste set) get the position offset
559+
const hasParentInPasteSet = block.data?.parentId && blocks[block.data.parentId]
560+
const newPosition = hasParentInPasteSet
561+
? { x: block.position.x, y: block.position.y } // Keep relative position
562+
: { x: block.position.x + positionOffset.x, y: block.position.y + positionOffset.y }
563+
564+
// Placeholder block - we'll update parentId in second pass
548565
const newBlock: BlockState = {
549566
...block,
550567
id: newId,
551568
name: newName,
552-
position: {
553-
x: block.position.x + positionOffset.x,
554-
y: block.position.y + positionOffset.y,
555-
},
556-
data: block.data ? { ...block.data, parentId: undefined } : block.data,
569+
position: newPosition,
570+
// Temporarily keep data as-is, we'll fix parentId in second pass
571+
data: block.data ? { ...block.data } : block.data,
557572
}
558573

559574
newBlocks[newId] = newBlock
@@ -565,6 +580,25 @@ export function regenerateBlockIds(
565580
}
566581
})
567582

583+
// Second pass: update parentId references for nested blocks
584+
// If a block's parent is also being pasted, map to new parentId; otherwise clear it
585+
Object.entries(newBlocks).forEach(([, block]) => {
586+
if (block.data?.parentId) {
587+
const oldParentId = block.data.parentId
588+
const newParentId = blockIdMap.get(oldParentId)
589+
590+
if (newParentId) {
591+
block.data = {
592+
...block.data,
593+
parentId: newParentId,
594+
extent: 'parent',
595+
}
596+
} else {
597+
block.data = { ...block.data, parentId: undefined, extent: undefined }
598+
}
599+
}
600+
})
601+
568602
const newEdges = edges.map((edge) => ({
569603
...edge,
570604
id: uuidv4(),

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

Lines changed: 5 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,7 @@ export const useWorkflowStore = create<WorkflowStore>()(
174174
...data,
175175
...(parentId && { parentId, extent: extent || 'parent' }),
176176
}
177+
// #endregion
177178

178179
const subBlocks: Record<string, SubBlockState> = {}
179180
const subBlockStore = useSubBlockStore.getState()
@@ -295,26 +296,16 @@ export const useWorkflowStore = create<WorkflowStore>()(
295296
return
296297
}
297298

298-
logger.info('UpdateParentId called:', {
299-
blockId: id,
300-
blockName: block.name,
301-
blockType: block.type,
302-
newParentId: parentId,
303-
extent,
304-
currentParentId: block.data?.parentId,
305-
})
299+
if (parentId === id) {
300+
logger.error('Blocked attempt to set block as its own parent', { blockId: id })
301+
return
302+
}
306303

307-
// Skip if the parent ID hasn't changed
308304
if (block.data?.parentId === parentId) {
309-
logger.info('Parent ID unchanged, skipping update')
310305
return
311306
}
312307

313-
// Store current absolute position
314308
const absolutePosition = { ...block.position }
315-
316-
// Handle empty or null parentId (removing from parent)
317-
// On removal, clear the data JSON entirely per normalized DB contract
318309
const newData = !parentId
319310
? {}
320311
: {
@@ -323,8 +314,6 @@ export const useWorkflowStore = create<WorkflowStore>()(
323314
extent,
324315
}
325316

326-
// For removal we already set data to {}; for setting a parent keep as-is
327-
328317
const newState = {
329318
blocks: {
330319
...get().blocks,
@@ -339,12 +328,6 @@ export const useWorkflowStore = create<WorkflowStore>()(
339328
parallels: { ...get().parallels },
340329
}
341330

342-
logger.info('[WorkflowStore/updateParentId] Updated parentId relationship:', {
343-
blockId: id,
344-
newParentId: parentId || 'None (removed parent)',
345-
keepingPosition: absolutePosition,
346-
})
347-
348331
set(newState)
349332
get().updateLastSaved()
350333
// Note: Socket.IO handles real-time sync automatically

0 commit comments

Comments
 (0)