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

feat(neon): Add neon::types::extract::Boxed for JsBox conversions #1073

Merged
merged 1 commit into from
Oct 7, 2024

Conversation

kjvalencik
Copy link
Member

@kjvalencik kjvalencik commented Sep 27, 2024

JsBox<T> can be extract in an exported function.

#[neon::export]
fn example(boxed: Handle<JsBox<String>>) {}

However, this only works for synchronous functions. If you need boxed data in an async function or task, it requires first cloning it in a sync function, which can be verbose.

#[neon::export]
fn example<'cx>(cx: &mut FunctionContext<'cx>, s: Handle<'cx, JsBox<String>>) -> Handle<'cx, JsPromise> {
    let s = String::clone(&s);
    
    cx.task(move || s).promise(|mut cx, s| cx.string(s))
}

However, we can trivially implement TryFromJs where T: Clone. It must be clone because JsBox only allows immutable references. This allows a much more ergonomic version.

#[neon::export(task)]
fn example(Boxed(s): Boxed<String>) -> String {
    s
}

Similarly, we can implement TryIntoJs for Boxed by calling cx.boxed(..). The TryIntoJs implementation does not require Clone because we already have an owned copy.

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.

This is very nice, and the name is perfect, given the symmetry with cx.boxed(). :shipit:

@kjvalencik kjvalencik merged commit 205ed38 into main Oct 7, 2024
9 checks passed
@kjvalencik kjvalencik deleted the kv/extract-box branch October 7, 2024 17:40
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