Expand API

[JeremyTCD]

• No changes to existing API
• Add members with "params object[] args" for convenience.
  E.g so instead of this:

  InvokeFromStringAsync<int>(javascriptModule, args: new object[] { 3, 5 });

  we can do this:

  InvokeFromStringAsync<int>(javascriptModule, 3, 5);

• Add members with factory methods to atomically check if cached and cache if not cached.
  E.g:

  Task<T> InvokeFromStringAsync<T>(Func<string> moduleFactory, string cacheIdentifier, string exportName, params object[] args);

  Task<T> InvokeFromStreamAsync<T>(Func<Stream> moduleFactory, string cacheIdentifier, string exportName, params object[] args);

  so instead of this:

  (bool success, Result result) = await nodeJSService.TryInvokeFromCacheAsync<Result>(cacheIdentifier, args: new[] { "success" });
  if(!success)
  {
      string moduleString = "module.exports = (callback, message) => callback(null, { resultMessage: message });"; 
      result = await nodeJSService.InvokeFromStringAsync<Result>(moduleString, cacheIdentifier, args: new[] { "success" });
  }

  we can do this:

  Func<string> moduleFactory = () => "Retrieve from file, remote source etc";
  Result result = await nodeJSService.InvokeFromStringAsync<Result>(moduleFactory, cacheIdentifier, "success");

• Add members with no generic parameter for when js doesn't return anything.
  E.g:

  Task InvokeFromFileAsync(string modulePath, string exportName, CancellationToken cancellationToken, params object[] args);

Edit:

No longer adding members with "params object[] args" because if we expose the following methods:

InvokeFromStringAsync<string>(string moduleSource, string newCacheIdentifier, string exportName, params object[] args);
InvokeFromStringAsync<string>(string moduleSource, string newCacheIdentifier, params object[] args);
InvokeFromStringAsync<string>(string moduleSource, params object[] args);

Users might expect:

InvokeFromStringAsync<string>(javascriptModule, arg1);

to invoke InvokeFromStringAsync<string>(string moduleSource, params object[] args), but it actually invokes InvokeFromStringAsync<string>(string moduleSource, string newCacheIdentifier, params object[] args);.

Similarly,

InvokeFromStringAsync<string>(javascriptModule, arg1, arg2);

Invokes InvokeFromStringAsync<string>(string moduleSource, string newCacheIdentifier, string exportName, params object[] args);.

Some framework methods like Activator.CreateInstance have the same issue. However,
with this library, I think there is higher probability of accidents because users might try INodeServices.InvokeAsync syntax like InvokeFromFileAsync<string>(javascriptModule, arg1); and it's likely that users will be sending strings to Node.js.

[DaniilSokolyuk]

@JeremyTCD
LGTM! :)

[Taritsyn]

It would also be nice if the GetFrom* methods appeared, which would allow to work with non-function exports.

For example, now the following code leads to errors:

using System;
using System.Threading.Tasks;

using Jering.Javascript.NodeJS;

namespace TestNodeJs
{
    class Program
    {
        static void Main(string[] args)
        {
            Task<double> task = StaticNodeJSService.InvokeFromStringAsync<double>(
                @"module.exports = { pi: 3.14 };",
                "math.js",
                "pi"
            );
            task.Wait();
            double result = task.Result;

            Console.WriteLine("result = {0}", result);
        }
    }
}

[codecov]

Codecov Report

Merging #57 into master will increase coverage by 0.7%.
The diff coverage is 100%.

@@            Coverage Diff            @@
##           master      #57     +/-   ##
=========================================
+ Coverage   96.81%   97.51%   +0.7%     
=========================================
  Files          18       18             
  Lines         533      564     +31     
=========================================
+ Hits          516      550     +34     
+ Misses         17       14      -3

Impacted Files	Coverage Δ
src/NodeJS/NodeJSServiceCollectionExtensions.cs	100% <100%> (ø)	⬆️
...ntations/OutOfProcess/OutOfProcessNodeJSService.cs	98.06% <100%> (+0.13%)	⬆️
...ementations/OutOfProcess/Http/HttpNodeJSService.cs	96.72% <100%> (+0.11%)	⬆️
src/NodeJS/StaticNodeJSService.cs	100% <100%> (ø)	⬆️
src/NodeJS/InvocationData/InvocationRequest.cs	100% <100%> (ø)	⬆️
src/NodeJS/InvocationData/InvocationException.cs	100% <100%> (ø)	⬆️
...tations/OutOfProcess/Http/HttpNodeJSPoolService.cs	97.14% <100%> (+11.95%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b2666f1...aa5b806. Read the comment docs.

[JeremyTCD]

@DaniilSokolyuk Thanks for taking a look! Should be released by tomorrow.

@Taritsyn I'll keep that suggestion in mind, it's somewhat out of scope for now. Wrt #56, I realized we don't need new overloads, we just need InvokeFrom*<object> to return ExpandoObjects! Had a look at implementing it with this PR, but there were some blockers - System.Text.Json doesn't support ExpandoObjects yet. Also, to serialize a custom undefined in JSON we'll have to use a unique string since JSON has no undefined type. This means custom JSON converters, which means we'll need a flag to "enable undefined" so as to not affect performance of those who don't need undefined. Will handle this in a separate PR since there is quite a bit to do.

[Taritsyn]

@JeremyTCD OK.

[DaniilSokolyuk]

System.Text.Json is extendable
Here example how to deseriailize object like Newtonsoft.Json, we can add to JsonService as default converter

[JeremyTCD]

@DaniilSokolyuk

Yeap it supports custom converters, but it doesn't support serializing to ExpandoObjects yet -
JsonSerializer support for ExpandoObject. So we can't implement this using System.Text.Json right now:

dynamic result = nodeJSService.InvokeFromStringAsync<object>(moduleString);

[JeremyTCD]

🚀 Released v5.2.0.