feat: javascript helpers extension

fix #881

Author: alandefreitas

include/mrdocs/Support/JavaScript.hpp

@@ -30,6 +30,30 @@ class Handlebars;
     These functions abstract over the embedded JavaScript engine so that
     scripts and helpers can be bound, invoked, and marshalled without leaking
     engine-specific types into the rest of the codebase.
+
+    ## Implementation Notes (for Python/Lua integrations)
+
+    The current implementation uses JerryScript with the following design choices:
+
+    ### Integer Limitations
+    JerryScript only guarantees 32-bit integer precision. Values outside the
+    int32 range (approximately ±2 billion) are converted to strings when passed
+    to JavaScript to avoid wraparound bugs. When reading values back, integers
+    that fit in int64 are returned as integers; others remain as doubles.
+
+    ### DOM ↔ JS Conversion Strategy
+    - **Objects**: Use lazy Proxy wrappers to avoid infinite recursion from
+      circular references (e.g., Handlebars symbol contexts that reference
+      parent symbols). Properties are converted on-demand when accessed.
+    - **Arrays**: Converted eagerly (snapshot semantics) because they rarely
+      contain circular references. This means JS mutations don't affect the
+      original C++ array and vice versa.
+    - **Functions**: Wrapped bidirectionally so JS can call C++ functions and
+      C++ can invoke JS functions through dom::Function.
+
+    ### Thread Safety
+    The Context mutex serializes all engine operations. Values can be safely
+    copied across threads; the underlying handle operations are protected.
 */
 namespace js {
 

src/lib/Gen/hbs/Builder.cpp

@@ -15,6 +15,7 @@
 #include <mrdocs/Metadata/DomCorpus.hpp>
 #include <mrdocs/Support/Path.hpp>
 #include <mrdocs/Support/Report.hpp>
+#include <algorithm>
 #include <filesystem>
 #include <format>
 #include <ranges>
@@ -272,6 +273,11 @@ registerUserHelpers(
     // Utility files (starting with '_') define shared globals and are
     // loaded before helper files. This allows helpers to share code
     // without duplicating implementations.
+    //
+    // Utility files are loaded in alphabetical order to ensure predictable
+    // behavior when utilities depend on each other (e.g., _a.js runs before
+    // _b.js). If you need complex dependencies, consider consolidating into
+    // a single utility file.
     std::vector<std::string> utilityFiles;
     std::vector<std::pair<std::string, std::string>> helperFiles; // (name, path)
 
@@ -305,6 +311,9 @@ registerUserHelpers(
             return Unexpected(exp.error());
     }
 
+    // Sort utility files alphabetically for predictable load order
+    std::sort(utilityFiles.begin(), utilityFiles.end());
+
     // Load utilities first (they define globals available to helpers).
     // Each utility is loaded in its own scope; globals persist across scopes.
     for (auto const& utilPath : utilityFiles)

src/lib/Support/JavaScript.cpp

@@ -57,13 +57,17 @@ invokeHelper(Value const& fn, dom::Array const& args)
 {
     if (args.empty())
     {
-        return Unexpected(Error("helper options missing"));
+        return Unexpected(Error(
+            "Handlebars helper called without arguments; "
+            "expected options object as last argument"));
     }
 
     dom::Value const& options = args.back();
     if (!options.isObject())
     {
-        return Unexpected(Error("helper options must be an object"));
+        return Unexpected(Error(
+            "Handlebars helper options must be an object; "
+            "ensure the helper is called from a template context"));
     }
 
     // Build arguments without the options object.
@@ -117,8 +121,13 @@ toString(jerry_value_t v)
 
 // Normalize a JerryScript exception into a MrDocs Error type.
 // Order: unwrap exception → if object use .message → else if string use it →
-// otherwise stringify the original exception. Prefix "Unexpected: " for
-// runtime errors to distinguish them from syntax-like failures.
+// otherwise stringify the original exception.
+//
+// Error message format:
+// - Syntax errors from JerryScript typically contain "Unexpected" or "SyntaxError"
+// - Runtime errors (thrown exceptions) are prefixed with "Unexpected: " if they
+//   don't already contain that marker, helping distinguish them from parse errors
+// - This prefix is intentionally consistent to aid debugging and testing
 static Error
 makeError(jerry_value_t exc)
 {
@@ -147,6 +156,9 @@ makeError(jerry_value_t exc)
         msg = toString(exc);
     }
 
+    // Prefix runtime exceptions for consistent error messaging. Skip if the
+    // message already indicates a syntax/parse error (contains "Unexpected")
+    // or if this isn't actually an exception value.
     if (jerry_value_is_exception(exc)
         && msg.find("Unexpected") == std::string::npos)
     {
@@ -380,6 +392,17 @@ Scope::compile_function(std::string_view script)
     // parenthesize to force expression parsing; if that fails, execute the
     // script and return the first declared function to match older behavior
     // used by templates.
+    //
+    // IMPORTANT: Side effects warning
+    // If the parenthesized expression attempt fails (e.g., for scripts with
+    // statements before the function), the fallback path executes the script
+    // to define the function. This means scripts with side effects may run
+    // those side effects even when compile_function ultimately fails. For
+    // pure function definitions this is not an issue, but scripts like:
+    //   "counter++; function foo() {}"
+    // will increment counter during compile_function even though the
+    // intent is only to extract the function.
+    //
     // Parenthesize the provided source so it is treated as a function expression
     std::string wrapped = "(";
     wrapped.append(script);
@@ -391,7 +414,7 @@ Scope::compile_function(std::string_view script)
     }
 
     // Fall back: execute declarations and return the first declared function
-    // name
+    // name. Note: this path runs the script, so any side effects will occur.
     auto findFirstFunctionName =
         [](std::string_view sv) -> std::optional<std::string> {
         std::size_t pos = 0;
@@ -533,6 +556,12 @@ Value::Value(Value const& other) : impl_(other.impl_), val_(0)
 {
     // Copy by bumping JerryScript handle refcount; paired with jerry_value_free
     // in the destructor for shared lifetime management across Value copies.
+    //
+    // Thread safety note: The shared_ptr copy (impl_) is done outside the lock
+    // because std::shared_ptr is thread-safe for concurrent copies. The
+    // jerry_value_copy call requires the lock since JerryScript is single-threaded.
+    // This allows Values to be safely copied across threads while ensuring all
+    // engine operations are serialized.
     auto lock = lockContext(other.impl_);
     if (other.val_)
     {
@@ -603,6 +632,8 @@ isSafeNumberForJerry(double d)
 {
     // JerryScript only guarantees 32-bit ints; reject wider values early to
     // avoid wraparound in the engine and round-trip surprises.
+    // Note: std::isfinite returns false for NaN and ±Infinity, so those are
+    // correctly rejected here without needing a separate std::isnan check.
     if (!std::isfinite(d))
     {
         return false;

src/test/Support/JavaScript.cpp

@@ -1813,6 +1813,127 @@ struct JavaScript_test
         }
     }
 
+    void
+    test_operator_bracket_access()
+    {
+        // Test operator[] for objects and arrays as a convenience alternative
+        // to the get() method.
+        Context ctx;
+        Scope scope(ctx);
+
+        // Object subscript access
+        {
+            Value obj = scope.eval("({ key: 'value', nested: { inner: 42 } })").value();
+            BOOST_TEST(obj.isObject());
+
+            // String key access
+            BOOST_TEST(obj["key"].isString());
+            BOOST_TEST(obj["key"].getString() == "value");
+
+            // Missing key returns undefined
+            BOOST_TEST(obj["missing"].isUndefined());
+
+            // Nested access via chained subscripts
+            BOOST_TEST(obj["nested"]["inner"].isNumber());
+            BOOST_TEST(obj["nested"]["inner"].getInteger() == 42);
+        }
+
+        // Array subscript access
+        {
+            Value arr = scope.eval("([10, 20, 30])").value();
+            BOOST_TEST(arr.isArray());
+
+            // Index access
+            BOOST_TEST(arr[0].isNumber());
+            BOOST_TEST(arr[0].getInteger() == 10);
+            BOOST_TEST(arr[1].getInteger() == 20);
+            BOOST_TEST(arr[2].getInteger() == 30);
+
+            // Out of bounds returns undefined
+            BOOST_TEST(arr[99].isUndefined());
+        }
+    }
+
+    void
+    test_getstring_owning_string()
+    {
+        // getString() returns std::string (owning) rather than string_view
+        // because JerryScript allocates new buffers for string extraction.
+        // This test documents this API behavior for users migrating from
+        // other JS engines that might return views.
+        Context ctx;
+        Scope scope(ctx);
+
+        Value str = scope.eval("'Hello, World!'").value();
+        BOOST_TEST(str.isString());
+
+        // getString returns std::string - verify it's a proper copy
+        std::string result = str.getString();
+        BOOST_TEST(result == "Hello, World!");
+
+        // The returned string should remain valid even after scope operations
+        // (unlike a string_view which might be invalidated)
+        scope.eval("'something else'");
+        BOOST_TEST(result == "Hello, World!"); // Still valid
+
+        // Works with non-ASCII UTF-8 content
+        Value utf8 = scope.eval("'日本語'").value();
+        std::string utf8Result = utf8.getString();
+        BOOST_TEST(utf8Result == "日本語");
+    }
+
+    void
+    test_utility_file_globals()
+    {
+        // Test that globals defined in one scope persist to subsequent scopes,
+        // which is the mechanism utility files use to provide shared functions.
+        Context ctx;
+
+        // First scope: define a utility function (simulates loading _utils.js)
+        {
+            Scope scope(ctx);
+            auto exp = scope.script(
+                "function sharedUtil(x) { return x * 2; }\n"
+                "var SHARED_CONSTANT = 42;");
+            BOOST_TEST(exp);
+        }
+
+        // Second scope: verify globals persist and can be used
+        {
+            Scope scope(ctx);
+
+            // Function should be available
+            auto result = scope.eval("sharedUtil(21)");
+            BOOST_TEST(result);
+            if (result)
+            {
+                BOOST_TEST(result->isNumber());
+                BOOST_TEST(result->getInteger() == 42);
+            }
+
+            // Constant should be available
+            auto constVal = scope.getGlobal("SHARED_CONSTANT");
+            BOOST_TEST(constVal);
+            if (constVal)
+            {
+                BOOST_TEST(constVal->isNumber());
+                BOOST_TEST(constVal->getInteger() == 42);
+            }
+        }
+
+        // Third scope: test that a helper can use the utility function
+        Handlebars hbs;
+        auto ok = js::registerHelper(
+            hbs,
+            "doubler",
+            ctx,
+            "(function(x) { return sharedUtil(x); })");
+        BOOST_TEST(ok);
+        // Registration succeeded, meaning the helper script was able to
+        // reference sharedUtil from the global scope. The helper is now
+        // registered and usable in templates.
+    }
+
     void run()
     {
         test_context();
@@ -1834,6 +1955,9 @@ struct JavaScript_test
         test_utility_globals_persist();
         test_circular_references();
         test_deep_nesting();
+        test_operator_bracket_access();
+        test_getstring_owning_string();
+        test_utility_file_globals();
     }
 };