Skip to content

Conversation

@vanch3d
Copy link
Contributor

@vanch3d vanch3d commented Dec 9, 2025

Pull Request: JavaScript Validation for DataHub Scripts

Kanban Ticket: https://hivemq.kanbanize.com/ctrl_board/57/cards/38512/details/

Note

For context, I'm adding validation to the RJSF infrastructure.
The implementation is the third iteration. A Monaco-based was too complex due to async/sync race conditions
This one is a clean approach BUT (and it's a big but) add 1.6M to the bundle size. I don't think it's worth the effort


Description

This PR adds real-time JavaScript validation to the DataHub Script Editor, transforming how users write transformation functions. Previously, users could save scripts with syntax errors and only discover problems at runtime. Now, users receive instant feedback about syntax problems while editing, with the Save button automatically disabled until errors are resolved.

The enhancement introduces:

  • Real-time validation: JavaScript syntax checking as users type, using TypeScript's compiler API
  • Inline error messages: Clear error descriptions with line and column numbers displayed beneath the code editor
  • Save protection: Automatic Save button disabling when validation errors exist
  • Persistent error state: Errors remain visible even when editing other fields, preventing accidental saves

Technical Summary

Implementation highlights:

  • Uses TypeScript Compiler API (ts.createSourceFile() and ts.getPreEmitDiagnostics()) for synchronous validation
  • Integrates with RJSF customValidate hook for seamless form validation
  • No code execution—purely static analysis for security
  • 46 unit tests for validation logic, 9 Cypress component tests for UI integration
  • Performance: 10-20ms validation time (5-10x faster than async Monaco approach)
  • Trade-off: Bundle size increased 1.3MB (2.2MB → 3.5MB) to include TypeScript compiler

Out-of-scope

BEFORE

Previous Behavior - No Validation

The Script Editor allowed users to save transformation functions without checking for syntax errors:

Limitations:

  • No feedback about syntax problems until script execution failed at runtime
  • Users could save scripts with missing braces, undefined variables, or unclosed strings
  • Debugging required manual testing with real data to discover syntax errors
  • Previous new Function() validation was disabled due to security concerns (code injection risk)

AFTER

https://www.loom.com/share/d4b32ae0b63c48abbc0b514a2486d3c0

New Behavior - Real-Time JavaScript Validation

The Script Editor now validates JavaScript syntax as users type, providing immediate feedback and preventing invalid scripts from being saved.

image

Key Visual Elements:

  • Error Alert Box: Red-bordered alert at top of form showing "Line 3, Column 12: '}' expected"
  • Line and Column Numbers: Precise location of the syntax problem for quick fixing
  • Error Description: Clear TypeScript compiler message explaining what's wrong
  • Disabled Save Button: Grayed out and unclickable until error is resolved
  • Monaco Editor: Code editor with the invalid JavaScript visible

Test Coverage

Comprehensive Testing

  • 55+ tests total, all passing ✅
  • Unit tests (46): TypeScript validator logic (tsValidator.spec.ts)
    • Valid code acceptance
    • Syntax error detection (missing braces, unclosed strings, etc.)
    • Error message formatting
    • Performance validation
  • Component tests (9): ScriptEditor validation integration (ScriptEditor.spec.cy.tsx)
    • Error display on invalid code
    • Error clearing when fixed
    • Save button state management
    • Error persistence across field changes
    • Multiple error handling (name + JS validation)
    • Create and modify mode validation

Performance Impact

Significant impact:

  • ⚠️ Bundle size increase: Production bundle increased from 2.2MB to 3.5MB (+1.3MB, ~60% larger)
    • TypeScript Compiler API added to bundle for validation
    • Users download larger initial bundle (one-time cost)
    • CI builds require 8GB Node heap memory (up from default 1.5GB)

Reviewer Notes

Focus areas for review:

  1. Validation accuracy: Test with various JavaScript syntax errors to verify error messages are helpful
  2. Performance: Confirm validation feels instant even with complex code
  3. Error persistence: Verify errors don't disappear when changing other fields (critical bug fix)
  4. User experience: Check that error messages guide users to fixes effectively

Manual testing suggestions:

  1. Create a new script and type function test() { (missing closing brace)
  2. Observe error appears: "'}' expected" with line/column number
  3. Complete the function to function test() { return true; }
  4. Verify error clears immediately and Save button enables
  5. Add a typo in a variable name, verify suggestion appears
  6. Change the description field, verify JS error persists

@vanch3d vanch3d self-assigned this Dec 9, 2025
@cla-bot cla-bot bot added the cla-signed label Dec 9, 2025
@github-actions
Copy link

github-actions bot commented Dec 11, 2025

Test Results

  483 files  ±0    483 suites  ±0   5m 52s ⏱️ ±0s
4 053 tests ±0  4 050 ✅ ±0  3 💤 ±0  0 ❌ ±0 
4 070 runs  ±0  4 067 ✅ ±0  3 💤 ±0  0 ❌ ±0 

Results for commit 1fa4651. ± Comparison against base commit eabcb94.

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented Dec 11, 2025

Coverage Report

Overall Project 65.45%

There is no coverage information present for the Files changed

@vanch3d vanch3d marked this pull request as ready for review December 11, 2025 13:19
@vanch3d vanch3d requested a review from Copilot December 11, 2025 13:29
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR implements JavaScript validation for Data Hub scripts using the TypeScript Compiler API, replacing the previously disabled new Function() approach that had security concerns. The solution provides synchronous validation that integrates cleanly with RJSF's customValidate function.

Key changes:

  • Added TypeScript Compiler API-based validation (tsValidator.ts) for synchronous JavaScript syntax checking
  • Created Monaco-based async validation hooks (useJavaScriptValidation) as an alternative approach
  • Integrated validation into ScriptEditor using the synchronous TypeScript compiler approach
  • Added comprehensive test coverage (468 unit tests for tsValidator, 149 for useJavaScriptValidation hook, 275 for javascriptValidator)

Reviewed changes

Copilot reviewed 28 out of 29 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
useJavaScriptValidation.ts React hook for Monaco-based async validation with graceful degradation
useJavaScriptValidation.spec.ts Test suite for validation hook (149 tests)
tsValidator.ts Synchronous JavaScript validation using TypeScript Compiler API
tsValidator.spec.ts Comprehensive test suite for synchronous validator (468 tests)
javascriptValidator.ts Monaco-based async validator implementation
javascriptValidator.spec.ts Test suite for Monaco validator (275 tests)
index.ts Exports for validation module
ScriptEditor.tsx Integration of synchronous validation into form's customValidate
ScriptEditor.spec.cy.tsx New Cypress tests for validation integration (323 new lines)
protobuf.config.ts Added type annotation for language list
datahub-commands.ts Added type annotation for model parameter
CodeEditor.Protobuf.spec.cy.tsx Added type annotations for language arrays
package.json Upgraded monaco-editor to 0.55.1, added js-yaml override
check-frontend.yml Increased Node memory to 8GB for TypeScript compiler
Documentation files Added extensive task documentation and testing guidelines

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@sonarqubecloud
Copy link

Copy link
Contributor

@oli-hivemq oli-hivemq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🦣

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants