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

Add support for passing tuples between Node and Rust #142

Merged
merged 9 commits into from
Mar 30, 2021
Merged

Add support for passing tuples between Node and Rust #142

merged 9 commits into from
Mar 30, 2021

Conversation

nicholastmosher
Copy link
Contributor

@nicholastmosher nicholastmosher commented Mar 30, 2021

Subtask of infinyon/fluvio#847

Previously, we were unable to pass tuples from Node to Rust and vice-versa. Something like this would fail to compile:

#[node_bindgen]
fn take_tuple(value: (String, i32)) -> (String, i32) { ... }

This PR adds support for making this possible. The Node.js representation of tuples is a heterogeneous list. An interface definition matching the function above would look like this:

interface TupleTester {
    takeTuple(value: [string, number]): [string, number]
}

The following edits were made to make this possible:

  • Implement JSValue<'_> for tuples. This has been implemented for 1Tuples through 8Tuples via a macro
  • Implement TryIntoJs for tuples. Implemented for 1Tuples through 8Tuples via a macro
  • Update the node_bindgen proc macro and code generator to recognize arguments of form value: (A, B) which previously did not work
  • Added an example test for tuple functionality

Makefile Show resolved Hide resolved
Copy link
Contributor

@simlay simlay left a comment

Choose a reason for hiding this comment

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

LGTM.

Copy link
Collaborator

@sehz sehz left a comment

Choose a reason for hiding this comment

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

Looks good. Just need to update version

Cargo.toml Outdated Show resolved Hide resolved
nj-core/Cargo.toml Outdated Show resolved Hide resolved
nj-derive/Cargo.toml Outdated Show resolved Hide resolved
@sehz sehz merged commit e7ae379 into master Mar 30, 2021
@sehz sehz deleted the tuples branch March 30, 2021 19:24
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.

3 participants