Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Promise.withResolvers() returned object had resolve and reject functions swapped #1983

Merged
merged 2 commits into from
Oct 24, 2024
Merged

Promise.withResolvers() returned object had resolve and reject functions swapped #1983

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a49ed9e
Promise.withResolvers() returned object had resolve and reject swapped
tomatosalat0 Oct 23, 2024
d1bf1ff
Refactoring
tomatosalat0 Oct 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 52 additions & 0 deletions Jint.Tests/Runtime/PromiseTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -497,4 +497,56 @@ public void ManualPromise_HasCorrectStackTrace()

Assert.Equal("at <anonymous>:1:56", logMessage?.Trim());
}

[Fact]
public void WithResolvers_calling_resolve_resolves_promise()
{
// Arrange
using var engine = new Engine();
List<string> logMessages = new();
engine.SetValue("log", logMessages.Add);

// Act
engine.Execute("""
const p = Promise.withResolvers();
const next = p.promise
.then(() => log('resolved'))
.catch(() => log('rejected'));

log('start');
p.resolve();
log('end');
""");
engine.RunAvailableContinuations();

// Assert
List<string> expected = new() { "start", "end", "resolved" };
Assert.Equal(expected, logMessages);
}

[Fact]
public void WithResolvers_calling_reject_rejects_promise()
{
// Arrange
using var engine = new Engine();
List<string> logMessages = new();
engine.SetValue("log", logMessages.Add);

// Act
engine.Execute("""
const p = Promise.withResolvers();
const next = p.promise
.then(() => log('resolved'))
.catch(() => log('rejected'));

log('start');
p.reject();
log('end');
""");
engine.RunAvailableContinuations();

// Assert
List<string> expected = new() { "start", "end", "rejected" };
Assert.Equal(expected, logMessages);
}
}
81 changes: 38 additions & 43 deletions Jint/Native/Promise/PromiseConstructor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,8 @@ internal sealed record PromiseCapability(
JsValue PromiseInstance,
ICallable Resolve,
ICallable Reject,
JsValue RejectObj,
JsValue ResolveObj
);
JsValue ResolveObj,
JsValue RejectObj);

internal sealed class PromiseConstructor : Constructor
{
Expand Down Expand Up @@ -139,11 +138,11 @@ private JsValue PromiseResolve(JsValue thisObject, JsValue x)
}
}

var (instance, resolve, _, _, _) = NewPromiseCapability(_engine, thisObject);
var capability = NewPromiseCapability(_engine, thisObject);

resolve.Call(Undefined, new[] { x });
capability.Resolve.Call(Undefined, new[] { x });

return instance;
return capability.PromiseInstance;
}

/// <summary>
Expand All @@ -163,11 +162,11 @@ private JsValue Reject(JsValue thisObject, JsValue[] arguments)

var r = arguments.At(0);

var (instance, _, reject, _, _) = NewPromiseCapability(_engine, thisObject);
var capability = NewPromiseCapability(_engine, thisObject);

reject.Call(Undefined, new[] { r });
capability.Reject.Call(Undefined, new[] { r });

return instance;
return capability.PromiseInstance;
}

/// <summary>
Expand Down Expand Up @@ -255,8 +254,6 @@ private JsValue All(JsValue thisObject, JsValue[] arguments)
if (!TryGetPromiseCapabilityAndIterator(thisObject, arguments, "Promise.all", out var capability, out var promiseResolve, out var iterator))
return capability.PromiseInstance;

var (resultingPromise, resolve, reject, _, rejectObj) = capability;

var results = new List<JsValue>();
bool doneIterating = false;

Expand All @@ -269,7 +266,7 @@ void ResolveIfFinished()
if (results.TrueForAll(static x => x is not null) && doneIterating)
{
var array = _realm.Intrinsics.Array.ConstructFast(results);
resolve.Call(Undefined, new JsValue[] { array });
capability.Resolve.Call(Undefined, new JsValue[] { array });
}
}

Expand All @@ -296,8 +293,8 @@ void ResolveIfFinished()
}
catch (JavaScriptException e)
{
reject.Call(Undefined, new[] { e.Error });
return resultingPromise;
capability.Reject.Call(Undefined, new[] { e.Error });
return capability.PromiseInstance;
}

// note that null here is important
Expand Down Expand Up @@ -325,7 +322,7 @@ void ResolveIfFinished()
return Undefined;
}, 1, PropertyFlag.Configurable);

thenFunc.Call(item, new JsValue[] { onSuccess, rejectObj });
thenFunc.Call(item, new JsValue[] { onSuccess, capability.RejectObj });
}
else
{
Expand All @@ -338,11 +335,11 @@ void ResolveIfFinished()
catch (JavaScriptException e)
{
iterator.Close(CompletionType.Throw);
reject.Call(Undefined, new[] { e.Error });
return resultingPromise;
capability.Reject.Call(Undefined, new[] { e.Error });
return capability.PromiseInstance;
}

return resultingPromise;
return capability.PromiseInstance;
}

// https://tc39.es/ecma262/#sec-promise.allsettled
Expand All @@ -351,8 +348,6 @@ private JsValue AllSettled(JsValue thisObject, JsValue[] arguments)
if (!TryGetPromiseCapabilityAndIterator(thisObject, arguments, "Promise.allSettled", out var capability, out var promiseResolve, out var iterator))
return capability.PromiseInstance;

