Revert "chore: move all node-api impl to ext (#24662)" (#24680)

This reverts commit d00fbd7025.

Reverting because, it caused a failure during v1.45.3 publish:

https://github.com/denoland/deno/actions/runs/10048730693/job/27773718095

Cargo.lock

@@ -1164,6 +1164,7 @@ dependencies = [
  "lsp-types",
  "memmem",
  "monch",
+ "napi_sym",
  "nix 0.26.2",
  "notify",
  "once_cell",
@@ -1687,10 +1688,7 @@ version = "0.91.0"
 dependencies = [
  "deno_core",
  "deno_permissions",
- "libc",
  "libloading 0.7.4",
- "log",
- "napi_sym",
 ]
 
 [[package]]

Cargo.toml

@@ -5,6 +5,7 @@ resolver = "2"
 members = [
   "bench_util",
   "cli",
+  "cli/napi/sym",
   "ext/broadcast_channel",
   "ext/cache",
   "ext/canvas",
@@ -18,7 +19,6 @@ members = [
   "ext/io",
   "ext/kv",
   "ext/napi",
-  "ext/napi/sym",
   "ext/net",
   "ext/node",
   "ext/url",
@@ -52,7 +52,7 @@ deno_media_type = { version = "0.1.4", features = ["module_specifier"] }
 deno_permissions = { version = "0.21.0", path = "./runtime/permissions" }
 deno_runtime = { version = "0.169.0", path = "./runtime" }
 deno_terminal = "0.2.0"
-napi_sym = { version = "0.91.0", path = "./ext/napi/sym" }
+napi_sym = { version = "0.91.0", path = "./cli/napi/sym" }
 test_util = { package = "test_server", path = "./tests/util/server" }
 
 denokv_proto = "0.8.1"

cli/Cargo.toml

@@ -78,6 +78,7 @@ deno_semver = "=0.5.7"
 deno_task_shell = "=0.17.0"
 deno_terminal.workspace = true
 eszip = "=0.72.2"
+napi_sym.workspace = true
 
 async-trait.workspace = true
 base32.workspace = true

cli/build.rs

@@ -396,7 +396,7 @@ fn main() {
     }
     os => format!("generated_symbol_exports_list_{}.def", os),
   };
-  let symbols_path = std::path::Path::new("../ext/napi")
+  let symbols_path = std::path::Path::new("napi")
     .join(symbols_file_name)
     .canonicalize()
     .expect(

cli/main.rs

@@ -15,6 +15,7 @@ mod js;
 mod jsr;
 mod lsp;
 mod module_loader;
+mod napi;
 mod node;
 mod npm;
 mod ops;

cli/mainrt.rs

@@ -15,6 +15,7 @@ mod errors;
 mod file_fetcher;
 mod http_util;
 mod js;
+mod napi;
 mod node;
 mod npm;
 mod resolver;

cli/napi/README.md

@@ -0,0 +1,114 @@
+# napi
+
+This directory contains source for Deno's Node-API implementation. It depends on
+`napi_sym` and `deno_napi`.
+
+Files are generally organized the same as in Node.js's implementation to ease in
+ensuring compatibility.
+
+## Adding a new function
+
+Add the symbol name to
+[`cli/napi_sym/symbol_exports.json`](../napi_sym/symbol_exports.json).
+
+```diff
+{
+  "symbols": [
+    ...
+    "napi_get_undefined",
+-   "napi_get_null"
++   "napi_get_null",
++   "napi_get_boolean"
+  ]
+}
+```
+
+Determine where to place the implementation. `napi_get_boolean` is related to JS
+values so we will place it in `js_native_api.rs`. If something is not clear,
+just create a new file module.
+
+See [`napi_sym`](../napi_sym/) for writing the implementation:
+
+```rust
+#[napi_sym::napi_sym]
+pub fn napi_get_boolean(
+  env: *mut Env,
+  value: bool,
+  result: *mut napi_value,
+) -> Result {
+  // ...
+  Ok(())
+}
+```
+
+Update the generated symbol lists using the script:
+
+```
+deno run --allow-write tools/napi/generate_symbols_lists.js
+```
+
+Add a test in [`/tests/napi`](../../tests/napi/). You can also refer to Node.js
+test suite for Node-API.
+
+```js
+// tests/napi/boolean_test.js
+import { assertEquals, loadTestLibrary } from "./common.js";
+const lib = loadTestLibrary();
+Deno.test("napi get boolean", function () {
+  assertEquals(lib.test_get_boolean(true), true);
+  assertEquals(lib.test_get_boolean(false), false);
+});
+```
+
+```rust
+// tests/napi/src/boolean.rs
+
+use napi_sys::Status::napi_ok;
+use napi_sys::ValueType::napi_boolean;
+use napi_sys::*;
+
+extern "C" fn test_boolean(
+  env: napi_env,
+  info: napi_callback_info,
+) -> napi_value {
+  let (args, argc, _) = crate::get_callback_info!(env, info, 1);
+  assert_eq!(argc, 1);
+
+  let mut ty = -1;
+  assert!(unsafe { napi_typeof(env, args[0], &mut ty) } == napi_ok);
+  assert_eq!(ty, napi_boolean);
+
+  // Use napi_get_boolean here...
+
+  value
+}
+
+pub fn init(env: napi_env, exports: napi_value) {
+  let properties = &[crate::new_property!(env, "test_boolean\0", test_boolean)];
+
+  unsafe {
+    napi_define_properties(env, exports, properties.len(), properties.as_ptr())
+  };
+}
+```
+
+```diff
+// tests/napi/src/lib.rs
+
++ mod boolean;
+
+...
+
+#[no_mangle]
+unsafe extern "C" fn napi_register_module_v1(
+  env: napi_env,
+  exports: napi_value,
+) -> napi_value {
+  ...
++ boolean::init(env, exports);
+
+  exports
+}
+```
+
+Run the test using `cargo test -p tests/napi`.

cli/napi/js_native_api.rs

@@ -5,7 +5,7 @@
 
 const NAPI_VERSION: u32 = 9;
 
-use crate::*;
+use deno_runtime::deno_napi::*;
 use libc::INT_MAX;
 
 use super::util::check_new_from_utf8;
@@ -17,9 +17,9 @@ use super::util::napi_set_last_error;
 use super::util::v8_name_from_property_descriptor;
 use crate::check_arg;
 use crate::check_env;
-use crate::function::create_function;
-use crate::function::create_function_template;
-use crate::function::CallbackInfo;
+use deno_runtime::deno_napi::function::create_function;
+use deno_runtime::deno_napi::function::create_function_template;
+use deno_runtime::deno_napi::function::CallbackInfo;
 use napi_sym::napi_sym;
 use std::ptr::NonNull;
 
@@ -1644,7 +1644,7 @@ fn napi_get_cb_info(
     check_arg!(env, argc);
     let argc = unsafe { *argc as usize };
     for i in 0..argc {
-      let arg = args.get(i as _);
+      let mut arg = args.get(i as _);
       unsafe {
         *argv.add(i) = arg.into();
       }

cli/napi/mod.rs

@@ -0,0 +1,20 @@
+// Copyright 2018-2024 the Deno authors. All rights reserved. MIT license.
+
+#![allow(unused_mut)]
+#![allow(non_camel_case_types)]
+#![allow(clippy::undocumented_unsafe_blocks)]
+
+//! Symbols to be exported are now defined in this JSON file.
+//! The `#[napi_sym]` macro checks for missing entries and panics.
+//!
+//! `./tools/napi/generate_symbols_list.js` is used to generate the LINK `cli/exports.def` on Windows,
+//! which is also checked into git.
+//!
+//! To add a new napi function:
+//! 1. Place `#[napi_sym]` on top of your implementation.
+//! 2. Add the function's identifier to this JSON list.
+//! 3. Finally, run `tools/napi/generate_symbols_list.js` to update `cli/napi/generated_symbol_exports_list_*.def`.
+
+pub mod js_native_api;
+pub mod node_api;
+pub mod util;

cli/napi/node_api.rs

@@ -9,10 +9,10 @@ use super::util::napi_set_last_error;
 use super::util::SendPtr;
 use crate::check_arg;
 use crate::check_env;
-use crate::*;
 use deno_core::parking_lot::Condvar;
 use deno_core::parking_lot::Mutex;
 use deno_core::V8CrossThreadTaskSpawner;
+use deno_runtime::deno_napi::*;
 use napi_sym::napi_sym;
 use std::sync::atomic::AtomicBool;
 use std::sync::atomic::AtomicU8;
@@ -892,7 +892,7 @@ fn napi_create_threadsafe_function(
   };
   let resource_name = resource_name.to_rust_string_lossy(&mut env.scope());
 
-  let tsfn = Box::new(TsFn {
+  let mut tsfn = Box::new(TsFn {
     env,
     func,
     max_queue_size,

cli/napi/sym/lib.rs

@@ -20,7 +20,7 @@ pub fn napi_sym(_attr: TokenStream, item: TokenStream) -> TokenStream {
   let name = &func.sig.ident;
   assert!(
     exports.symbols.contains(&name.to_string()),
-    "ext/napi/sym/symbol_exports.json is out of sync!"
+    "cli/napi/sym/symbol_exports.json is out of sync!"
   );
 
   TokenStream::from(quote! {

cli/napi/util.rs

@@ -1,13 +1,13 @@
 // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license.
-use crate::*;
+use deno_runtime::deno_napi::*;
 use libc::INT_MAX;
 
 #[repr(transparent)]
-pub(crate) struct SendPtr<T>(pub(crate) *const T);
+pub struct SendPtr<T>(pub *const T);
 
 impl<T> SendPtr<T> {
   // silly function to get around `clippy::redundant_locals`
-  pub(crate) fn take(self) -> *const T {
+  pub fn take(self) -> *const T {
     self.0
   }
 }
@@ -15,9 +15,7 @@ impl<T> SendPtr<T> {
 unsafe impl<T> Send for SendPtr<T> {}
 unsafe impl<T> Sync for SendPtr<T> {}
 
-pub(crate) fn get_array_buffer_ptr(
-  ab: v8::Local<v8::ArrayBuffer>,
-) -> *mut c_void {
+pub fn get_array_buffer_ptr(ab: v8::Local<v8::ArrayBuffer>) -> *mut c_void {
   match ab.data() {
     Some(p) => p.as_ptr(),
     None => std::ptr::null_mut(),
@@ -39,7 +37,7 @@ impl Drop for BufferFinalizer {
   }
 }
 
-pub(crate) extern "C" fn backing_store_deleter_callback(
+pub extern "C" fn backing_store_deleter_callback(
   data: *mut c_void,
   _byte_length: usize,
   deleter_data: *mut c_void,
@@ -52,7 +50,7 @@ pub(crate) extern "C" fn backing_store_deleter_callback(
   drop(finalizer);
 }
 
-pub(crate) fn make_external_backing_store(
+pub fn make_external_backing_store(
   env: *mut Env,
   data: *mut c_void,
   byte_length: usize,
@@ -92,7 +90,9 @@ macro_rules! check_env {
 macro_rules! return_error_status_if_false {
   ($env: expr, $condition: expr, $status: ident) => {
     if !$condition {
-      return Err($crate::util::napi_set_last_error($env, $status).into());
+      return Err(
+        $crate::napi::util::napi_set_last_error($env, $status).into(),
+      );
     }
   };
 }
@@ -101,7 +101,7 @@ macro_rules! return_error_status_if_false {
 macro_rules! return_status_if_false {
   ($env: expr, $condition: expr, $status: ident) => {
     if !$condition {
-      return $crate::util::napi_set_last_error($env, $status);
+      return $crate::napi::util::napi_set_last_error($env, $status);
     }
   };
 }
@@ -222,7 +222,7 @@ macro_rules! check_arg {
   ($env: expr, $ptr: expr) => {
     $crate::return_status_if_false!(
       $env,
-      !$crate::util::Nullable::is_null(&$ptr),
+      !$crate::napi::util::Nullable::is_null(&$ptr),
       napi_invalid_arg
     );
   };
@@ -233,7 +233,6 @@ macro_rules! napi_wrap {
   ( $( # $attr:tt )* fn $name:ident $( < $( $x:lifetime ),* > )? ( $env:ident : & $( $lt:lifetime )? mut Env $( , $ident:ident : $ty:ty )* $(,)? ) -> napi_status $body:block ) => {
     $( # $attr )*
     #[no_mangle]
-    #[allow(clippy::missing_safety_doc)]
     pub unsafe extern "C" fn $name $( < $( $x ),* > )? ( env_ptr : *mut Env , $( $ident : $ty ),* ) -> napi_status {
       let env: & $( $lt )? mut Env = $crate::check_env!(env_ptr);
 
@@ -241,7 +240,7 @@ macro_rules! napi_wrap {
         return napi_pending_exception;
       }
 
-      $crate::util::napi_clear_last_error(env);
+      $crate::napi::util::napi_clear_last_error(env);
 
       let scope_env = unsafe { &mut *env_ptr };
       let scope = &mut scope_env.scope();
@@ -260,11 +259,11 @@ macro_rules! napi_wrap {
         let env = unsafe { &mut *env_ptr };
         let global = v8::Global::new(env.isolate(), exception);
         env.last_exception = Some(global);
-        return $crate::util::napi_set_last_error(env_ptr, napi_pending_exception);
+        return $crate::napi::util::napi_set_last_error(env_ptr, napi_pending_exception);
       }
 
       if result != napi_ok {
-        return $crate::util::napi_set_last_error(env_ptr, result);
+        return $crate::napi::util::napi_set_last_error(env_ptr, result);
       }
 
       return result;
@@ -274,7 +273,6 @@ macro_rules! napi_wrap {
   ( $( # $attr:tt )* fn $name:ident $( < $( $x:lifetime ),* > )? ( $( $ident:ident : $ty:ty ),* $(,)? ) -> napi_status $body:block ) => {
     $( # $attr )*
     #[no_mangle]
-    #[allow(clippy::missing_safety_doc)]
     pub unsafe extern "C" fn $name $( < $( $x ),* > )? ( $( $ident : $ty ),* ) -> napi_status {
       #[inline(always)]
       fn inner $( < $( $x ),* > )? ( $( $ident : $ty ),* ) -> napi_status $body

ext/napi/Cargo.toml

@@ -16,7 +16,4 @@ path = "lib.rs"
 [dependencies]
 deno_core.workspace = true
 deno_permissions.workspace = true
-libc.workspace = true
 libloading = { version = "0.7" }
-log.workspace = true
-napi_sym.workspace = true

ext/napi/README.md

@@ -1,114 +0,0 @@
-# napi
-
-This directory contains source for Deno's Node-API implementation. It depends on
-`napi_sym` and `deno_napi`.
-
-Files are generally organized the same as in Node.js's implementation to ease in
-ensuring compatibility.
-
-## Adding a new function
-
-Add the symbol name to
-[`cli/napi_sym/symbol_exports.json`](../napi_sym/symbol_exports.json).
-
-```diff
-{
-  "symbols": [
-    ...
-    "napi_get_undefined",
--   "napi_get_null"
-+   "napi_get_null",
-+   "napi_get_boolean"
-  ]
-}
-```
-
-Determine where to place the implementation. `napi_get_boolean` is related to JS
-values so we will place it in `js_native_api.rs`. If something is not clear,
-just create a new file module.
-
-See [`napi_sym`](../napi_sym/) for writing the implementation:
-
-```rust
-#[napi_sym::napi_sym]
-pub fn napi_get_boolean(
-  env: *mut Env,
-  value: bool,
-  result: *mut napi_value,
-) -> Result {
-  // ...
-  Ok(())
-}
-```
-
-Update the generated symbol lists using the script:
-
-```
-deno run --allow-write tools/napi/generate_symbols_lists.js
-```
-
-Add a test in [`/tests/napi`](../../tests/napi/). You can also refer to Node.js
-test suite for Node-API.
-
-```js
-// tests/napi/boolean_test.js
-import { assertEquals, loadTestLibrary } from "./common.js";
-const lib = loadTestLibrary();
-Deno.test("napi get boolean", function () {
-  assertEquals(lib.test_get_boolean(true), true);
-  assertEquals(lib.test_get_boolean(false), false);
-});
-```
-
-```rust
-// tests/napi/src/boolean.rs
-
-use napi_sys::Status::napi_ok;
-use napi_sys::ValueType::napi_boolean;
-use napi_sys::*;
-
-extern "C" fn test_boolean(
-  env: napi_env,
-  info: napi_callback_info,
-) -> napi_value {
-  let (args, argc, _) = crate::get_callback_info!(env, info, 1);
-  assert_eq!(argc, 1);
-
-  let mut ty = -1;
-  assert!(unsafe { napi_typeof(env, args[0], &mut ty) } == napi_ok);
-  assert_eq!(ty, napi_boolean);
-
-  // Use napi_get_boolean here...
-
-  value
-}
-
-pub fn init(env: napi_env, exports: napi_value) {
-  let properties = &[crate::new_property!(env, "test_boolean\0", test_boolean)];
-
-  unsafe {
-    napi_define_properties(env, exports, properties.len(), properties.as_ptr())
-  };
-}
-```
-
-```diff
-// tests/napi/src/lib.rs
-
-+ mod boolean;
-
-...
-
-#[no_mangle]
-unsafe extern "C" fn napi_register_module_v1(
-  env: napi_env,
-  exports: napi_value,
-) -> napi_value {
-  ...
-+ boolean::init(env, exports);
-
-  exports
-}
-```
-
-Run the test using `cargo test -p tests/napi`.

ext/napi/lib.rs

@@ -5,21 +5,6 @@
 #![allow(clippy::undocumented_unsafe_blocks)]
 #![deny(clippy::missing_safety_doc)]
 
-//! Symbols to be exported are now defined in this JSON file.
-//! The `#[napi_sym]` macro checks for missing entries and panics.
-//!
-//! `./tools/napi/generate_symbols_list.js` is used to generate the LINK `cli/exports.def` on Windows,
-//! which is also checked into git.
-//!
-//! To add a new napi function:
-//! 1. Place `#[napi_sym]` on top of your implementation.
-//! 2. Add the function's identifier to this JSON list.
-//! 3. Finally, run `tools/napi/generate_symbols_list.js` to update `cli/napi/generated_symbol_exports_list_*.def`.
-
-pub mod js_native_api;
-pub mod node_api;
-pub mod util;
-
 use core::ptr::NonNull;
 use deno_core::error::type_error;
 use deno_core::error::AnyError;

tools/napi/generate_symbols_lists.js

@@ -1,7 +1,7 @@
 #!/usr/bin/env -S deno run --allow-read --allow-write
 // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license.
 
-import exports from "../../ext/napi/sym/symbol_exports.json" with {
+import exports from "../../cli/napi/sym/symbol_exports.json" with {
   type: "json",
 };
 
@@ -17,7 +17,7 @@ const symbolExportLists = {
 
 for await (const [os, def] of Object.entries(symbolExportLists)) {
   const defUrl = new URL(
-    `../../ext/napi/generated_symbol_exports_list_${os}.def`,
+    `../../cli/napi/generated_symbol_exports_list_${os}.def`,
     import.meta.url,
   );
   await Deno.writeTextFile(defUrl.pathname, def, { create: true });