Skip to content

Commit

Permalink
refactor: Use safe generated version T::into_instance by napi-rs (N…
Browse files Browse the repository at this point in the history
…omicFoundation#907)

Rather than hand-rolling it; The codegen uses exactly the same code but
with the benefit of abstracting away the unsafe details, getting rid of
the only instance of `unsafe` we use.

For reference:
- `ToNapiValue`/`into_instance`:
https://github.com/napi-rs/napi-rs/blob/0550c56fcf8eeec61d2e5abfa00c92a990c34e25/crates/backend/src/codegen/struct.rs#L278-L330
- `JsObject::from_raw_unchecked`/`ClassInstance::as_object`:
https://github.com/napi-rs/napi-rs/blob/0550c56fcf8eeec61d2e5abfa00c92a990c34e25/crates/napi/src/bindgen_runtime/js_values/class.rs#L25

Additionally, I changed `Env` to be passed by value since it's a `Copy`
newtype wrapper around pointer (no double indirection) and makes the
code a bit clearer: no need to additionally dereference with `*`in the
related calls, since napi-rs API accepts it by value.
  • Loading branch information
Xanewok authored Apr 3, 2024
1 parent f45b148 commit a001a46
Show file tree
Hide file tree
Showing 12 changed files with 60 additions and 57 deletions.
31 changes: 16 additions & 15 deletions crates/codegen/parser/runtime/src/napi_interface/cst.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use std::rc::Rc;

use napi::bindgen_prelude::{Env, ToNapiValue};
use napi::{JsObject, NapiValue};
use napi::bindgen_prelude::Env;
use napi::JsObject;
use napi_derive::napi;

use crate::napi_interface::cursor::Cursor;
Expand Down Expand Up @@ -54,7 +54,7 @@ impl RuleNode {
self.0
.children
.iter()
.map(|child| child.to_js(&env))
.map(|child| child.to_js(env))
.collect()
}

Expand Down Expand Up @@ -118,29 +118,30 @@ impl TokenNode {
}
}

pub trait ToJS {
fn to_js(&self, env: &Env) -> JsObject;
pub(crate) trait ToJS {
fn to_js(&self, env: Env) -> JsObject;
}

impl ToJS for Rc<RustRuleNode> {
fn to_js(&self, env: &Env) -> JsObject {
let obj =
unsafe { <RuleNode as ToNapiValue>::to_napi_value(env.raw(), RuleNode(self.clone())) };
unsafe { JsObject::from_raw_unchecked(env.raw(), obj.unwrap()) }
fn to_js(&self, env: Env) -> JsObject {
RuleNode(self.clone())
.into_instance(env)
.expect("Class constructor to be defined by #[napi]")
.as_object(env)
}
}

impl ToJS for Rc<RustTokenNode> {
fn to_js(&self, env: &Env) -> JsObject {
let obj = unsafe {
<TokenNode as ToNapiValue>::to_napi_value(env.raw(), TokenNode(self.clone()))
};
unsafe { JsObject::from_raw_unchecked(env.raw(), obj.unwrap()) }
fn to_js(&self, env: Env) -> JsObject {
TokenNode(self.clone())
.into_instance(env)
.expect("Class constructor to be defined by #[napi]")
.as_object(env)
}
}

impl ToJS for RustNode {
fn to_js(&self, env: &Env) -> JsObject {
fn to_js(&self, env: Env) -> JsObject {
match self {
RustNode::Rule(rust_rule_node) => rust_rule_node.to_js(env),
RustNode::Token(rust_token_node) => rust_token_node.to_js(env),
Expand Down
4 changes: 2 additions & 2 deletions crates/codegen/parser/runtime/src/napi_interface/cursor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ impl Cursor {

#[napi(ts_return_type = "cst.Node", catch_unwind)]
pub fn node(&self, env: Env) -> JsObject {
self.0.node().to_js(&env)
self.0.node().to_js(env)
}

#[napi(getter, ts_return_type = "kinds.NodeLabel", catch_unwind)]
Expand Down Expand Up @@ -82,7 +82,7 @@ impl Cursor {
pub fn ancestors(&self, env: Env) -> Vec<JsObject> {
self.0
.ancestors()
.map(|rust_rule_node| rust_rule_node.to_js(&env))
.map(|rust_rule_node| rust_rule_node.to_js(env))
.collect()
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ impl From<RustParseOutput> for ParseOutput {
impl ParseOutput {
#[napi(ts_return_type = "cst.Node", catch_unwind)]
pub fn tree(&self, env: Env) -> napi::JsObject {
self.0.tree().to_js(&env)
self.0.tree().to_js(env)
}

#[napi(ts_return_type = "Array<parse_error.ParseError>", catch_unwind)]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,7 @@ impl Selector {
}
node if filter(node) => {
self.index += 1;
return Ok(Some(node.to_js(&self.env)));
return Ok(Some(node.to_js(self.env)));
}
_ => {
break;
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit a001a46

Please sign in to comment.