Skip to content

Commit 605f4e8

Browse files
committed
improvement(editor): fix React effect/state anti-patterns across the /w surface
Removes derived-state-in-effect, prop-sync effects, and event-logic-in-effect across the workflow editor in favor of render-phase adjustment, derived render values, and event-handler side effects. Stabilizes unstable references (memoization, module-scope hoists, state->ref for non-rendered values), fixes several stale-closure dependency bugs, and adds accessibility labels/roles. No behavior changes; typecheck and lint clean.
1 parent b8e88b1 commit 605f4e8

92 files changed

Lines changed: 2232 additions & 1840 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

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

Lines changed: 55 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -269,10 +269,10 @@ export function Chat() {
269269
const { addToQueue } = useOperationQueue()
270270

271271
const [chatMessage, setChatMessage] = useState('')
272-
const [promptHistory, setPromptHistory] = useState<string[]>([])
273-
const [historyIndex, setHistoryIndex] = useState(-1)
274272
const [moreMenuOpen, setMoreMenuOpen] = useState(false)
275273

274+
const promptHistoryRef = useRef<string[]>([])
275+
const historyIndexRef = useRef(-1)
276276
const inputRef = useRef<HTMLInputElement>(null)
277277
const timeoutRef = useRef<NodeJS.Timeout | null>(null)
278278
const streamReaderRef = useRef<ReadableStreamDefaultReader<Uint8Array> | null>(null)
@@ -292,22 +292,26 @@ export function Chat() {
292292
handleDrop,
293293
} = useChatFileUpload()
294294

295-
const filePreviewUrls = useRef<Map<string, string>>(new Map())
295+
const filePreviewUrls = useRef<Map<string, string> | null>(null)
296+
if (filePreviewUrls.current === null) {
297+
filePreviewUrls.current = new Map()
298+
}
296299

297300
const getFilePreviewUrl = useCallback((file: ChatFile): string | null => {
298301
if (!file.type.startsWith('image/')) return null
299302

300-
const existing = filePreviewUrls.current.get(file.id)
303+
const urls = (filePreviewUrls.current ??= new Map())
304+
const existing = urls.get(file.id)
301305
if (existing) return existing
302306

303307
const url = URL.createObjectURL(file.file)
304-
filePreviewUrls.current.set(file.id, url)
308+
urls.set(file.id, url)
305309
return url
306310
}, [])
307311

308312
useEffect(() => {
309313
const currentFileIds = new Set(chatFiles.map((f) => f.id))
310-
const urlMap = filePreviewUrls.current
314+
const urlMap = (filePreviewUrls.current ??= new Map())
311315

312316
for (const [fileId, url] of urlMap.entries()) {
313317
if (!currentFileIds.has(fileId)) {
@@ -454,21 +458,24 @@ export function Chat() {
454458
)
455459

456460
const userMessages = useMemo(() => {
457-
return workflowMessages
458-
.filter((msg) => msg.type === 'user')
459-
.map((msg) => msg.content)
460-
.filter((content): content is string => typeof content === 'string')
461+
const result: string[] = []
462+
for (const msg of workflowMessages) {
463+
if (msg.type === 'user' && typeof msg.content === 'string') {
464+
result.push(msg.content)
465+
}
466+
}
467+
return result
461468
}, [workflowMessages])
462469

463470
useEffect(() => {
464471
if (!activeWorkflowId) {
465-
setPromptHistory([])
466-
setHistoryIndex(-1)
472+
promptHistoryRef.current = []
473+
historyIndexRef.current = -1
467474
return
468475
}
469476

470-
setPromptHistory(userMessages)
471-
setHistoryIndex(-1)
477+
promptHistoryRef.current = userMessages
478+
historyIndexRef.current = -1
472479
}, [activeWorkflowId, userMessages])
473480

474481
/**
@@ -607,7 +614,7 @@ export function Chat() {
607614
focusInput(100)
608615
}
609616
},
610-
[appendMessageContent, finalizeMessageStream, focusInput, selectedOutputs, activeWorkflowId]
617+
[appendMessageContent, finalizeMessageStream, focusInput]
611618
)
612619

613620
/**
@@ -633,19 +640,18 @@ export function Chat() {
633640
}
634641

635642
if ('success' in result && result.success && 'logs' in result && Array.isArray(result.logs)) {
636-
selectedOutputs
637-
.map((outputId) => extractOutputFromLogs(result.logs as BlockLog[], outputId))
638-
.filter((output) => output !== undefined)
639-
.forEach((output) => {
640-
const content = formatOutputContent(output)
641-
if (content) {
642-
addMessage({
643-
content,
644-
workflowId: activeWorkflowId,
645-
type: 'workflow',
646-
})
647-
}
648-
})
643+
for (const outputId of selectedOutputs) {
644+
const output = extractOutputFromLogs(result.logs as BlockLog[], outputId)
645+
if (output === undefined) continue
646+
const content = formatOutputContent(output)
647+
if (content) {
648+
addMessage({
649+
content,
650+
workflowId: activeWorkflowId,
651+
type: 'workflow',
652+
})
653+
}
654+
}
649655
return
650656
}
651657

@@ -674,10 +680,13 @@ export function Chat() {
674680

675681
const sentMessage = chatMessage.trim()
676682

677-
if (sentMessage && promptHistory[promptHistory.length - 1] !== sentMessage) {
678-
setPromptHistory((prev) => [...prev, sentMessage])
683+
if (
684+
sentMessage &&
685+
promptHistoryRef.current[promptHistoryRef.current.length - 1] !== sentMessage
686+
) {
687+
promptHistoryRef.current = [...promptHistoryRef.current, sentMessage]
679688
}
680-
setHistoryIndex(-1)
689+
historyIndexRef.current = -1
681690

682691
const conversationId = getConversationId(activeWorkflowId)
683692

@@ -732,7 +741,6 @@ export function Chat() {
732741
chatFiles,
733742
activeWorkflowId,
734743
isExecuting,
735-
promptHistory,
736744
getConversationId,
737745
addMessage,
738746
handleRunWorkflow,
@@ -756,27 +764,29 @@ export function Chat() {
756764
}
757765
} else if (e.key === 'ArrowUp') {
758766
e.preventDefault()
759-
if (promptHistory.length > 0) {
767+
if (promptHistoryRef.current.length > 0) {
760768
const newIndex =
761-
historyIndex === -1 ? promptHistory.length - 1 : Math.max(0, historyIndex - 1)
762-
setHistoryIndex(newIndex)
763-
setChatMessage(promptHistory[newIndex])
769+
historyIndexRef.current === -1
770+
? promptHistoryRef.current.length - 1
771+
: Math.max(0, historyIndexRef.current - 1)
772+
historyIndexRef.current = newIndex
773+
setChatMessage(promptHistoryRef.current[newIndex])
764774
}
765775
} else if (e.key === 'ArrowDown') {
766776
e.preventDefault()
767-
if (historyIndex >= 0) {
768-
const newIndex = historyIndex + 1
769-
if (newIndex >= promptHistory.length) {
770-
setHistoryIndex(-1)
777+
if (historyIndexRef.current >= 0) {
778+
const newIndex = historyIndexRef.current + 1
779+
if (newIndex >= promptHistoryRef.current.length) {
780+
historyIndexRef.current = -1
771781
setChatMessage('')
772782
} else {
773-
setHistoryIndex(newIndex)
774-
setChatMessage(promptHistory[newIndex])
783+
historyIndexRef.current = newIndex
784+
setChatMessage(promptHistoryRef.current[newIndex])
775785
}
776786
}
777787
}
778788
},
779-
[handleSendMessage, promptHistory, historyIndex, isStreaming, isExecuting]
789+
[handleSendMessage, isStreaming, isExecuting]
780790
)
781791

782792
/**
@@ -1085,7 +1095,7 @@ export function Chat() {
10851095
value={chatMessage}
10861096
onChange={(e) => {
10871097
setChatMessage(e.target.value)
1088-
setHistoryIndex(-1)
1098+
historyIndexRef.current = -1
10891099
}}
10901100
onKeyDown={handleKeyPress}
10911101
placeholder={isDragOver ? 'Drop files here...' : 'Type a message...'}
@@ -1122,6 +1132,7 @@ export function Chat() {
11221132
) : (
11231133
<Button
11241134
onClick={handleSendMessage}
1135+
aria-label='Send message'
11251136
variant='ghost'
11261137
disabled={
11271138
(!chatMessage.trim() && chatFiles.length === 0) ||

apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/chat/components/output-select/output-select.tsx

Lines changed: 11 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,16 @@ const TagIcon: React.FC<{
4444

4545
const EMPTY_OUTPUTS: string[] = []
4646

47+
/**
48+
* Gets the background color for a block output based on its type
49+
* @param blockType - The type of the block
50+
* @returns The hex color code for the block
51+
*/
52+
const getOutputColor = (blockType: string) => {
53+
const blockConfig = getBlock(blockType)
54+
return blockConfig?.bgColor || '#2F55FF'
55+
}
56+
4757
/**
4858
* Props for the OutputSelect component
4959
*/
@@ -159,16 +169,7 @@ export function OutputSelect({
159169
path: f.path,
160170
}
161171
})
162-
}, [
163-
workflowBlocks,
164-
workflowId,
165-
isShowingDiff,
166-
isDiffReady,
167-
baselineWorkflow,
168-
blocks,
169-
subBlockValues,
170-
shouldUseBaseline,
171-
])
172+
}, [workflowBlocks, workflowId, baselineWorkflow, subBlockValues, shouldUseBaseline])
172173

