-
Notifications
You must be signed in to change notification settings - Fork 3.4k
feat(editor): add syntax error detection for JS and Python code blocks #3407
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
waleedlatif1
wants to merge
2
commits into
staging
Choose a base branch
from
waleedlatif1/syntax-error-detection
base: staging
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+398
−28
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
197 changes: 197 additions & 0 deletions
197
...p/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/utils.test.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,197 @@ | ||
| /** | ||
| * @vitest-environment node | ||
| */ | ||
| import { describe, expect, it } from 'vitest' | ||
| import { sanitizeForParsing, validateJavaScript, validatePython } from './utils' | ||
|
|
||
| describe('sanitizeForParsing', () => { | ||
| it('replaces <Block.output> references with valid identifiers', () => { | ||
| const result = sanitizeForParsing('const x = <Block.output>') | ||
| expect(result).not.toContain('<') | ||
| expect(result).not.toContain('>') | ||
| expect(result).toContain('__placeholder_') | ||
| }) | ||
|
|
||
| it('replaces {{ENV_VAR}} with valid identifiers', () => { | ||
| const result = sanitizeForParsing('const url = {{API_URL}}') | ||
| expect(result).not.toContain('{{') | ||
| expect(result).not.toContain('}}') | ||
| expect(result).toContain('__placeholder_') | ||
| }) | ||
|
|
||
| it('replaces nested path references like <Block.output[0].field>', () => { | ||
| const result = sanitizeForParsing('const x = <Agent.response.choices[0].text>') | ||
| expect(result).not.toContain('<Agent') | ||
| }) | ||
|
|
||
| it('replaces loop/parallel context references', () => { | ||
| const result = sanitizeForParsing('const item = <loop.currentItem>') | ||
| expect(result).not.toContain('<loop') | ||
| }) | ||
|
|
||
| it('replaces variable references', () => { | ||
| const result = sanitizeForParsing('const v = <variable.myVar>') | ||
| expect(result).not.toContain('<variable') | ||
| }) | ||
|
|
||
| it('handles multiple references in one string', () => { | ||
| const code = 'const a = <Block1.out>; const b = {{SECRET}}; const c = <Block2.value>' | ||
| const result = sanitizeForParsing(code) | ||
| expect(result).not.toContain('<Block1') | ||
| expect(result).not.toContain('{{SECRET}}') | ||
| expect(result).not.toContain('<Block2') | ||
| expect(result.match(/__placeholder_/g)?.length).toBe(3) | ||
| }) | ||
|
|
||
| it('does not replace regular JS comparison operators', () => { | ||
| const code = 'if (a < b && c > d) {}' | ||
| const result = sanitizeForParsing(code) | ||
| expect(result).toBe(code) | ||
| }) | ||
|
|
||
| it('does not replace HTML tags that are not references', () => { | ||
| const code = 'const html = "<div>hello</div>"' | ||
| const result = sanitizeForParsing(code) | ||
| expect(result).toBe(code) | ||
| }) | ||
| }) | ||
|
|
||
| describe('validateJavaScript', () => { | ||
| it('returns null for valid JavaScript', () => { | ||
| expect(validateJavaScript('const x = 1')).toBeNull() | ||
| expect(validateJavaScript('function foo() { return 42 }')).toBeNull() | ||
| expect(validateJavaScript('const arr = [1, 2, 3].map(x => x * 2)')).toBeNull() | ||
| }) | ||
|
|
||
| it('returns null for valid async/await code', () => { | ||
| expect(validateJavaScript('async function foo() { await bar() }')).toBeNull() | ||
| }) | ||
|
|
||
| it('returns null for bare return statements (function block wraps in async fn)', () => { | ||
| expect(validateJavaScript('return 42')).toBeNull() | ||
| expect(validateJavaScript(sanitizeForParsing('return <Block.output>'))).toBeNull() | ||
| expect(validateJavaScript('const x = 1\nreturn x')).toBeNull() | ||
| }) | ||
|
|
||
| it('returns null for await at top level (wrapped in async fn)', () => { | ||
| expect(validateJavaScript('const res = await fetch("url")')).toBeNull() | ||
| }) | ||
|
|
||
| it('returns null for valid ES module syntax', () => { | ||
| expect(validateJavaScript('import { foo } from "bar"')).toBeNull() | ||
| expect(validateJavaScript('export default function() {}')).toBeNull() | ||
| }) | ||
|
|
||
| it('detects missing closing brace', () => { | ||
| const result = validateJavaScript('function foo() {') | ||
| expect(result).not.toBeNull() | ||
| expect(result).toContain('Syntax error') | ||
| }) | ||
|
|
||
| it('detects missing closing paren', () => { | ||
| const result = validateJavaScript('console.log("hello"') | ||
| expect(result).not.toBeNull() | ||
| expect(result).toContain('Syntax error') | ||
| }) | ||
|
|
||
| it('detects unexpected token', () => { | ||
| const result = validateJavaScript('const = 5') | ||
| expect(result).not.toBeNull() | ||
| expect(result).toContain('Syntax error') | ||
| }) | ||
|
|
||
| it('includes adjusted line and column in error message', () => { | ||
| const result = validateJavaScript('const x = 1\nconst = 5') | ||
| expect(result).toMatch(/line 2/) | ||
| expect(result).toMatch(/col \d+/) | ||
| }) | ||
|
|
||
| it('returns null for empty code', () => { | ||
| expect(validateJavaScript('')).toBeNull() | ||
| }) | ||
|
|
||
| it('does not error on sanitized references', () => { | ||
| const code = sanitizeForParsing('const x = <Block.output> + {{ENV_VAR}}') | ||
| expect(validateJavaScript(code)).toBeNull() | ||
| }) | ||
| }) | ||
|
|
||
| describe('validatePython', () => { | ||
| it('returns null for valid Python', () => { | ||
| expect(validatePython('x = 1')).toBeNull() | ||
| expect(validatePython('def foo():\n return 42')).toBeNull() | ||
| expect(validatePython('arr = [1, 2, 3]')).toBeNull() | ||
| }) | ||
|
|
||
| it('returns null for Python with comments', () => { | ||
| expect(validatePython('x = 1 # this is a comment')).toBeNull() | ||
| expect(validatePython('# full line comment\nx = 1')).toBeNull() | ||
| }) | ||
|
|
||
| it('returns null for Python with strings containing brackets', () => { | ||
| expect(validatePython('x = "hello (world)"')).toBeNull() | ||
| expect(validatePython("x = 'brackets [here] {too}'")).toBeNull() | ||
| }) | ||
|
|
||
| it('returns null for triple-quoted strings', () => { | ||
| expect(validatePython('x = """hello\nworld"""')).toBeNull() | ||
| expect(validatePython("x = '''multi\nline\nstring'''")).toBeNull() | ||
| }) | ||
|
|
||
| it('returns null for triple-quoted strings with brackets', () => { | ||
| expect(validatePython('x = """has { and ( inside"""')).toBeNull() | ||
| }) | ||
|
|
||
| it('detects unmatched opening paren', () => { | ||
| const result = validatePython('foo(1, 2') | ||
| expect(result).not.toBeNull() | ||
| expect(result).toContain("'('") | ||
| }) | ||
|
|
||
| it('detects unmatched closing paren', () => { | ||
| const result = validatePython('foo)') | ||
| expect(result).not.toBeNull() | ||
| expect(result).toContain("')'") | ||
| }) | ||
|
|
||
| it('detects unmatched bracket', () => { | ||
| const result = validatePython('arr = [1, 2') | ||
| expect(result).not.toBeNull() | ||
| expect(result).toContain("'['") | ||
| }) | ||
|
|
||
| it('detects unterminated string', () => { | ||
| const result = validatePython('x = "hello') | ||
| expect(result).not.toBeNull() | ||
| expect(result).toContain('Unterminated string') | ||
| }) | ||
|
|
||
| it('detects unterminated triple-quoted string', () => { | ||
| const result = validatePython('x = """hello') | ||
| expect(result).not.toBeNull() | ||
| expect(result).toContain('Unterminated triple-quoted string') | ||
| }) | ||
|
|
||
| it('includes line number in error', () => { | ||
| const result = validatePython('x = 1\ny = (2') | ||
| expect(result).toMatch(/line 2/) | ||
| }) | ||
|
|
||
| it('handles escaped quotes in strings', () => { | ||
| expect(validatePython('x = "hello \\"world\\""')).toBeNull() | ||
| expect(validatePython("x = 'it\\'s fine'")).toBeNull() | ||
| }) | ||
|
|
||
| it('handles brackets inside comments', () => { | ||
| expect(validatePython('x = 1 # unmatched ( here')).toBeNull() | ||
| }) | ||
|
|
||
| it('returns null for empty code', () => { | ||
| expect(validatePython('')).toBeNull() | ||
| }) | ||
|
|
||
| it('does not error on sanitized references', () => { | ||
| const code = sanitizeForParsing('x = <Block.output> + {{ENV_VAR}}') | ||
| expect(validatePython(code)).toBeNull() | ||
| }) | ||
| }) |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
handleValidationChangenot memoized — triggers unnecessary re-rendershandleValidationChangeis a plain function created on every render. Because it is passed as theonValidationChangeprop toCode(which is amemocomponent with default shallow comparison), every state update insideSubBlockComponent— including those caused byhandleValidationChangeitself — creates a new function reference, which then causesCodeto re-render and re-fire theuseEffectthat hasonValidationChangein its dependency array. This sets a fresh 150 ms timer for invalid code, and the cycle repeats once more before React's bail-out for equal state values finally stops it.Wrapping the callback in
useCallbackwith an empty dependency array (thesetStatefunctions fromuseStateare stable) fixes this: