refactor(ext/node): remove NodeEnv trait (denoland#18986)

cli/build.rs

@@ -362,7 +362,7 @@ fn create_cli_snapshot(snapshot_path: PathBuf) {
     deno_http::deno_http::init_ops(),
     deno_io::deno_io::init_ops(Default::default()),
     deno_fs::deno_fs::init_ops::<_, PermissionsContainer>(false, StdFs),
-    deno_node::deno_node::init_ops::<deno_runtime::RuntimeNodeEnv>(
+    deno_node::deno_node::init_ops::<PermissionsContainer>(
       None,
       Some(Arc::new(deno_node::RealFs)),
     ),

ext/node/lib.rs

@@ -39,10 +39,6 @@ pub use resolution::NodeResolution;
 pub use resolution::NodeResolutionMode;
 pub use resolution::NodeResolver;
 
-pub trait NodeEnv {
-  type P: NodePermissions;
-}
-
 pub trait NodePermissions {
   fn check_read(&self, path: &Path) -> Result<(), AnyError>;
 }
@@ -192,7 +188,7 @@ fn op_node_build_os() -> String {
 
 deno_core::extension!(deno_node,
   deps = [ deno_io, deno_fs ],
-  parameters = [Env: NodeEnv],
+  parameters = [P: NodePermissions],
   ops = [
     ops::crypto::op_node_create_decipheriv,
     ops::crypto::op_node_cipheriv_encrypt,
@@ -271,26 +267,26 @@ deno_core::extension!(deno_node,
     ops::zlib::op_zlib_reset,
     op_node_build_os,
     ops::require::op_require_init_paths,
-    ops::require::op_require_node_module_paths<Env>,
+    ops::require::op_require_node_module_paths<P>,
     ops::require::op_require_proxy_path,
     ops::require::op_require_is_deno_dir_package,
     ops::require::op_require_resolve_deno_dir,
     ops::require::op_require_is_request_relative,
     ops::require::op_require_resolve_lookup_paths,
-    ops::require::op_require_try_self_parent_path<Env>,
-    ops::require::op_require_try_self<Env>,
-    ops::require::op_require_real_path<Env>,
+    ops::require::op_require_try_self_parent_path<P>,
+    ops::require::op_require_try_self<P>,
+    ops::require::op_require_real_path<P>,
     ops::require::op_require_path_is_absolute,
     ops::require::op_require_path_dirname,
-    ops::require::op_require_stat<Env>,
+    ops::require::op_require_stat<P>,
     ops::require::op_require_path_resolve,
     ops::require::op_require_path_basename,
-    ops::require::op_require_read_file<Env>,
+    ops::require::op_require_read_file<P>,
     ops::require::op_require_as_file_path,
-    ops::require::op_require_resolve_exports<Env>,
-    ops::require::op_require_read_closest_package_json<Env>,
-    ops::require::op_require_read_package_scope<Env>,
-    ops::require::op_require_package_imports_resolve<Env>,
+    ops::require::op_require_resolve_exports<P>,
+    ops::require::op_require_read_closest_package_json<P>,
+    ops::require::op_require_read_package_scope<P>,
+    ops::require::op_require_package_imports_resolve<P>,
     ops::require::op_require_break_on_next_statement,
   ],
   esm_entry_point = "ext:deno_node/02_init.js",

ext/node/ops/require.rs

@@ -16,7 +16,6 @@ use std::rc::Rc;
 use std::sync::Arc;
 
 use crate::resolution;
-use crate::NodeEnv;
 use crate::NodeFs;
 use crate::NodeModuleKind;
 use crate::NodePermissions;
@@ -88,12 +87,12 @@ pub fn op_require_init_paths() -> Vec<String> {
 }
 
 #[op]
-pub fn op_require_node_module_paths<Env>(
+pub fn op_require_node_module_paths<P>(
   state: &mut OpState,
   from: String,
 ) -> Result<Vec<String>, AnyError>
 where
-  Env: NodeEnv + 'static,
+  P: NodePermissions + 'static,
 {
   let fs = state.borrow::<Arc<dyn NodeFs>>();
   // Guarantee that "from" is absolute.
@@ -105,7 +104,7 @@ where
   .to_file_path()
   .unwrap();
 
-  ensure_read_permission::<Env::P>(state, &from)?;
+  ensure_read_permission::<P>(state, &from)?;
 
   if cfg!(windows) {
     // return root node_modules when path is 'D:\\'.
@@ -255,15 +254,15 @@ fn op_require_path_is_absolute(p: String) -> bool {
 }
 
 #[op]
-fn op_require_stat<Env>(
+fn op_require_stat<P>(
   state: &mut OpState,
   path: String,
 ) -> Result<i32, AnyError>
 where
-  Env: NodeEnv + 'static,
+  P: NodePermissions + 'static,
 {
   let path = PathBuf::from(path);
-  ensure_read_permission::<Env::P>(state, &path)?;
+  ensure_read_permission::<P>(state, &path)?;
   let fs = state.borrow::<Arc<dyn NodeFs>>();
   if let Ok(metadata) = fs.metadata(&path) {
     if metadata.is_file {
@@ -277,15 +276,15 @@ where
 }
 
 #[op]
-fn op_require_real_path<Env>(
+fn op_require_real_path<P>(
   state: &mut OpState,
   request: String,
 ) -> Result<String, AnyError>
 where
-  Env: NodeEnv + 'static,
+  P: NodePermissions + 'static,
 {
   let path = PathBuf::from(request);
-  ensure_read_permission::<Env::P>(state, &path)?;
+  ensure_read_permission::<P>(state, &path)?;
   let fs = state.borrow::<Arc<dyn NodeFs>>();
   let canonicalized_path = deno_core::strip_unc_prefix(fs.canonicalize(&path)?);
   Ok(canonicalized_path.to_string_lossy().to_string())
@@ -328,14 +327,14 @@ fn op_require_path_basename(request: String) -> Result<String, AnyError> {
 }
 
 #[op]
-fn op_require_try_self_parent_path<Env>(
+fn op_require_try_self_parent_path<P>(
   state: &mut OpState,
   has_parent: bool,
   maybe_parent_filename: Option<String>,
   maybe_parent_id: Option<String>,
 ) -> Result<Option<String>, AnyError>
 where
-  Env: NodeEnv + 'static,
+  P: NodePermissions + 'static,
 {
   if !has_parent {
     return Ok(None);
@@ -349,7 +348,7 @@ where
     if parent_id == "<repl>" || parent_id == "internal/preload" {
       let fs = state.borrow::<Arc<dyn NodeFs>>();
       if let Ok(cwd) = fs.current_dir() {
-        ensure_read_permission::<Env::P>(state, &cwd)?;
+        ensure_read_permission::<P>(state, &cwd)?;
         return Ok(Some(cwd.to_string_lossy().to_string()));
       }
     }
@@ -358,20 +357,20 @@ where
 }
 
 #[op]
-fn op_require_try_self<Env>(
+fn op_require_try_self<P>(
   state: &mut OpState,
   parent_path: Option<String>,
   request: String,
 ) -> Result<Option<String>, AnyError>
 where
-  Env: NodeEnv + 'static,
+  P: NodePermissions + 'static,
 {
   if parent_path.is_none() {
     return Ok(None);
   }
 
   let node_resolver = state.borrow::<Rc<NodeResolver>>();
-  let permissions = state.borrow::<Env::P>();
+  let permissions = state.borrow::<P>();
   let pkg = node_resolver
     .get_package_scope_config(
       &Url::from_file_path(parent_path.unwrap()).unwrap(),
@@ -421,15 +420,15 @@ where
 }
 
 #[op]
-fn op_require_read_file<Env>(
+fn op_require_read_file<P>(
   state: &mut OpState,
   file_path: String,
 ) -> Result<String, AnyError>
 where
-  Env: NodeEnv + 'static,
+  P: NodePermissions + 'static,
 {
   let file_path = PathBuf::from(file_path);
-  ensure_read_permission::<Env::P>(state, &file_path)?;
+  ensure_read_permission::<P>(state, &file_path)?;
   let fs = state.borrow::<Arc<dyn NodeFs>>();
   Ok(fs.read_to_string(&file_path)?)
 }
@@ -446,7 +445,7 @@ pub fn op_require_as_file_path(file_or_url: String) -> String {
 }
 
 #[op]
-fn op_require_resolve_exports<Env>(
+fn op_require_resolve_exports<P>(
   state: &mut OpState,
   uses_local_node_modules_dir: bool,
   modules_path: String,
@@ -456,12 +455,12 @@ fn op_require_resolve_exports<Env>(
   parent_path: String,
 ) -> Result<Option<String>, AnyError>
 where
-  Env: NodeEnv + 'static,
+  P: NodePermissions + 'static,
 {
   let fs = state.borrow::<Arc<dyn NodeFs>>();
   let npm_resolver = state.borrow::<Arc<dyn NpmResolver>>();
   let node_resolver = state.borrow::<Rc<NodeResolver>>();
-  let permissions = state.borrow::<Env::P>();
+  let permissions = state.borrow::<P>();
 
   let pkg_path = if npm_resolver
     .in_npm_package_at_path(&PathBuf::from(&modules_path))
@@ -502,54 +501,54 @@ where
 }
 
 #[op]
-fn op_require_read_closest_package_json<Env>(
+fn op_require_read_closest_package_json<P>(
   state: &mut OpState,
   filename: String,
 ) -> Result<PackageJson, AnyError>
 where
-  Env: NodeEnv + 'static,
+  P: NodePermissions + 'static,
 {
-  ensure_read_permission::<Env::P>(
+  ensure_read_permission::<P>(
     state,
     PathBuf::from(&filename).parent().unwrap(),
   )?;
   let node_resolver = state.borrow::<Rc<NodeResolver>>();
-  let permissions = state.borrow::<Env::P>();
+  let permissions = state.borrow::<P>();
   node_resolver.get_closest_package_json(
     &Url::from_file_path(filename).unwrap(),
     permissions,
   )
 }
 
 #[op]
-fn op_require_read_package_scope<Env>(
+fn op_require_read_package_scope<P>(
   state: &mut OpState,
   package_json_path: String,
 ) -> Option<PackageJson>
 where
-  Env: NodeEnv + 'static,
+  P: NodePermissions + 'static,
 {
   let node_resolver = state.borrow::<Rc<NodeResolver>>();
-  let permissions = state.borrow::<Env::P>();
+  let permissions = state.borrow::<P>();
   let package_json_path = PathBuf::from(package_json_path);
   node_resolver
     .load_package_json(permissions, package_json_path)
     .ok()
 }
 
 #[op]
-fn op_require_package_imports_resolve<Env>(
+fn op_require_package_imports_resolve<P>(
   state: &mut OpState,
   parent_filename: String,
   request: String,
 ) -> Result<Option<String>, AnyError>
 where
-  Env: NodeEnv + 'static,
+  P: NodePermissions + 'static,
 {
   let parent_path = PathBuf::from(&parent_filename);
-  ensure_read_permission::<Env::P>(state, &parent_path)?;
+  ensure_read_permission::<P>(state, &parent_path)?;
   let node_resolver = state.borrow::<Rc<NodeResolver>>();
-  let permissions = state.borrow::<Env::P>();
+  let permissions = state.borrow::<P>();
   let pkg = node_resolver
     .load_package_json(permissions, parent_path.join("package.json"))?;
 

runtime/build.rs

@@ -215,12 +215,6 @@ mod startup_snapshot {
     }
   }
 
-  struct SnapshotNodeEnv;
-
-  impl deno_node::NodeEnv for SnapshotNodeEnv {
-    type P = Permissions;
-  }
-
   deno_core::extension!(runtime,
     deps = [
       deno_webidl,
@@ -320,7 +314,7 @@ mod startup_snapshot {
       runtime::init_ops_and_esm(),
       // FIXME(bartlomieju): these extensions are specified last, because they
       // depend on `runtime`, even though it should be other way around
-      deno_node::deno_node::init_ops_and_esm::<SnapshotNodeEnv>(None, None),
+      deno_node::deno_node::init_ops_and_esm::<Permissions>(None, None),
       #[cfg(not(feature = "snapshot_from_snapshot"))]
       runtime_main::init_ops_and_esm(),
     ];

runtime/lib.rs

@@ -35,8 +35,3 @@ pub mod worker;
 
 mod worker_bootstrap;
 pub use worker_bootstrap::BootstrapOptions;
-
-pub struct RuntimeNodeEnv;
-impl deno_node::NodeEnv for RuntimeNodeEnv {
-  type P = permissions::PermissionsContainer;
-}

runtime/web_worker.rs

@@ -442,7 +442,7 @@ impl WebWorker {
       deno_http::deno_http::init_ops(),
       deno_io::deno_io::init_ops(Some(options.stdio)),
       deno_fs::deno_fs::init_ops::<_, PermissionsContainer>(unstable, StdFs),
-      deno_node::deno_node::init_ops::<crate::RuntimeNodeEnv>(
+      deno_node::deno_node::init_ops::<PermissionsContainer>(
         options.npm_resolver,
         options.node_fs,
       ),

runtime/worker.rs

@@ -267,7 +267,7 @@ impl MainWorker {
       deno_http::deno_http::init_ops(),
       deno_io::deno_io::init_ops(Some(options.stdio)),
       deno_fs::deno_fs::init_ops::<_, PermissionsContainer>(unstable, StdFs),
-      deno_node::deno_node::init_ops::<crate::RuntimeNodeEnv>(
+      deno_node::deno_node::init_ops::<PermissionsContainer>(
         options.npm_resolver,
         options.node_fs,
       ),