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.

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

Cleanup deprecated jsg/promise methods #2048

Merged
merged 3 commits into from
Apr 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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
Loading