feat: complete JavaScript helpers support

This commit uses proxy objects to offer complete support for JavaScript helpers that return reference types. Previously, JavaScript reference types returned by these functions were deep-copied or not handled at all. Now, proxy objects in both directions are used while helpers create a scope that's kept alive as long as necessary by the Handlebars engine.

As the objects don't need to be deep copied, this change improves performance and allows objects with circular references, which are common in MrDocs. Additionally, JavaScript helpers receive a proxy object equivalent to the handlebars `options` object, and helper function registration was also simplified and improved to remove redundant code.

This commit provides new test cases to validate the current code without counting on MrDocs.

CMakeUserPresets.json.example

@@ -60,6 +60,15 @@
         "Clang_ROOT": "C:\\Users\\$env{USERNAME}\\Libraries\\llvm-project\\llvm\\install\\MSVC\\RelWithDebInfo"
       }
     },
+    {
+      "name": "DebWithOpt-ClangCL",
+      "inherits": "DebWithOpt-MSVC",
+      "binaryDir": "${sourceDir}/build/${presetName}",
+      "cacheVariables": {
+        "CMAKE_C_COMPILER": "clang-cl.exe",
+        "CMAKE_CXX_COMPILER": "clang-cl.exe"
+      }
+    },
     {
       "name": "Debug-GCC",
       "inherits": "debug",

include/mrdocs/Support/JavaScript.hpp

@@ -20,6 +20,9 @@
 
 namespace clang {
 namespace mrdocs {
+
+class Handlebars;
+
 namespace js {
 
 struct Access;
@@ -214,6 +217,42 @@ class Scope
     MRDOCS_DECL
     ~Scope();
 
+    /** Push an integer to the stack
+     */
+    MRDOCS_DECL
+    Value
+    pushInteger(std::int64_t value);
+
+    /** Push a double to the stack
+     */
+    MRDOCS_DECL
+    Value
+    pushDouble(double value);
+
+    /** Push a boolean to the stack
+     */
+    MRDOCS_DECL
+    Value
+    pushBoolean(bool value);
+
+    /** Push a string to the stack
+     */
+    MRDOCS_DECL
+    Value
+    pushString(std::string_view value);
+
+    /** Push a new object to the stack
+     */
+    MRDOCS_DECL
+    Value
+    pushObject();
+
+    /** Push a new array to the stack
+     */
+    MRDOCS_DECL
+    Value
+    pushArray();
+
     /** Compile and run a script.
 
         This function compiles and executes
@@ -973,6 +1012,20 @@ isFunction() const noexcept
     return type() == Type::function;
 }
 
+/** Register a JavaScript helper function
+
+    This function registers a JavaScript function
+    as a helper function that can be called from
+    Handlebars templates.
+ */
+MRDOCS_DECL
+Expected<void, Error>
+registerHelper(
+    clang::mrdocs::Handlebars& hbs,
+    std::string_view name,
+    Context& ctx,
+    std::string_view script);
+
 } // js
 } // mrdocs
 } // clang

include/mrdocs/Support/Path.hpp

@@ -44,32 +44,80 @@ forEachFile(
     bool recursive,
     AnyFileVisitor& visitor);
 
-/** Visit each file in a directory.
-*/
-template<class Visitor>
-Error
-forEachFile(
-    std::string_view dirPath,
-    bool recursive,
-    Visitor&& visitor)
+namespace detail {
+template <class Visitor>
+struct FileVisitor : AnyFileVisitor
 {
-    struct FileVisitor : AnyFileVisitor
+    Visitor& visitor_;
+
+    explicit FileVisitor(Visitor& v)
+        : visitor_(v)
     {
-        Visitor& visitor_;
+    }
 
-        explicit FileVisitor(Visitor& v)
-            : visitor_(v)
+    Error
+    visitFile(std::string_view fileName) override
+    {
+        using R = std::invoke_result_t<Visitor, std::string_view>;
+        if (std::same_as<R, void>)
+        {
+            visitor_(fileName);
+            return Error::success();
+        }
+        else
         {
+            return toError(visitor_(fileName));
         }
+    }
+
+    static
+    Error
+    toError(Expected<void, Error> const& e)
+    {
+        return e ? Error::success() : e.error();
+    }
+
+    template <class T>
+    static
+    Error
+    toError(Expected<T, Error> const& e)
+    {
+        return e ? toError(e.value()) : e.error();
+    }
 
-        Error
-        visitFile(std::string_view fileName) override
+    template <class T>
+    static
+    Error
+    toError(T const& e)
+    {
+        if constexpr (std::same_as<T, Error>)
+        {
+            return e;
+        }
+        else if constexpr (std::convertible_to<T, bool>)
+        {
+            if (e)
+                return Error::success();
+            return Error("visitor returned falsy");
+        }
+        else
         {
-            return visitor_(fileName);
+            return Error::success();
         }
-    };
+    }
+};
+}
 
