fix: clean up some node-api details #24178
Conversation
devsnek
force-pushed
the
x/napi-cleanup
branch
2 times, most recently
from
610cecf
to
024efcf
Compare
| &mut env.scope(), | ||
| env_ptr, | ||
| Some(name), | ||
| constructor.unwrap(), |
There was a problem hiding this comment.
That looks like a potential panic :D Maybe do let ... else { return napi_invalid_arg; }?
There was a problem hiding this comment.
its not because it comes after check_arg!(env, constructor). but yea this gets confusing and wastes performance if rust doesn't optimize it out... i'd like if check_arg!() unwrapped it automatically but there's a bit of effort needed there. maybe another followup...
| let local_getter: v8::Local<v8::Value> = if let Some(getter) = | ||
| property.getter | ||
| { | ||
| create_function(&mut env.scope(), env_ptr, None, getter, property.data) |
There was a problem hiding this comment.
If create_function accepted &mut Env instead of env_ptr I think Rust would complain here about double mutable borrow. Should we change create_function signature?
There was a problem hiding this comment.
i would enjoy to be wrong about this but the reason i did this is because i'm not sure its possible to encode the correct borrow in rust's type system for a function which accepts &mut Env and uses env.scope() to create and return a local handle.
There was a problem hiding this comment.
Yeah, that might get tricky. That's fine then
- fix a few napi_* types - clean up env hooks - implement blocking queue in tsfn - reduce transmutes - use new `DataView::new` api from rusty_v8
- fix a few napi_* types - clean up env hooks - implement blocking queue in tsfn - reduce transmutes - use new `DataView::new` api from rusty_v8
DataView::newapi from rusty_v8