var (resultingPromise, resolve, reject, _, rejectObj) = capability;

var results = new List<JsValue>();
bool doneIterating = false;

Expand All @@ -365,7 +360,7 @@ void ResolveIfFinished()
if (results.TrueForAll(static x => x is not null) && doneIterating)
{
var array = _realm.Intrinsics.Array.ConstructFast(results);
resolve.Call(Undefined, new JsValue[] { array });
capability.Resolve.Call(Undefined, new JsValue[] { array });
}
}

Expand All @@ -392,8 +387,8 @@ void ResolveIfFinished()
}
catch (JavaScriptException e)
{
reject.Call(Undefined, new[] { e.Error });
return resultingPromise;
capability.Reject.Call(Undefined, new[] { e.Error });
return capability.PromiseInstance;
}

// note that null here is important
Expand Down Expand Up @@ -456,11 +451,11 @@ void ResolveIfFinished()
catch (JavaScriptException e)
{
iterator.Close(CompletionType.Throw);
reject.Call(Undefined, new[] { e.Error });
return resultingPromise;
capability.Reject.Call(Undefined, new[] { e.Error });
return capability.PromiseInstance;
}

return resultingPromise;
return capability.PromiseInstance;
}

// https://tc39.es/ecma262/#sec-promise.any
Expand All @@ -471,8 +466,6 @@ private JsValue Any(JsValue thisObject, JsValue[] arguments)
return capability.PromiseInstance;
}

var (resultingPromise, resolve, reject, resolveObj, _) = capability;

var errors = new List<JsValue>();
var doneIterating = false;

Expand All @@ -487,7 +480,7 @@ void RejectIfAllRejected()
{
var array = _realm.Intrinsics.Array.ConstructFast(errors);

reject.Call(Undefined, new JsValue[] { Construct(_realm.Intrinsics.AggregateError, new JsValue[] { array }) });
capability.Reject.Call(Undefined, new JsValue[] { Construct(_realm.Intrinsics.AggregateError, new JsValue[] { array }) });
}
}

Expand Down Expand Up @@ -547,7 +540,7 @@ void RejectIfAllRejected()
return Undefined;
}, 1, PropertyFlag.Configurable);

thenFunc.Call(item, new JsValue[] { resolveObj, onError });
thenFunc.Call(item, new JsValue[] { capability.ResolveObj, onError });
}
else
{
Expand All @@ -560,11 +553,11 @@ void RejectIfAllRejected()
catch (JavaScriptException e)
{
iterator.Close(CompletionType.Throw);
reject.Call(Undefined, new[] { e.Error });
return resultingPromise;
capability.Reject.Call(Undefined, new[] { e.Error });
return capability.PromiseInstance;
}

return resultingPromise;
return capability.PromiseInstance;
}

// https://tc39.es/ecma262/#sec-promise.race
Expand All @@ -573,9 +566,6 @@ private JsValue Race(JsValue thisObject, JsValue[] arguments)
if (!TryGetPromiseCapabilityAndIterator(thisObject, arguments, "Promise.race", out var capability, out var promiseResolve, out var iterator))
return capability.PromiseInstance;

var (resultingPromise, resolve, reject, _, rejectObj) = capability;


// 7. Let result be PerformPromiseRace(iteratorRecord, C, promiseCapability, promiseResolve).
// https://tc39.es/ecma262/#sec-performpromiserace
try
Expand All @@ -594,16 +584,16 @@ private JsValue Race(JsValue thisObject, JsValue[] arguments)
}
catch (JavaScriptException e)
{
reject.Call(Undefined, new[] { e.Error });
return resultingPromise;
capability.Reject.Call(Undefined, new[] { e.Error });
return capability.PromiseInstance;
}

// h. Let nextPromise be ? Call(promiseResolve, constructor, « nextValue »).
var nextPromise = promiseResolve.Call(thisObject, new JsValue[] { nextValue });

// i. Perform ? Invoke(nextPromise, "then", « resultCapability.[[Resolve]], resultCapability.[[Reject]] »).

_engine.Invoke(nextPromise, "then", new[] { (JsValue) resolve, rejectObj });
_engine.Invoke(nextPromise, "then", new[] { (JsValue) capability.Resolve, capability.RejectObj });
} while (true);
}
catch (JavaScriptException e)
Expand All @@ -612,13 +602,13 @@ private JsValue Race(JsValue thisObject, JsValue[] arguments)
// a. If iteratorRecord.[[Done]] is false, set result to IteratorClose(iteratorRecord, result).
// b. IfAbruptRejectPromise(result, promiseCapability).
iterator.Close(CompletionType.Throw);
reject.Call(Undefined, new[] { e.Error });
return resultingPromise;
capability.Reject.Call(Undefined, new[] { e.Error });
return capability.PromiseInstance;
}

// 9. Return Completion(result).
// Note that PerformPromiseRace returns a Promise instance in success case
return resultingPromise;
return capability.PromiseInstance;
}


Expand Down Expand Up @@ -715,6 +705,11 @@ JsValue Executor(JsValue thisObject, JsValue[] arguments)
ExceptionHelper.ThrowTypeError(engine.Realm, "reject is not a function");
}

return new PromiseCapability(instance, resolve, reject, resolveArg, rejectArg);
return new PromiseCapability(
PromiseInstance: instance,
Resolve: resolve,
Reject: reject,
RejectObj: rejectArg,
ResolveObj: resolveArg);
}
}