-    FileVisitor v{visitor};
+/** Visit each file in a directory.
+*/
+template<class Visitor>
+Error
+forEachFile(
+    std::string_view dirPath,
+    bool recursive,
+    Visitor&& visitor)
+{
+    detail::FileVisitor<Visitor> v{visitor};
     return forEachFile(dirPath, recursive,
         static_cast<AnyFileVisitor&>(v));
 }

share/mrdocs/addons/generator/html/helpers/add.js

@@ -0,0 +1,3 @@
+function add(a, b) {
+    return a + b;
+}

src/lib/Gen/adoc/Builder.cpp

@@ -57,75 +57,28 @@ Builder(
             return Error::success();
     }).maybeThrow();
 
-    // load helpers
-    js::Scope scope(ctx_);
+    // Load JavaScript helpers
     std::string helpersPath = files::appendPath(
         config->addonsDir, "generator", "asciidoc", "helpers");
     forEachFile(helpersPath, true,
-        [&](std::string_view pathName)
+        [&](std::string_view pathName)-> Expected<void>
         {
             // Register JS helper function in the global object
             constexpr std::string_view ext = ".js";
-            if (!pathName.ends_with(ext))
-            {
-                return Error::success();
-            }
+            if (!pathName.ends_with(ext)) return {};
             auto name = files::getFileName(pathName);
             name.remove_suffix(ext.size());
-            auto text = files::getFileText(pathName);
-            if (!text)
-            {
-                return text.error();
-            }
-            auto JSFn = scope.compile_function(*text);
-            if (!JSFn)
-            {
-                return JSFn.error();
-            }
-            scope.getGlobalObject().set(name, *JSFn);
-
-            // Register C++ helper that retrieves the helper
-            // from the global object, converts the arguments,
-            // and invokes the JS function.
-            hbs_.registerHelper(name, dom::makeVariadicInvocable(
-                [this, name=std::string(name)](
-                    dom::Array const& args) -> Expected<dom::Value>
-                {
-                    // Get function from global scope
-                    js::Scope scope(ctx_);
-                    js::Value fn = scope.getGlobalObject().get(name);
-                    if (fn.isUndefined())
-                    {
-                        return Unexpected(Error("helper not found"));
-                    }
-                    if (!fn.isFunction())
-                    {
-                        return Unexpected(Error("helper is not a function"));
-                    }
-
-                    // Call function
-                    std::vector<dom::Value> arg_span;
-                    arg_span.reserve(args.size());
-                    for (auto const& arg : args)
-                    {
-                        arg_span.push_back(arg);
-                    }
-                    auto result = fn.apply(arg_span);
-                    if (!result)
-                    {
-                        return dom::Kind::Undefined;
-                    }
-
-                    // Convert result to dom::Value
-                    return result->getDom();
-                }));
-            return Error::success();
+            MRDOCS_TRY(auto script, files::getFileText(pathName));
+            MRDOCS_TRY(js::registerHelper(hbs_, name, ctx_, script));
+            return {};
         }).maybeThrow();
