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

Initial implementation for JsPromise and TaskBuilder #789

Merged
merged 1 commit into from
Sep 17, 2021

Conversation

kjvalencik
Copy link
Member

@kjvalencik kjvalencik commented Sep 13, 2021

Implements the JsPromise and TaskBuilder RFC.

Adds two new feature flags:

  • promise-api
  • task-api

Rust Docs

pub fn sum(mut cx: FunctionContext) -> JsResult<JsPromise> {
    let nums = cx.argument::<JsTypedArray<f64>>(0)?.as_slice(&cx).to_vec();
    let promise = cx
        .task(move || nums.into_iter().sum())
        .promise(|cx, n: f64| Ok(cx.number(n)));

    Ok(promise)
}

@kjvalencik kjvalencik marked this pull request as draft September 13, 2021 22:01
@kjvalencik kjvalencik force-pushed the kv/jspromise branch 3 times, most recently from 0803489 to 391567a Compare September 14, 2021 22:12
@kjvalencik kjvalencik marked this pull request as ready for review September 14, 2021 22:12
@kjvalencik kjvalencik requested a review from dherman September 14, 2021 22:43
@neon-bindings neon-bindings deleted a comment from netlify bot Sep 14, 2021
Copy link
Collaborator

@dherman dherman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's remarkable how clean the implementation is -- I bet the hardest part was figuring out the API, and then the primitives match the Node-API design enough that the implementation is pretty clean!

I made a couple tiny suggestions in async_work but I'll leave it to you.

crates/neon-runtime/src/napi/promise.rs Outdated Show resolved Hide resolved
crates/neon-runtime/src/napi/async_work.rs Outdated Show resolved Hide resolved
crates/neon-runtime/src/napi/async_work.rs Outdated Show resolved Hide resolved
@kjvalencik kjvalencik merged commit 8a4ca10 into next/0.10 Sep 17, 2021
@kjvalencik kjvalencik deleted the kv/jspromise branch September 17, 2021 19:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants