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): Extractors #1024

Merged
merged 2 commits into from
Apr 2, 2024
Merged

feat(neon): Extractors #1024

merged 2 commits into from
Apr 2, 2024

Conversation

kjvalencik
Copy link
Member

@kjvalencik kjvalencik commented Mar 26, 2024

Extractors allow quickly getting JavaScript arguments as Rust values. In addition to immediate ergonomics and performance improvements, extractors set a foundation for future proc macros.

The Neon API surface area increases by two traits:

  • TryFromJs
  • FromArgs

TryFromJs

The TryFromJs trait provides a standardized way of extracting a Rust value from a JsValue. It is initially Sealed while we determine if changes are needed. In a future release, we will likely make it available for users to implement.

FromArgs

The FromArgs trait is an implementation detail and will remain Sealed. It is implemented by a macro for tuples up to size 32.

Extractors

TryFromJs is implemented for many standard library values (f64, String) and wrapper types (Option). It also has blanket implementations for Handle and NeonResult. Additionally, I introduce the concept of an "extractor". The idea is inspired by axum. These extractors allow for specialization where extracted types would otherwise overlap. For example, getting an f64 from a Date.

Most extractors are implemented using the sys crate directly in order to avoid duplicate type checks. In the future, we will likely want to convert existing functions to use the TryFromJs implementations.

Json

As found previously, serialization to JSON is nearly always faster than manually reading JavaScript objects. Working with JSON, thanks to serde, is also much easier. Community crates like neon-serde have demonstrated a high demand for this functionality.

The Json extractor leverages serde and serde_json to convert to a Rust type. The ✨ magic ✨ is that it doesn't require the value to already be stringified on the JavaScript side. It leverage's Neon's new LocalKey functionality for fast access to JSON.stringify. JSON.stringify and serde_json::from_str are implicitly called when extracting.

Extractors allow quickly getting JavaScript arguments as Rust values. In addition to immediate ergonomics and performance improvements, extractors set a foundation for future proc macros.
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.

Mostly a bunch of questions, so maybe it won't require any changes in the end, but I'm marking it as "request changes" just to talk through the comments.

crates/neon/src/context/mod.rs Show resolved Hide resolved
crates/neon/src/lib.rs Outdated Show resolved Hide resolved
crates/neon/src/types_impl/extract/json.rs Show resolved Hide resolved
crates/neon/src/types_impl/extract/json.rs Show resolved Hide resolved
crates/neon/src/types_impl/extract/mod.rs Outdated Show resolved Hide resolved
crates/neon/src/types_impl/extract/mod.rs Show resolved Hide resolved
crates/neon/src/types_impl/extract/mod.rs Show resolved Hide resolved
crates/neon/src/types_impl/extract/types.rs Show resolved Hide resolved
crates/neon/src/types_impl/extract/types.rs Show resolved Hide resolved
test/napi/src/js/extract.rs Show resolved Hide resolved
@kjvalencik kjvalencik requested a review from dherman April 1, 2024 17:32
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.

LGTM -- this one is super exciting, ship it!

@kjvalencik kjvalencik merged commit 2e0a0f1 into main Apr 2, 2024
9 checks passed
@kjvalencik kjvalencik deleted the kv/extract branch April 2, 2024 17:37
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.

None yet

2 participants