+
     hbs_.registerHelper(
         "is_multipage",
         dom::makeInvocable([res = config->multiPage]() -> Expected<dom::Value> {
         return res;
     }));
+
     hbs_.registerHelper("primary_location",
         dom::makeInvocable([](dom::Value const& v) ->
             dom::Value
@@ -162,6 +115,7 @@ Builder(
             }
             return first;
         }));
+
     helpers::registerStringHelpers(hbs_);
     helpers::registerAntoraHelpers(hbs_);
     helpers::registerContainerHelpers(hbs_);
@@ -183,8 +137,6 @@ callTemplate(
     MRDOCS_TRY(auto fileText, files::getFileText(pathName));
     HandlebarsOptions options;
     options.noEscape = true;
-    // options.compat = true;
-
     Expected<std::string, HandlebarsError> exp =
         hbs_.try_render(fileText, context, options);
     if (!exp)

src/lib/Gen/html/Builder.cpp

@@ -60,70 +60,23 @@ Builder(
         return Error::success();
     }).maybeThrow();
 
-    // load helpers
-    js::Scope scope(ctx_);
+    // Load JavaScript helpers
     std::string helpersPath = files::appendPath(
-        config->addonsDir, "generator", "asciidoc", "helpers");
+        config->addonsDir, "generator", "html", "helpers");
     forEachFile(helpersPath, true,
-                [&](std::string_view pathName)
-    {
-        // Register JS helper function in the global object
-        constexpr std::string_view ext = ".js";
-        if (!pathName.ends_with(ext))
-        {
-            return Error::success();
-        }
-        auto name = files::getFileName(pathName);
-        name.remove_suffix(ext.size());
-        auto text = files::getFileText(pathName);
-        if (!text)
+        [&](std::string_view pathName)-> Expected<void>
         {
-            return text.error();
-        }
-        auto JSFn = scope.compile_function(*text);
-        if (!JSFn)
-        {
-            return JSFn.error();
-        }
-        scope.getGlobalObject().set(name, *JSFn);
-
-        // Register C++ helper that retrieves the helper
-        // from the global object, converts the arguments,
-        // and invokes the JS function.
-        hbs_.registerHelper(name, dom::makeVariadicInvocable(
-                                      [this, name=std::string(name)](
-                                          dom::Array const& args) -> Expected<dom::Value>
-        {
-            // Get function from global scope
-            js::Scope scope(ctx_);
-            js::Value fn = scope.getGlobalObject().get(name);
-            if (fn.isUndefined())
-            {
-                return Unexpected(Error("helper not found"));
-            }
-            if (!fn.isFunction())
-            {
-                return Unexpected(Error("helper is not a function"));
-            }
-
-            // Call function
-            std::vector<dom::Value> arg_span;
-            arg_span.reserve(args.size());
-            for (auto const& arg : args)
-            {
-                arg_span.push_back(arg);
-            }
-            auto result = fn.apply(arg_span);
-            if (!result)
-            {
-                return dom::Kind::Undefined;
-            }
-
-            // Convert result to dom::Value
-            return result->getDom();
-        }));
-        return Error::success();
-    }).maybeThrow();
+            // Register JS helper function in the global object
+            constexpr std::string_view ext = ".js";
+            if (!pathName.ends_with(ext))
+                return {};
+            auto name = files::getFileName(pathName);
+            name.remove_suffix(ext.size());
+            MRDOCS_TRY(auto script, files::getFileText(pathName));
+            MRDOCS_TRY(js::registerHelper(hbs_, name, ctx_, script));
+            return {};
+        }).maybeThrow();
+
     hbs_.registerHelper(
         "is_multipage",
         dom::makeInvocable([res = config->multiPage]() -> Expected<dom::Value> {