Complete Promise implementation

[miroslav22]

Branch that provides a complete implementation of a Promise as per the specs.

Only missing method is Promise.allSettled() which it seems isn't widely supported in browsers at the moment.

The Promises are implemented, built upon and wrap .net Tasks

If the return value of a CLR method returns a Task or Task then this is automatically converted to a Promise in the run time. This allows you to now use fully asynchronous javascript code in Jint.

GetCompletionValueAsync() has been added to Engine which awaits and unwraps the execution completion value if it's a Promise type. If the completion value isn't a Promise then it's just returned as per GetCompletionValue().

Due to the nature of the Promise continuations I've had to add thread locking to the engine execution, as otherwise it'd be possible for multiple threads to execute code concurrently within the same engine.

I've added a fairly comprehensive set of tests which covers all the specs as far as I can tell.

This opens the door for async/await keywords to be supported (as they essentially use promises 'under the hood'). Is it possible for the Esprima library to be updated to support parsing of async/await keywords and methods? If so I'm more than happy to add support for async/await in Jint too.

[lahma]

Great work and quite consistent with existing Jint code base! If you would like to walk the extra mile, you could add a test class to Jint.Tests.Test262 project as like this:

using Xunit;

namespace Jint.Tests.Test262
{
    public class PromiseTests : Test262Test
    {
        [Theory(DisplayName = "built-ins\\Promise")]
        [MemberData(nameof(SourceFiles), "built-ins\\Promise", false)]
        [MemberData(nameof(SourceFiles), "built-ins\\Promise", true, Skip = "Skipped")]
        protected void Promise(SourceFile sourceFile)
        {
            RunTestInternal(sourceFile);
        }
    }
}

Which will show you some spec violations that are still present. If you find that they cannot work yet due to preconditions not implemented yet or such, you can ignore them in skipped.json which is part of the same project.

[sebastienros]

Oh boy! we got a new champion 🎉

[lahma]

This PR can also contain the missing tick mark in README.md in repository root for promises 🥇

[miroslav22]

Currently reworking code so official ecma tests pass. Most are now passing.

[alexmg]

This really looks awesome @miroslav22!

I'm very excited to see this PR as I am looking for a solution to running JavaScript with Jint asynchronously from Microsoft Orleans grains (actors).

I noticed that the implementation is using Task.Run which for Orleans means the current task scheduler would not be respected. Orleans has its own task scheduler implementation called ActivationTaskScheduler that implements turn based concurrency for grains.

It would be awesome if this started the Task using Task.Factory.StartNew which does respect the current task scheduler. This would mean that code called from JavaScript could invoke other grains. Using Task.Factory.StartNew would also allow for the user to provide the desired ITaskScheduler, which would be an added bonus that would make this awesome feature even more flexible.

https://dotnet.github.io/orleans/Documentation/grains/external_tasks_and_grains.html

I thought I would put this out there now in hope that this scenario could be accommodated.

[lahma]

Hey @miroslav22 anything we could help you with? It's understandable that this is time consuming we'd gladly help if there's anything to debug etc 🙂

[miroslav22]

Hi, it's okay thanks it's nearly finished. I've temporarily been moved to another project at work which is the reason for the delay. I should be back on it shortly as we need promises/async for a mobile app we're developing.

[viceice]

Any news on this? We need promises too and i don't want to add my own implementation.

Is there anything i can help with?

[lahma]

@miroslav22 I hope you don't mind that I force-pushed a rebased version, there were some fundamental changes to properties recently so I thought it would be easier for me to do the rebase. I also enabled the ECMA tests and checked them for easy win. I probably can try to figure out some fixes required to spec requirements.

[ChrML]

What's left before this can be merged?

[tottaka]

How can I use this?

[lahma]

Improvements and fixes as pull requests are welcome. This feature is still incomplete as the test suite isn't yet passing.

[miroslav22]

Apologies there's been no updates on this. Unfortunately it's unlikely I will do any further work on this as I've now developed my own wrapper around Chromium's V8 engine.

As far as I can remember there wasn't a great deal that needed finishing to get it to comply with the spec.

[lahma]

Thanks @miroslav22 for the update and the great groundwork you made! As said, there's still some rough edges we need to smoothen to get official test suite pass which is the usual requirement before PR can be merged. All contributions are welcome!

[twop]

First of all, thanks for working on this and for Jint! It is an amazing tool <3

Sorry for pinging with no immediate contribution, but I wad wondering what is the roadmap for Promise support?

I'm embedding Jint into a Mac/iOS app and I will definitely need async iteration (ideally for await (let value of range)), I haven't flushed out API I will provide yet but any form of async support will probably do.

Thus, I'm trying to decide should I wait for this PR or try to explore other options for embedding? Any context would be super appreciated!

[cesarchefinho]

May be take a look at https://chromium.googlesource.com/v8/v8/+/3.29.45/src/promise.js?autodive=0%2F

This is the source code of promise in chromium

[sfmskywalker]

Same as @twop - eagerly waiting for async/await support.

[miroslav22]

If there's no-one else able to finish this then I'll try and get around to it. I cant promise anything though and I won't have time for at least a few months I'm afraid.

As I remember it did essentially work. It's just it didn't comply with the spec on several fringe-use cases (most of which I can't believe are ever used).

If all you want to do is say embed an async network call then as far as I know it converted the Task <-> Promise back and forth between Net <-> Javascript just fine and awaiting it was truly asynchronous.

So what I'm saying is it's probably usable as it stands for most cases if you merged my changes into your own fork of the project.

[sfmskywalker]

If all you want to do is say embed an async network call then as far as I know it converted the Task <-> Promise back and forth between Net <-> Javascript just fine and awaiting it was truly asynchronous.

That's pretty much what I'm looking for.

So what I'm saying is it's probably usable as it stands for most cases if you merged my changes into your own fork of the project.

@sebastienros if you merge it today than Jint will be better than it was yesterday, no? ;)

@miroslav22 Thanks for all of your hard work that was put into this!

[twop]

I will try to rebase the PR and see what can I do. I'm using F# and haven't touched C# in a while, so no promises ^_^

[sebastienros]

From my memory, I did some work on this PR in order to follow the specification more closely. If we don't it will bite us either in test or when people start using libraries that expect the official behavior. Meaning using the correct Constructor object, the correct method names, the correct usage on bindings ... So any work should follow the spec as much as possible. I can't find any personal branch so all the work might already be in this PR. It's not impossible that we need to create a new PR since there were many internal changes since then. As long as the final merge contains @miroslav22 as a co-author I am totally fine with creating a new one and copying the code over.

[twop]

Working on reviving the PR. So far I created a new branch from dev and just ported changes to it.

@lahma I'm fixing unit tests that are not passing after your commit 5b4d551 (tweaks)

Specifically I'm really curious about your thinking process for this code:

 private PromiseInstance NewPromiseCapability(JsValue c)
        {
            var constructor = AssertConstructor(_engine, c);

            var executor = new PromiseInstance(_engine);
            var test = Construct(constructor, new JsValue[] { executor });
            var promiseCapability = executor;
            return promiseCapability;
        }

with it the Promise.resolve throws an exception that says roughly PromiseInstance is not not callable (same for reject etc)

I tried to change the code a little bit and all unit tests are passing:

 private PromiseInstance NewPromiseCapability(JsValue c)
        {
            var constructor = AssertConstructor(_engine, c);

            var executor = new PromiseInstance(_engine);
           //  var test = Construct(constructor, new JsValue[] { executor }); <-------
            var promiseCapability = executor;
            return promiseCapability;
        }

Could you help me understand why NewPromiseCapability is needed?

Thanks!

[twop]

Ok, I started running Ecma scripts tests and got issues... Working through them right now. It seems that they are related to passing custom Contructor function, that in turn seems to be related to https://tc39.es/ecma262/#sec-promisereaction-records

@lahma I understand terminology now but still not sure I understand the meaning of the spec ^_^

so WIP

[lahma]

Great, I have only faint memories but usually I've tried to align naming and steps (methods etc) like in spec in order ensure the delicate parts (JS weirdness) is properly taken care of. @twop this is a good series for understanding how to read the spec.

[lahma]

Closing this this as Promise has landed in dev branch, thanks everyone involved for helping out!

[sfmskywalker]

Great stuff, thanks everyone for the hard work done, really fantastic!