Skip to content

Commit

Permalink
Cleanup deprecated jsg/promise methods (#2048)
Browse files Browse the repository at this point in the history
  • Loading branch information
jasnell authored and garrettgu10 committed May 13, 2024
1 parent 7873b24 commit 6946a92
Show file tree
Hide file tree
Showing 5 changed files with 51 additions and 136 deletions.
14 changes: 7 additions & 7 deletions src/workerd/api/streams/queue.c++
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,12 @@ namespace workerd::api {

#pragma region ValueQueue::ReadRequest

void ValueQueue::ReadRequest::resolveAsDone(jsg::Lock&) {
resolver.resolve(ReadResult { .done = true });
void ValueQueue::ReadRequest::resolveAsDone(jsg::Lock& js) {
resolver.resolve(js, ReadResult { .done = true });
}

void ValueQueue::ReadRequest::resolve(jsg::Lock&, jsg::Value value) {
resolver.resolve(ReadResult { .value = kj::mv(value), .done = false });
void ValueQueue::ReadRequest::resolve(jsg::Lock& js, jsg::Value value) {
resolver.resolve(js, ReadResult { .value = kj::mv(value), .done = false });
}

void ValueQueue::ReadRequest::reject(jsg::Lock& js, jsg::Value& value) {
Expand Down Expand Up @@ -265,15 +265,15 @@ void ByteQueue::ReadRequest::resolveAsDone(jsg::Lock& js) {
// There's been at least some data written, we need to respond but not
// set done to true since that's what the streams spec requires.
pullInto.store.trim(pullInto.store.size() - pullInto.filled);
resolver.resolve(ReadResult {
resolver.resolve(js, ReadResult {
.value = js.v8Ref(pullInto.store.createHandle(js)),
.done = false
});
} else {
// Otherwise, we set the length to zero
pullInto.store.trim(pullInto.store.size());
KJ_ASSERT(pullInto.store.size() == 0);
resolver.resolve(ReadResult {
resolver.resolve(js, ReadResult {
.value = js.v8Ref(pullInto.store.createHandle(js)),
.done = true
});
Expand All @@ -283,7 +283,7 @@ void ByteQueue::ReadRequest::resolveAsDone(jsg::Lock& js) {

void ByteQueue::ReadRequest::resolve(jsg::Lock& js) {
pullInto.store.trim(pullInto.store.size() - pullInto.filled);
resolver.resolve(ReadResult {
resolver.resolve(js, ReadResult {
.value = js.v8Ref(pullInto.store.createHandle(js)),
.done = false
});
Expand Down
4 changes: 2 additions & 2 deletions src/workerd/io/io-context.h
Original file line number Diff line number Diff line change
Expand Up @@ -1063,7 +1063,7 @@ jsg::PromiseForResult<Func, T, true> IoContext::awaitIoImpl(
// trace as long as we construct the Error inside of a promise continuation, so we use
// a `.then()` below that acutally extracts the kj::Exception and turn it into a JS
// Error.
resolver.resolve(kj::mv(e));
resolver.resolve(js, kj::mv(e));
} else {
try {
js.tryCatch([&]() {
Expand Down Expand Up @@ -1285,7 +1285,7 @@ jsg::PromiseForResult<Func, void, true> IoContext::blockConcurrencyWhile(
return run([value = kj::mv(value),resolver = kj::mv(resolver),
maybeAsyncContext = kj::mv(maybeAsyncContext)](Worker::Lock& lock) mutable {
jsg::AsyncContextFrame::Scope scope(lock, maybeAsyncContext);
resolver.resolve(kj::mv(value));
resolver.resolve(lock, kj::mv(value));
}, kj::mv(inputLock));
}, [cs = kj::mv(cs2)]
(kj::Exception&& e) mutable {
Expand Down
8 changes: 5 additions & 3 deletions src/workerd/jsg/jsg.h
Original file line number Diff line number Diff line change
Expand Up @@ -1476,13 +1476,13 @@ class Constructor;
// never resolve). This is a convenience so that method implementations that return promises do
// not need to carefully capture a reference to `JSG_THIS`.
//
// You can construct an immediate Promise value using jsg::resolvedPromise() and
// jsg::rejectedPromise() (see below).
// You can construct an immediate Promise value using js.resolvedPromise() and
// js.rejectedPromise() (see below).
//
// You can also create a promise/resolver pair:
//
// auto [promise, resolver] = js.newPromiseAndResolver<kj::String>();
// resolver.resolve(kj::str(foo));
// resolver.resolve(js, kj::str(foo));
//
// The Promise exposes a markAsHandled() API that will mark JavaScript Promise such that rejections
// are not reported to the isolate's unhandled rejection tracking mechanisms. Importantly, any then
Expand Down Expand Up @@ -1557,6 +1557,8 @@ using ReturnType = typename ReturnType_<Func, T, passLock>::Type;
// Convenience template to produce a promise for the result of calling a function with the given
// parameter type. This wraps the function's result type in `jsg::Promise` UNLESS the function
// already returns a `jsg::Promise`, in which case the type is unchanged.
// TODO(cleanup): The passLock = false variation is currently only used for js.evalNow().
// It would be nice to refactor that a bit so we can clean up this template and simplify.
template <typename Func, typename Param, bool passLock>
using PromiseForResult = Promise<RemovePromise<ReturnType<Func, Param, passLock>>>;

Expand Down
10 changes: 5 additions & 5 deletions src/workerd/jsg/promise-test.c++
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ struct PromiseContext: public jsg::Object, public jsg::ContextGlobal {
.then(js, [](jsg::Lock& js, int i) { return kj::str(i); });
}

void resolvePromise(int i) {
KJ_ASSERT_NONNULL(resolver).resolve(kj::mv(i));
void resolvePromise(Lock& js, int i) {
KJ_ASSERT_NONNULL(resolver).resolve(js, kj::mv(i));
}

void setResult(jsg::Lock& js, Promise<kj::String> promise) {
Expand All @@ -48,8 +48,8 @@ struct PromiseContext: public jsg::Object, public jsg::ContextGlobal {
});
}

Promise<kj::String> makeRejected(jsg::Value exception, v8::Isolate* isolate) {
return rejectedPromise<kj::String>(isolate, kj::mv(exception));
Promise<kj::String> makeRejected(jsg::Lock& js, jsg::Value exception) {
return js.rejectedPromise<kj::String>(kj::mv(exception));
}

Promise<kj::String> makeRejectedKj(jsg::Lock& js) {
Expand All @@ -59,7 +59,7 @@ struct PromiseContext: public jsg::Object, public jsg::ContextGlobal {
void testConsumeResolved(jsg::Lock& js) {
auto [ promise, resolver ] = js.newPromiseAndResolver<int>();
KJ_EXPECT(promise.tryConsumeResolved(js) == kj::none);
resolver.resolve(123);
resolver.resolve(js, 123);
KJ_EXPECT(KJ_ASSERT_NONNULL(promise.tryConsumeResolved(js)) == 123);

KJ_EXPECT(js.rejectedPromise<kj::String>(v8StrIntern(js.v8Isolate, "foo"))
Expand Down
Loading

0 comments on commit 6946a92

Please sign in to comment.