173174
/**
174175
* Gets display text for selected outputs
@@ -193,16 +194,6 @@ export function OutputSelect({
193194
return `${validOutputs.length} outputs`
194195
}, [selectedOutputs, workflowOutputs, placeholder])
195196

196-
/**
197-
* Gets the background color for a block output based on its type
198-
* @param blockType - The type of the block
199-
* @returns The hex color code for the block
200-
*/
201-
const getOutputColor = (blockType: string) => {
202-
const blockConfig = getBlock(blockType)
203-
return blockConfig?.bgColor || '#2F55FF'
204-
}
205-
206197
/**
207198
* Groups outputs by block and sorts by distance from starter block.
208199
* Returns ComboboxOptionGroup[] for use with Combobox.

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

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -32,15 +32,17 @@ const CursorsComponent = () => {
3232
return []
3333
}
3434

35-
return presenceUsers
36-
.filter((user): user is typeof user & { cursor: CursorPoint } => Boolean(user.cursor))
37-
.filter((user) => user.socketId !== currentSocketId)
38-
.map((user) => ({
35+
const rendered: CursorRenderData[] = []
36+
for (const user of presenceUsers) {
37+
if (!user.cursor || user.socketId === currentSocketId) continue
38+
rendered.push({
3939
id: user.socketId,
4040
name: user.userName?.trim() || 'Collaborator',
4141
cursor: user.cursor,
4242
color: getUserColor(user.userId),
43-
}))
43+
})
44+
}
45+
return rendered
4446
}, [activeWorkflowId, currentSocketId, currentWorkflowId, presenceUsers])
4547

4648
if (!cursors.length) {

apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/diff-controls/diff-controls.tsx

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ export const DiffControls = memo(function DiffControls() {
6363
>
6464
{/* Reject side */}
6565
<button
66+
type='button'
6667
onClick={handleReject}
6768
title='Reject changes'
6869
className='relative flex h-full items-center border border-[var(--border)] bg-[var(--surface-4)] pr-5 pl-3 font-medium text-[var(--text-secondary)] text-small transition-colors hover-hover:border-[var(--border-1)] hover-hover:bg-[var(--surface-6)] hover-hover:text-[var(--text-primary)] dark:hover-hover:bg-[var(--surface-5)]'
@@ -86,6 +87,7 @@ export const DiffControls = memo(function DiffControls() {
8687
/>
8788
{/* Accept side */}
8889
<button
90+
type='button'
8991
onClick={handleAccept}
9092
title='Accept changes (⇧⌘⏎)'
9193
className='-ml-2.5 relative flex h-full items-center border border-[rgba(0,0,0,0.15)] bg-[var(--brand-accent)] pr-3 pl-5 font-medium text-[var(--text-inverse)] text-small transition-[background-color,border-color,fill,stroke] hover-hover:brightness-110 dark:border-[rgba(255,255,255,0.1)]'

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,11 @@ export const NoteBlock = memo(function NoteBlock({
6464
const userPermissions = useUserPermissionsContext()
6565
const canEditWorkflow = userPermissions.canEdit && !data.isWorkflowLocked
6666

67+
const actionBar = useMemo(
68+
() => <ActionBar blockId={id} blockType={type} disabled={!canEditWorkflow} />,
69+
[id, type, canEditWorkflow]
70+
)
71+
6772
/**
6873
* Calculate deterministic dimensions based on content structure. Uses fixed
6974
* width and computed height to avoid ResizeObserver jitter.
@@ -90,7 +95,7 @@ export const NoteBlock = memo(function NoteBlock({
9095
hasRing={hasRing}
9196
ringStyles={ringStyles}
9297
onSelect={handleClick}
93-
actionBar={<ActionBar blockId={id} blockType={type} disabled={!canEditWorkflow} />}
98+
actionBar={actionBar}
9499
/>
95100
)
96101
})

apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/copilot/components/user-input/hooks/use-context-management.ts

Lines changed: 33 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { useCallback, useEffect, useState } from 'react'
1+
import { useCallback, useRef, useState } from 'react'
22
import {
33
escapeRegex,
44
filterOutContext,
@@ -56,40 +56,45 @@ export function useContextManagement({ message, initialContexts }: UseContextMan
5656
/**
5757
* Synchronizes selected contexts with inline @label or /label tokens in the message.
5858
* Removes contexts whose labels are no longer present in the message.
59+
*
60+
* Adjusted during render with the prev-ref idiom rather than in an effect so the
61+
* stale, pre-filtered list is never committed. The ref starts as `undefined` so the
62+
* very first render runs the same sync the mounting effect used to.
5963
*/
60-
useEffect(() => {
64+
const prevMessageRef = useRef<string | undefined>(undefined)
65+
if (prevMessageRef.current !== message) {
66+
prevMessageRef.current = message
6167
if (!message) {
6268
// Functional updater bails out when already empty; a fresh `[]` literal would
6369
// emit a new reference and invalidate downstream memos that key on identity.
6470
setSelectedContexts((prev) => (prev.length === 0 ? prev : []))
65-
return
66-
}
67-
68-
setSelectedContexts((prev) => {
69-
if (prev.length === 0) return prev
71+
} else {
72+
setSelectedContexts((prev) => {
73+
if (prev.length === 0) return prev
7074

71-
const filtered = prev.filter((c) => {
72-
if (!c.label) return false
73-
// Check for slash command tokens or mention tokens based on kind.
74-
// The trailing lookahead `(?![A-Za-z0-9_])` accepts any word-boundary
75-
// — whitespace, end-of-string, or punctuation — so `@Slack.` and
76-
// `@Slack,` survive the sync. A strict `(\s|$)` here would strip
77-
// contexts whenever the user ends a sentence with a mention.
78-
// Skills store a wide EM SPACE sentinel (SKILL_CHIP_TRIGGER) as their
79-
// trigger so the chip icon fits; slash commands keep '/'; everything
80-
// else uses '@'. The sentinel is itself a whitespace character, but the
81-
// `(^|\s)` boundary still matches the (regular) space or start that
82-
// precedes it, then the literal sentinel.
83-
const prefix =
84-
c.kind === 'skill' ? SKILL_CHIP_TRIGGER : c.kind === 'slash_command' ? '/' : '@'
85-
const tokenPattern = new RegExp(
86-
`(^|\\s)${escapeRegex(prefix)}${escapeRegex(c.label)}(?![A-Za-z0-9_])`
87-
)
88-
return tokenPattern.test(message)
75+
const filtered = prev.filter((c) => {
76+
if (!c.label) return false
77+
// Check for slash command tokens or mention tokens based on kind.
78+
// The trailing lookahead `(?![A-Za-z0-9_])` accepts any word-boundary
79+
// — whitespace, end-of-string, or punctuation — so `@Slack.` and
80+
// `@Slack,` survive the sync. A strict `(\s|$)` here would strip
81+
// contexts whenever the user ends a sentence with a mention.
82+
// Skills store a wide EM SPACE sentinel (SKILL_CHIP_TRIGGER) as their
83+
// trigger so the chip icon fits; slash commands keep '/'; everything
84+
// else uses '@'. The sentinel is itself a whitespace character, but the
85+
// `(^|\s)` boundary still matches the (regular) space or start that
86+
// precedes it, then the literal sentinel.
87+
const prefix =
88+
c.kind === 'skill' ? SKILL_CHIP_TRIGGER : c.kind === 'slash_command' ? '/' : '@'
89+
const tokenPattern = new RegExp(
90+
`(^|\\s)${escapeRegex(prefix)}${escapeRegex(c.label)}(?![A-Za-z0-9_])`
91+
)
92+
return tokenPattern.test(message)
93+
})
94+
return filtered.length === prev.length ? prev : filtered
8995
})
90-
return filtered.length === prev.length ? prev : filtered
91-
})
92-
}, [message])
96+
}
97+
}
9398

9499
return {
95100
selectedContexts,

0 commit comments

Comments
 (0)