ValueTask<TResult> should be converted to promise when obtained from a clr object method call

[REscobar]

Version/platform/runtime used

Version: 4.4.1
Platform: Windows
Runtime: .net 8

Describe the bug

When calling a method from a clr object reference that returns a ValueTask<TResult> and the experimental feature ExperimentalFeature.TaskInterop is enabled the returned task is not converted to a promise due to a missing check in

jint/Jint/Runtime/Interop/DefaultObjectConverter.cs

Lines 83 to 89 in ddf187c

	#if NETSTANDARD2_1_OR_GREATER || NETCOREAPP
	if (value is ValueTask valueTask)
	{
	result = JsValue.ConvertAwaitableToPromise(engine, valueTask);
	return result is not null;
	}
	#endif

This should also check for ValueTask<TResult> with something similar to (taken from JsValue.ConvertAwaitableToPromise):

                var asTask = value.GetType().GetMethod(nameof(ValueTask<object>.AsTask));
                if (asTask is not null)
                {
                    result = JsValue.ConvertAwaitableToPromise(engine, value);
                    return result is not null;
                }

To Reproduce

    [Fact]
    public void ShouldIterateOverAsyncEnumeratorConvertedToPromiseInJS() //Executing this test will actually achieve the not so small feat of crashing the runtime
    {
        Engine engine = new(options => options.ExperimentalFeatures = ExperimentalFeature.TaskInterop);
        engine.SetValue("asyncTestClass", new AsyncTestClass());
        var result = engine.Evaluate(
        @"async function doIteration() {
            const iter = asyncTestClass.AsyncEnumerable().GetAsyncEnumerator();
            var lastVal = 'notfromenum';
            while(await iter.MoveNextAsync()) {
                lastVal = iter.Current;
            }
            return lastVal;
        };
        doIteration()");
        result = result.UnwrapIfPromise();
        Assert.Equal(AsyncTestClass.TestString, result);
    }

    [Fact]
    public void ShouldReturnedCompletedValueTaskConvertedToPromiseInJS() //This will fail because the returned value is not a promise
    {
        Engine engine = new(options => options.ExperimentalFeatures = ExperimentalFeature.TaskInterop);
        engine.SetValue("asyncTestClass", new AsyncTestClass());
        var result = engine.Evaluate("asyncTestClass.ReturnCompletedValueTask()");
        result = result.UnwrapIfPromise();
        Assert.Equal(AsyncTestClass.TestString, result);
    }

    [Fact]
    public void ShouldAwaitUnwrapPromiseWithCustomTimeoutValueTask() //This will fail because the returned value is not a promise
    {
        Engine engine = new(options => { options.ExperimentalFeatures = ExperimentalFeature.TaskInterop; options.Constraints.PromiseTimeout = TimeSpan.FromMilliseconds(500); });
        engine.SetValue("asyncTestClass", new AsyncTestClass());
        engine.Execute(""" 
        async function test() {
            return await asyncTestClass.ReturnDelayedValueTaskAsync();
        }
        """);
        var result = engine.Invoke("test").UnwrapIfPromise();
        Assert.Equal(AsyncTestClass.TestString, result);
    }

internal class AsyncTestClass
{
    public static readonly string TestString = "Hello World";

    public string StringToAppend { get; set; } = string.Empty;
    public ValueTask<string> ReturnCompletedValueTask()
    {
#if NET9_0_OR_GREATER
        return ValueTask.FromResult(TestString);
#else
        return new ValueTask<string>(TestString);
#endif
    }

    public async ValueTask<string> ReturnDelayedValueTaskAsync()
    {
        await Task.Delay(100).ConfigureAwait(false);

        return TestString;
    }

    public async ValueTask AddToStringDelayedValueTaskAsync(string appendWith)
    {
        await Task.Delay(100).ConfigureAwait(false);

        StringToAppend += appendWith;
    }

#pragma warning disable CS1998 // Async method lacks 'await' operators and will run synchronously
    public async IAsyncEnumerable<string> NonYieldingAsyncEnumerable()
    {
        yield return TestString;
    }

    public async IAsyncEnumerable<string> AsyncEnumerable()
    {
        await Task.Delay(100);
        yield return StringToAppend;
        yield return TestString;
    }
#pragma warning restore CS1998 // Async method lacks 'await' operators and will run synchronously
}
    

Expected behavior

The ValueTask<TResult> are treated the same as Task, Task<TResult and ValueTask, and when the experimental feature ExperimentalFeature.TaskInterop is enabled the returned task is converted to a promise.

Additional context

• I already looked for similar issues and didn't find any
• I believe a workaround can be implemented by registering an IObjectConverter that handles ValueTask<TResult>
• I can provide a pull requests with tests to fix this

Edit

This IObjectConverter is able to workaround the issue:

using System.Diagnostics.CodeAnalysis;
using System.Threading.Tasks;
using Jint;
using Jint.Native;
using Jint.Runtime;
using Jint.Runtime.Interop;

internal class ValueTaskConverter : IObjectConverter
{
    public bool TryConvert(Engine engine, object value, [NotNullWhen(true)] out JsValue? result)
    {
        var asTask = value.GetType().GetMethod(nameof(ValueTask<object>.AsTask));
        if (asTask is not null)
        {
            result = ConvertTaskToPromise(engine, (Task)asTask.Invoke(value, parameters: null)!);
            return result is not null;
        }

        result = null;
        return false;
    }

    internal static JsValue ConvertTaskToPromise(Engine engine, Task task)
    {
        var (promise, resolve, reject) = engine.Advanced.RegisterPromise();
        task = task.ContinueWith(
            continuationAction =>
            {
                if (continuationAction.IsFaulted)
                {
                    reject(JsValue.FromObject(engine, continuationAction.Exception));
                }
                else if (continuationAction.IsCanceled)
                {
                    reject(JsValue.FromObject(engine, new ExecutionCanceledException()));
                }
                else
                {
                    // Special case: Marshal `async Task` as undefined, as this is `Task<VoidTaskResult>` at runtime
                    // See https://github.com/sebastienros/jint/pull/1567#issuecomment-1681987702
                    if (Task.CompletedTask.Equals(continuationAction))
                    {
                        resolve(JsValue.FromObject(engine, JsValue.Undefined));
                        return;
                    }

                    var result = continuationAction.GetType().GetProperty(nameof(Task<object>.Result));
                    if (result is not null)
                    {
                        resolve(JsValue.FromObject(engine, result.GetValue(continuationAction)));
                    }
                    else
                    {
                        resolve(JsValue.FromObject(engine, JsValue.Undefined));
                    }
                }
            },
            // Ensure continuation is completed before unwrapping Promise
            continuationOptions: TaskContinuationOptions.AttachedToParent | TaskContinuationOptions.ExecuteSynchronously);

        return promise;
    }
}

(taken from):

jint/Jint/Native/JsValue.cs

Line 146 in ddf187c

internal static JsValue ConvertTaskToPromise(Engine engine, Task task)