Skip to content

Commit 230143a

Browse files
improvement(variables): variable experience and removed execution time (#349)
* improvement(variables): removed string type and kept plaintext without processing input * improvement(function): removed execution time * fix(variables): inputting variables and validating format * fix(variables): sync * fix(tests): upgraded tests * Apply suggestions from code review Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com> --------- Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
1 parent c27698f commit 230143a

File tree

13 files changed

+476
-143
lines changed

13 files changed

+476
-143
lines changed

apps/sim/app/api/workflows/[id]/variables/route.ts

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,11 +63,23 @@ export async function POST(req: NextRequest, { params }: { params: Promise<{ id:
6363
variablesRecord[variable.id] = variable
6464
})
6565

66+
// Get existing variables to merge with the incoming ones
67+
const existingVariables = (workflowRecord[0].variables as Record<string, Variable>) || {}
68+
69+
// Create a timestamp based on the current request
70+
71+
// Merge variables: Keep existing ones and update/add new ones
72+
// This prevents variables from being deleted during race conditions
73+
const mergedVariables = {
74+
...existingVariables,
75+
...variablesRecord
76+
}
77+
6678
// Update workflow with variables
6779
await db
6880
.update(workflow)
6981
.set({
70-
variables: variablesRecord,
82+
variables: mergedVariables,
7183
updatedAt: new Date(),
7284
})
7385
.where(eq(workflow.id, workflowId))

apps/sim/app/w/[id]/components/panel/components/variables/variables.tsx

Lines changed: 142 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,16 @@
11
'use client'
22

3-
import { useEffect, useRef } from 'react'
4-
import { ChevronDown, Copy, MoreVertical, Plus, Trash } from 'lucide-react'
3+
import { useEffect, useRef, useState } from 'react'
4+
import {
5+
AlertCircle,
6+
AlertTriangle,
7+
Check,
8+
ChevronDown,
9+
Copy,
10+
MoreVertical,
11+
Plus,
12+
Trash,
13+
} from 'lucide-react'
514
import { highlight, languages } from 'prismjs'
615
import 'prismjs/components/prism-javascript'
716
import 'prismjs/themes/prism.css'
@@ -51,6 +60,9 @@ export function Variables({ panelWidth }: VariablesProps) {
5160
// Track editor references
5261
const editorRefs = useRef<Record<string, HTMLDivElement | null>>({})
5362

63+
// Track which variables are currently being edited
64+
const [activeEditors, setActiveEditors] = useState<Record<string, boolean>>({})
65+
5466
// Auto-save when variables are added/edited
5567
const handleAddVariable = () => {
5668
if (!activeWorkflowId) return
@@ -68,8 +80,8 @@ export function Variables({ panelWidth }: VariablesProps) {
6880

6981
const getTypeIcon = (type: VariableType) => {
7082
switch (type) {
71-
case 'string':
72-
return 'Aa'
83+
case 'plain':
84+
return 'Abc'
7385
case 'number':
7486
return '123'
7587
case 'boolean':
@@ -78,7 +90,7 @@ export function Variables({ panelWidth }: VariablesProps) {
7890
return '{}'
7991
case 'array':
8092
return '[]'
81-
case 'plain':
93+
case 'string':
8294
return 'Abc'
8395
default:
8496
return '?'
@@ -87,8 +99,8 @@ export function Variables({ panelWidth }: VariablesProps) {
8799

88100
const getPlaceholder = (type: VariableType) => {
89101
switch (type) {
90-
case 'string':
91-
return '"Hello world"'
102+
case 'plain':
103+
return 'Plain text value'
92104
case 'number':
93105
return '42'
94106
case 'boolean':
@@ -97,7 +109,7 @@ export function Variables({ panelWidth }: VariablesProps) {
97109
return '{\n "key": "value"\n}'
98110
case 'array':
99111
return '[\n 1,\n 2,\n 3\n]'
100-
case 'plain':
112+
case 'string':
101113
return 'Plain text value'
102114
default:
103115
return ''
@@ -117,16 +129,100 @@ export function Variables({ panelWidth }: VariablesProps) {
117129
}
118130
}
119131

132+
// Handle editor value changes - store exactly what user types
133+
const handleEditorChange = (variable: Variable, newValue: string) => {
134+
// Store the raw value directly, no parsing or formatting
135+
updateVariable(variable.id, {
136+
value: newValue,
137+
// Clear any previous validation errors so they'll be recalculated
138+
validationError: undefined,
139+
})
140+
}
141+
142+
// Only track focus state for UI purposes
143+
const handleEditorBlur = (variableId: string) => {
144+
setActiveEditors((prev) => ({
145+
...prev,
146+
[variableId]: false,
147+
}))
148+
}
149+
150+
// Track when editor becomes active
151+
const handleEditorFocus = (variableId: string) => {
152+
setActiveEditors((prev) => ({
153+
...prev,
154+
[variableId]: true,
155+
}))
156+
}
157+
158+
// Always return raw value without any formatting
120159
const formatValue = (variable: Variable) => {
121160
if (variable.value === '') return ''
122161

123-
try {
124-
// Use the VariableManager to format values consistently
125-
return VariableManager.formatForEditor(variable.value, variable.type)
126-
} catch (e) {
127-
console.error('Error formatting value:', e)
128-
// If formatting fails, return as is
129-
return typeof variable.value === 'string' ? variable.value : JSON.stringify(variable.value)
162+
// Always return raw value exactly as typed
163+
return typeof variable.value === 'string' ? variable.value : JSON.stringify(variable.value)
164+
}
165+
166+
// Get validation status based on type and value
167+
const getValidationStatus = (variable: Variable): string | undefined => {
168+
// Empty values don't need validation
169+
if (variable.value === '') return undefined
170+
171+
// Otherwise validate based on type
172+
switch (variable.type) {
173+
case 'number':
174+
return isNaN(Number(variable.value)) ? 'Not a valid number' : undefined
175+
case 'boolean':
176+
return !/^(true|false)$/i.test(String(variable.value).trim())
177+
? 'Expected "true" or "false"'
178+
: undefined
179+
case 'object':
180+
try {
181+
// Handle both JavaScript and JSON syntax
182+
let valueToValidate = String(variable.value).trim()
183+
184+
// If it's clearly JS syntax, convert it to valid JSON
185+
if (valueToValidate.includes("'") || /\b\w+\s*:/.test(valueToValidate)) {
186+
// Replace JS single quotes with double quotes, but handle escaped quotes correctly
187+
valueToValidate = valueToValidate
188+
.replace(/(\w+)\s*:/g, '"$1":') // Convert unquoted property names to quoted
189+
.replace(/'/g, '"') // Replace single quotes with double quotes
190+
}
191+
192+
const parsed = JSON.parse(valueToValidate)
193+
return !parsed || typeof parsed !== 'object' || Array.isArray(parsed)
194+
? 'Not a valid JSON object'
195+
: undefined
196+
} catch {
197+
return 'Invalid JSON object syntax'
198+
}
199+
case 'array':
200+
try {
201+
// Use actual JavaScript evaluation instead of trying to convert to JSON
202+
// This properly handles all valid JS array syntax including mixed types
203+
let valueToEvaluate = String(variable.value).trim()
204+
205+
// Basic security check to prevent arbitrary code execution
206+
if (!valueToEvaluate.startsWith('[') || !valueToEvaluate.endsWith(']')) {
207+
return 'Not a valid array format'
208+
}
209+
210+
// Use Function constructor to safely evaluate the array expression
211+
// This is safer than eval() and handles all JS array syntax correctly
212+
const parsed = new Function(`return ${valueToEvaluate}`)()
213+
214+
// Verify it's actually an array
215+
if (!Array.isArray(parsed)) {
216+
return 'Not a valid array'
217+
}
218+
219+
return undefined // Valid array
220+
} catch (e) {
221+
console.log('Array parsing error:', e)
222+
return 'Invalid array syntax'
223+
}
224+
default:
225+
return undefined
130226
}
131227
}
132228

@@ -140,20 +236,6 @@ export function Variables({ panelWidth }: VariablesProps) {
140236
})
141237
}, [workflowVariables])
142238

143-
// Handle editor value changes
144-
const handleEditorChange = (variable: Variable, newValue: string) => {
145-
try {
146-
// Use the VariableManager to consistently parse input values
147-
const processedValue = VariableManager.parseInputForStorage(newValue, variable.type)
148-
149-
// Update the variable with the processed value
150-
updateVariable(variable.id, { value: processedValue })
151-
} catch (e) {
152-
// If processing fails, use the raw value
153-
updateVariable(variable.id, { value: newValue })
154-
}
155-
}
156-
157239
return (
158240
<ScrollArea className="h-full">
159241
<div className="p-4 space-y-3">
@@ -199,11 +281,11 @@ export function Variables({ panelWidth }: VariablesProps) {
199281
</Tooltip>
200282
<DropdownMenuContent align="end" className="min-w-32">
201283
<DropdownMenuItem
202-
onClick={() => updateVariable(variable.id, { type: 'string' })}
284+
onClick={() => updateVariable(variable.id, { type: 'plain' })}
203285
className="cursor-pointer flex items-center"
204286
>
205-
<div className="w-5 text-center mr-2 font-mono text-sm">Aa</div>
206-
<span>String</span>
287+
<div className="w-5 text-center mr-2 font-mono text-sm">Abc</div>
288+
<span>Plain</span>
207289
</DropdownMenuItem>
208290
<DropdownMenuItem
209291
onClick={() => updateVariable(variable.id, { type: 'number' })}
@@ -233,13 +315,6 @@ export function Variables({ panelWidth }: VariablesProps) {
233315
<div className="w-5 text-center mr-2 font-mono text-sm">[]</div>
234316
<span>Array</span>
235317
</DropdownMenuItem>
236-
<DropdownMenuItem
237-
onClick={() => updateVariable(variable.id, { type: 'plain' })}
238-
className="cursor-pointer flex items-center"
239-
>
240-
<div className="w-5 text-center mr-2 font-mono text-sm">Abc</div>
241-
<span>Plain</span>
242-
</DropdownMenuItem>
243318
</DropdownMenuContent>
244319
</DropdownMenu>
245320

@@ -281,6 +356,10 @@ export function Variables({ panelWidth }: VariablesProps) {
281356
ref={(el) => {
282357
editorRefs.current[variable.id] = el
283358
}}
359+
style={{
360+
maxWidth: panelWidth ? `${panelWidth - 50}px` : '100%',
361+
overflowWrap: 'break-word',
362+
}}
284363
>
285364
{variable.value === '' && (
286365
<div className="absolute top-[8.5px] left-4 text-muted-foreground/50 pointer-events-none select-none">
@@ -291,6 +370,8 @@ export function Variables({ panelWidth }: VariablesProps) {
291370
key={`editor-${variable.id}-${variable.type}`}
292371
value={formatValue(variable)}
293372
onValueChange={handleEditorChange.bind(null, variable)}
373+
onBlur={() => handleEditorBlur(variable.id)}
374+
onFocus={() => handleEditorFocus(variable.id)}
294375
highlight={(code) =>
295376
highlight(
296377
code,
@@ -302,10 +383,31 @@ export function Variables({ panelWidth }: VariablesProps) {
302383
style={{
303384
fontFamily: 'inherit',
304385
lineHeight: '21px',
386+
width: '100%',
387+
wordWrap: 'break-word',
388+
whiteSpace: 'pre-wrap',
305389
}}
306390
className="focus:outline-none w-full"
307-
textareaClassName="focus:outline-none focus:ring-0 bg-transparent resize-none w-full overflow-hidden whitespace-pre-wrap"
391+
textareaClassName="focus:outline-none focus:ring-0 bg-transparent resize-none w-full whitespace-pre-wrap break-words overflow-visible"
308392
/>
393+
394+
{/* Show validation indicator for any non-empty variable */}
395+
{variable.value !== '' && (
396+
<Tooltip>
397+
<TooltipTrigger asChild>
398+
<div className="absolute top-[4px] right-[0px] cursor-help group">
399+
{getValidationStatus(variable) && (
400+
<div className="p-1 rounded-md group-hover:bg-muted/80 group-hover:shadow-sm transition-all duration-200 border border-transparent group-hover:border-muted/50">
401+
<AlertTriangle className="h-4 w-4 text-muted-foreground opacity-30 group-hover:opacity-100 transition-opacity duration-200" />
402+
</div>
403+
)}
404+
</div>
405+
</TooltipTrigger>
406+
<TooltipContent side="bottom" className="max-w-xs">
407+
{getValidationStatus(variable) && <p>{getValidationStatus(variable)}</p>}
408+
</TooltipContent>
409+
</Tooltip>
410+
)}
309411
</div>
310412
</div>
311413
))}

apps/sim/blocks/blocks/function.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@ export const FunctionBlock: BlockConfig<CodeExecutionOutput> = {
3030
type: {
3131
result: 'any',
3232
stdout: 'string',
33-
executionTime: 'number',
3433
},
3534
},
3635
},

apps/sim/executor/index.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1284,7 +1284,6 @@ export class Executor {
12841284
response: {
12851285
result: output?.result,
12861286
stdout: output?.stdout || '',
1287-
executionTime: output?.executionTime || 0,
12881287
},
12891288
}
12901289
}

apps/sim/executor/resolver.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -599,7 +599,7 @@ describe('InputResolver', () => {
599599
const result = resolver.resolveInputs(block, mockContext)
600600

601601
// String should be quoted in code context
602-
expect(result.code).toContain('const name = "\"Hello\"";')
602+
expect(result.code).toContain('const name = "Hello";')
603603
// Number should not be quoted
604604
expect(result.code).toContain('const num = 42;')
605605
})

apps/sim/executor/resolver.ts

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -242,8 +242,11 @@ export class InputResolver {
242242
}
243243

244244
try {
245+
// Handle 'string' type the same as 'plain' for backward compatibility
246+
const type = variable.type === 'string' ? 'plain' : variable.type
247+
245248
// Use the centralized VariableManager to resolve variable values
246-
return VariableManager.resolveForExecution(variable.value, variable.type)
249+
return VariableManager.resolveForExecution(variable.value, type)
247250
} catch (error) {
248251
logger.error(`Error processing variable ${variable.name} (type: ${variable.type}):`, error)
249252
return variable.value // Fallback to original value on error
@@ -266,14 +269,23 @@ export class InputResolver {
266269
currentBlock?: SerializedBlock
267270
): string {
268271
try {
272+
// Handle 'string' type the same as 'plain' for backward compatibility
273+
const normalizedType = type === 'string' ? 'plain' : type
274+
275+
// For plain text, use exactly what's entered without modifications
276+
if (normalizedType === 'plain' && typeof value === 'string') {
277+
return value
278+
}
279+
269280
// Determine if this needs special handling for code contexts
270281
const needsCodeStringLiteral = this.needsCodeStringLiteral(currentBlock, String(value))
282+
const isFunctionBlock = currentBlock?.metadata?.id === 'function'
271283

272-
// Use the appropriate formatting method based on context
273-
if (needsCodeStringLiteral) {
274-
return VariableManager.formatForCodeContext(value, type as any)
284+
// Always use code formatting for function blocks
285+
if (isFunctionBlock || needsCodeStringLiteral) {
286+
return VariableManager.formatForCodeContext(value, normalizedType as any)
275287
} else {
276-
return VariableManager.formatForTemplateInterpolation(value, type as any)
288+
return VariableManager.formatForTemplateInterpolation(value, normalizedType as any)
277289
}
278290
} catch (error) {
279291
logger.error(`Error formatting value for interpolation (type: ${type}):`, error)
@@ -1065,6 +1077,11 @@ export class InputResolver {
10651077

10661078
// Check if this is a block that executes code
10671079
if (block.metadata?.id && codeExecutionBlocks.includes(block.metadata.id)) {
1080+
// Always return true for function blocks - they need properly formatted string literals
1081+
if (block.metadata.id === 'function') {
1082+
return true
1083+
}
1084+
10681085
// Specifically for condition blocks, stringifyForCondition handles quoting
10691086
// so we don't need extra quoting here unless it's within an expression.
10701087
if (block.metadata.id === 'condition' && !expression) {

0 commit comments

Comments
 (0)