Skip to content
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

feat: added require to load scripts using CommonJs #2272

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

JulienLeal
Copy link

@JulienLeal JulienLeal commented Jan 26, 2025

Proposed changes

Enabled CommonJS require support in GraalJS engine for Maestro flows

  • Added GraalJS context configuration with js.commonjs-require and js.commonjs-require-cwd options
  • Implemented module resolution relative to the executed YAML file's directory
  • Set MAESTRO_YAML_DIR environment variable automatically for script context
  • Added validation for module directory existence
  • Modified RunScriptCommand to propagate YAML file location context

Testing

Added integration test:

  • Case 121 - CommonJS require functionality
    • Verifies successful module resolution from YAML file directory
    • Tests cross-file require() with JSON serialization
    • Validates console output logging

Manual verification:

  1. Tested multi-level directory structures
  2. Verified relative path resolution (./lib/utils.js)
  3. Checked error handling for missing modules
  4. Validated Windows/Linux path compatibility

Issues fixed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant