Update for Node.js embedding API changes (#219)

Author: jasongin

.github/workflows/build.yml

@@ -87,7 +87,8 @@ jobs:
       continue-on-error: true
 
     - name: Upload test logs
-      uses: actions/upload-artifact@v4
+      # upload-artifact@v4 breaks the test reporter: https://github.com/dorny/test-reporter/issues/343
+      uses: actions/upload-artifact@v3
       with:
         name: test-logs-${{ matrix.os }}-${{matrix.dotnet-version}}-node${{matrix.node-version}}-${{matrix.configuration}}
         path: |

bench/Benchmarks.cs

@@ -88,7 +88,8 @@ protected void Setup()
 
         // This setup avoids using NodejsEnvironment so benchmarks can run on the same thread.
         // NodejsEnvironment creates a separate thread that would slow down the micro-benchmarks.
-        platform.Runtime.CreateEnvironment(platform, Console.WriteLine, null, out _env)
+        platform.Runtime.CreateEnvironment(
+            platform, Console.WriteLine, null, NodejsEnvironment.NodeApiVersion, out _env)
             .ThrowIfFailed();
 
         // The new scope instance saves itself as the thread-local JSValueScope.Current.

src/NodeApi/Runtime/JSRuntime.cs

@@ -521,14 +521,14 @@ public virtual napi_status GetBufferInfo(
 
     public virtual napi_status CreatePlatform(
         string[]? args,
-        string[]? execArgs,
         Action<string>? errorHandler,
         out napi_platform result) => throw NS();
     public virtual napi_status DestroyPlatform(napi_platform platform) => throw NS();
     public virtual napi_status CreateEnvironment(
         napi_platform platform,
         Action<string>? errorHandler,
         string? mainScript,
+        int apiVersion,
         out napi_env result) => throw NS();
     public virtual napi_status DestroyEnvironment(napi_env env, out int exitCode) => throw NS();
     public virtual napi_status RunEnvironment(napi_env env) => throw NS();

src/NodeApi/Runtime/NodejsEnvironment.cs

@@ -23,6 +23,11 @@ namespace Microsoft.JavaScript.NodeApi.Runtime;
 /// </remarks>
 public sealed class NodejsEnvironment : IDisposable
 {
+    /// <summary>
+    /// Corresponds to NAPI_VERSION from js_native_api.h.
+    /// </summary>
+    public const int NodeApiVersion = 8;
+
     private readonly JSValueScope _scope;
     private readonly Thread _thread;
     private readonly TaskCompletionSource<bool> _completion = new();
@@ -44,6 +49,7 @@ internal NodejsEnvironment(NodejsPlatform platform, string? mainScript)
                 (napi_platform)platform,
                 (error) => Console.WriteLine(error),
                 mainScript,
+                NodeApiVersion,
                 out napi_env env).ThrowIfFailed();
 
             // The new scope instance saves itself as the thread-local JSValueScope.Current.

src/NodeApi/Runtime/NodejsPlatform.cs

@@ -26,14 +26,12 @@ public sealed class NodejsPlatform : IDisposable
     /// Initializes the Node.js platform.
     /// </summary>
     /// <param name="libnodePath">Path to the `libnode` shared library, including extension.</param>
-    /// <param name="args">Optional application arguments.</param>
-    /// <param name="execArgs">Optional platform options.</param>
+    /// <param name="args">Optional platform arguments.</param>
     /// <exception cref="InvalidOperationException">A Node.js platform instance has already been
     /// loaded in the current process.</exception>
     public NodejsPlatform(
         string libnodePath,
-        string[]? args = null,
-        string[]? execArgs = null)
+        string[]? args = null)
     {
         if (string.IsNullOrEmpty(libnodePath)) throw new ArgumentNullException(nameof(libnodePath));
 
@@ -46,7 +44,7 @@ public NodejsPlatform(
         nint libnodeHandle = NativeLibrary.Load(libnodePath);
         Runtime = new NodejsRuntime(libnodeHandle);
 
-        Runtime.CreatePlatform(args, execArgs, (error) => Console.WriteLine(error), out _platform)
+        Runtime.CreatePlatform(args, (error) => Console.WriteLine(error), out _platform)
             .ThrowIfFailed();
         Current = this;
     }

src/NodeApi/Runtime/NodejsRuntime.Embedding.cs

@@ -11,13 +11,11 @@ public unsafe partial class NodejsRuntime
 {
 #pragma warning disable IDE1006 // Naming: missing prefix '_'
 
-    private delegate* unmanaged[Cdecl]<
-        int, nint, int, nint, napi_error_message_handler, nint, napi_status>
+    private delegate* unmanaged[Cdecl]<int, nint, napi_error_message_handler, nint, napi_status>
         napi_create_platform;
 
     public override napi_status CreatePlatform(
         string[]? args,
-        string[]? execArgs,
         Action<string>? errorHandler,
         out napi_platform result)
     {
@@ -29,7 +27,6 @@ public override napi_status CreatePlatform(
             });
 
         nint args_ptr = StringsToHGlobalUtf8(args, out int args_count);
-        nint exec_args_ptr = StringsToHGlobalUtf8(execArgs, out int exec_args_count);
 
         try
         {
@@ -39,23 +36,20 @@ public override napi_status CreatePlatform(
                 if (napi_create_platform == null)
                 {
                     napi_create_platform = (delegate* unmanaged[Cdecl]<
-                        int, nint, int, nint, napi_error_message_handler, nint, napi_status>)
+                        int, nint, napi_error_message_handler, nint, napi_status>)
                         Import(nameof(napi_create_platform));
                 }
 
                 return napi_create_platform(
                     args_count,
                     args_ptr,
-                    exec_args_count,
-                    exec_args_ptr,
                     native_error_handler,
                     (nint)result_ptr);
             }
         }
         finally
         {
             FreeStringsHGlobal(args_ptr, args_count);
-            FreeStringsHGlobal(exec_args_ptr, exec_args_count);
         }
     }
 
@@ -68,13 +62,14 @@ public override napi_status DestroyPlatform(napi_platform platform)
     }
 
     private delegate* unmanaged[Cdecl]<
-        napi_platform, napi_error_message_handler, nint, nint, napi_status>
+        napi_platform, napi_error_message_handler, nint, int, nint, napi_status>
         napi_create_environment;
 
     public override napi_status CreateEnvironment(
         napi_platform platform,
         Action<string>? errorHandler,
         string? mainScript,
+        int apiVersion,
         out napi_env result)
     {
         napi_error_message_handler native_error_handler = errorHandler == null ? default :
@@ -91,7 +86,7 @@ public override napi_status CreateEnvironment(
             fixed (napi_env* result_ptr = &result)
             {
                 return Import(ref napi_create_environment)(
-                    platform, native_error_handler, main_script_ptr, (nint)result_ptr);
+                    platform, native_error_handler, main_script_ptr, apiVersion, (nint)result_ptr);
             }
         }
         finally

src/NodeApi/Runtime/TracingJSRuntime.cs

@@ -2648,15 +2648,14 @@ public override napi_status GetNodeVersion(napi_env env, out napi_node_version r
     #region Embedding
 
     public override napi_status CreatePlatform(
-        string[]? args, string[]? execArgs, Action<string>? errorHandler, out napi_platform result)
+        string[]? args, Action<string>? errorHandler, out napi_platform result)
     {
         napi_platform resultValue = default;
         napi_status status = TraceCall(
             [
                 $"[{string.Join(", ", args ?? [])}]",
-                $"[{string.Join(", ", execArgs ?? [])}]",
             ],
-            () => (_runtime.CreatePlatform(args, execArgs, errorHandler, out resultValue),
+            () => (_runtime.CreatePlatform(args, errorHandler, out resultValue),
                 Format(resultValue)));
         result = resultValue;
         return status;
@@ -2673,12 +2672,14 @@ public override napi_status CreateEnvironment(
         napi_platform platform,
         Action<string>? errorHandler,
         string? mainScript,
+        int apiVersion,
         out napi_env result)
     {
         napi_env resultValue = default;
         napi_status status = TraceCall(
             [Format(platform), Format(mainScript)],
-            () => (_runtime.CreateEnvironment(platform, errorHandler, mainScript, out resultValue),
+            () => (_runtime.CreateEnvironment(
+                platform, errorHandler, mainScript, apiVersion, out resultValue),
                 Format(resultValue)));
         result = resultValue;
         return status;

test/NodejsEmbeddingTests.cs

@@ -46,6 +46,14 @@ public void NodejsStart()
         Assert.Equal(0, nodejs.ExitCode);
     }
 
+    [SkippableFact]
+    public void NodejsRestart()
+    {
+        // Create and destory a Node.js environment twice, using the same platform instance.
+        NodejsStart();
+        NodejsStart();
+    }
+
     [SkippableFact]
     public void NodejsCallFunction()
     {