refactor(ext/node): change extension type parameter (denoland#18483)

This commit changes the type parameter for "deno_node" extension, from
`P: NodePermission` to `Env: NodeEnv`.

`NodeEnv` is a new trait that has associated type `P: NodePermission`.

This is a stepping stone to support swappable file system for the
extension, that will be added as a second associated type to the 
`NodeEnv` trait.

cli/build.rs

@@ -363,7 +363,7 @@ fn create_cli_snapshot(snapshot_path: PathBuf) {
  deno_io::deno_io::init_ops(Default::default()),
  deno_fs::deno_fs::init_ops::<PermissionsContainer>(false),
  deno_flash::deno_flash::init_ops::<PermissionsContainer>(false), // No --unstable
- deno_node::deno_node::init_ops::<PermissionsContainer>(None),
+ deno_node::deno_node::init_ops::<deno_runtime::RuntimeNodeEnv>(None),
  cli::init_ops_and_esm(), // NOTE: This needs to be init_ops_and_esm!
  ];
 

ext/node/lib.rs

@@ -39,10 +39,21 @@ pub use resolution::NodeModuleKind;
 pub use resolution::NodeResolutionMode;
 pub use resolution::DEFAULT_CONDITIONS;
 
+pub trait NodeEnv {
+ type P: NodePermissions;
+ // TODO(bartlomieju):
+ // type Fs: NodeFs;
+}
+
 pub trait NodePermissions {
  fn check_read(&mut self, path: &Path) -> Result<(), AnyError>;
 }
 
+// TODO(bartlomieju):
+// pub trait NodeFs {
+// fn current_dir() -> Result<PathBuf, AnyError>;
+// }
+
 pub trait RequireNpmResolver {
  fn resolve_package_folder_from_package(
  &self,
@@ -96,7 +107,7 @@ fn op_node_build_os() -> String {
 
 deno_core::extension!(deno_node,
  deps = [ deno_io, deno_fs ],
- parameters = [P: NodePermissions],
+ parameters = [Env: NodeEnv],
  ops = [
  crypto::op_node_create_decipheriv,
  crypto::op_node_cipheriv_encrypt,
@@ -137,26 +148,26 @@ deno_core::extension!(deno_node,
  op_node_build_os,
 
  ops::op_require_init_paths,
- ops::op_require_node_module_paths<P>,
+ ops::op_require_node_module_paths<Env>,
  ops::op_require_proxy_path,
  ops::op_require_is_deno_dir_package,
  ops::op_require_resolve_deno_dir,
  ops::op_require_is_request_relative,
  ops::op_require_resolve_lookup_paths,
- ops::op_require_try_self_parent_path<P>,
- ops::op_require_try_self<P>,
- ops::op_require_real_path<P>,
+ ops::op_require_try_self_parent_path<Env>,
+ ops::op_require_try_self<Env>,
+ ops::op_require_real_path<Env>,
  ops::op_require_path_is_absolute,
  ops::op_require_path_dirname,
- ops::op_require_stat<P>,
+ ops::op_require_stat<Env>,
  ops::op_require_path_resolve,
  ops::op_require_path_basename,
- ops::op_require_read_file<P>,
+ ops::op_require_read_file<Env>,
  ops::op_require_as_file_path,
- ops::op_require_resolve_exports<P>,
- ops::op_require_read_closest_package_json<P>,
- ops::op_require_read_package_scope<P>,
- ops::op_require_package_imports_resolve<P>,
+ ops::op_require_resolve_exports<Env>,
+ ops::op_require_read_closest_package_json<Env>,
+ ops::op_require_read_package_scope<Env>,
+ ops::op_require_package_imports_resolve<Env>,
  ops::op_require_break_on_next_statement,
  ],
  esm_entry_point = "ext:deno_node/02_init.js",

ext/node/ops.rs

@@ -13,6 +13,8 @@ use std::path::Path;
 use std::path::PathBuf;
 use std::rc::Rc;
 
+use crate::NodeEnv;
+
 use super::resolution;
 use super::NodeModuleKind;
 use super::NodePermissions;
@@ -86,12 +88,12 @@ pub fn op_require_init_paths() -> Vec<String> {
 }
 
 #[op]
-pub fn op_require_node_module_paths<P>(
+pub fn op_require_node_module_paths<Env>(
  state: &mut OpState,
  from: String,
 ) -> Result<Vec<String>, AnyError>
 where
- P: NodePermissions + 'static,
+ Env: NodeEnv + 'static,
 {
  // Guarantee that "from" is absolute.
  let from = deno_core::resolve_path(
@@ -102,7 +104,7 @@ where
  .to_file_path()
  .unwrap();
 
- ensure_read_permission::<P>(state, &from)?;
+ ensure_read_permission::<Env::P>(state, &from)?;
 
  if cfg!(windows) {
  // return root node_modules when path is 'D:\\'.
@@ -252,15 +254,15 @@ fn op_require_path_is_absolute(p: String) -> bool {
 }
 
 #[op]
-fn op_require_stat<P>(
+fn op_require_stat<Env>(
  state: &mut OpState,
  path: String,
 ) -> Result<i32, AnyError>
 where
- P: NodePermissions + 'static,
+ Env: NodeEnv + 'static,
 {
  let path = PathBuf::from(path);
- ensure_read_permission::<P>(state, &path)?;
+ ensure_read_permission::<Env::P>(state, &path)?;
  if let Ok(metadata) = std::fs::metadata(&path) {
  if metadata.is_file() {
  return Ok(0);
@@ -273,15 +275,15 @@ where
 }
 
 #[op]
-fn op_require_real_path<P>(
+fn op_require_real_path<Env>(
  state: &mut OpState,
  request: String,
 ) -> Result<String, AnyError>
 where
- P: NodePermissions + 'static,
+ Env: NodeEnv + 'static,
 {
  let path = PathBuf::from(request);
- ensure_read_permission::<P>(state, &path)?;
+ ensure_read_permission::<Env::P>(state, &path)?;
  let mut canonicalized_path = path.canonicalize()?;
  if cfg!(windows) {
  canonicalized_path = PathBuf::from(
@@ -331,14 +333,14 @@ fn op_require_path_basename(request: String) -> Result<String, AnyError> {
 }
 
 #[op]
-fn op_require_try_self_parent_path<P>(
+fn op_require_try_self_parent_path<Env>(
  state: &mut OpState,
  has_parent: bool,
  maybe_parent_filename: Option<String>,
  maybe_parent_id: Option<String>,
 ) -> Result<Option<String>, AnyError>
 where
- P: NodePermissions + 'static,
+ Env: NodeEnv + 'static,
 {
  if !has_parent {
  return Ok(None);
@@ -351,7 +353,7 @@ where
  if let Some(parent_id) = maybe_parent_id {
  if parent_id == "<repl>" || parent_id == "internal/preload" {
  if let Ok(cwd) = std::env::current_dir() {
- ensure_read_permission::<P>(state, &cwd)?;
+ ensure_read_permission::<Env::P>(state, &cwd)?;
  return Ok(Some(cwd.to_string_lossy().to_string()));
  }
  }
@@ -360,20 +362,20 @@ where
 }
 
 #[op]
-fn op_require_try_self<P>(
+fn op_require_try_self<Env>(
  state: &mut OpState,
  parent_path: Option<String>,
  request: String,
 ) -> Result<Option<String>, AnyError>
 where
- P: NodePermissions + 'static,
+ Env: NodeEnv + 'static,
 {
  if parent_path.is_none() {
  return Ok(None);
  }
 
  let resolver = state.borrow::<Rc<dyn RequireNpmResolver>>().clone();
- let permissions = state.borrow_mut::<P>();
+ let permissions = state.borrow_mut::<Env::P>();
  let pkg = resolution::get_package_scope_config(
  &Url::from_file_path(parent_path.unwrap()).unwrap(),
  &*resolver,
@@ -423,15 +425,15 @@ where
 }
 
 #[op]
-fn op_require_read_file<P>(
+fn op_require_read_file<Env>(
  state: &mut OpState,
  file_path: String,
 ) -> Result<String, AnyError>
 where
- P: NodePermissions + 'static,
+ Env: NodeEnv + 'static,
 {
  let file_path = PathBuf::from(file_path);
- ensure_read_permission::<P>(state, &file_path)?;
+ ensure_read_permission::<Env::P>(state, &file_path)?;
  Ok(std::fs::read_to_string(file_path)?)
 }
 
@@ -447,7 +449,7 @@ pub fn op_require_as_file_path(file_or_url: String) -> String {
 }
 
 #[op]
-fn op_require_resolve_exports<P>(
+fn op_require_resolve_exports<Env>(
  state: &mut OpState,
  uses_local_node_modules_dir: bool,
  modules_path: String,
@@ -457,10 +459,10 @@ fn op_require_resolve_exports<P>(
  parent_path: String,
 ) -> Result<Option<String>, AnyError>
 where
- P: NodePermissions + 'static,
+ Env: NodeEnv + 'static,
 {
  let resolver = state.borrow::<Rc<dyn RequireNpmResolver>>().clone();
- let permissions = state.borrow_mut::<P>();
+ let permissions = state.borrow_mut::<Env::P>();
 
  let pkg_path = if resolver.in_npm_package(&PathBuf::from(&modules_path))
  && !uses_local_node_modules_dir
@@ -495,19 +497,19 @@ where
 }
 
 #[op]
-fn op_require_read_closest_package_json<P>(
+fn op_require_read_closest_package_json<Env>(
  state: &mut OpState,
  filename: String,
 ) -> Result<PackageJson, AnyError>
 where
- P: NodePermissions + 'static,
+ Env: NodeEnv + 'static,
 {
- ensure_read_permission::<P>(
+ ensure_read_permission::<Env::P>(
  state,
  PathBuf::from(&filename).parent().unwrap(),
  )?;
  let resolver = state.borrow::<Rc<dyn RequireNpmResolver>>().clone();
- let permissions = state.borrow_mut::<P>();
+ let permissions = state.borrow_mut::<Env::P>();
  resolution::get_closest_package_json(
  &Url::from_file_path(filename).unwrap(),
  &*resolver,
@@ -516,32 +518,32 @@ where
 }
 
 #[op]
-fn op_require_read_package_scope<P>(
+fn op_require_read_package_scope<Env>(
  state: &mut OpState,
  package_json_path: String,
 ) -> Option<PackageJson>
 where
- P: NodePermissions + 'static,
+ Env: NodeEnv + 'static,
 {
  let resolver = state.borrow::<Rc<dyn RequireNpmResolver>>().clone();
- let permissions = state.borrow_mut::<P>();
+ let permissions = state.borrow_mut::<Env::P>();
  let package_json_path = PathBuf::from(package_json_path);
  PackageJson::load(&*resolver, permissions, package_json_path).ok()
 }
 
 #[op]
-fn op_require_package_imports_resolve<P>(
+fn op_require_package_imports_resolve<Env>(
  state: &mut OpState,
  parent_filename: String,
  request: String,
 ) -> Result<Option<String>, AnyError>
 where
- P: NodePermissions + 'static,
+ Env: NodeEnv + 'static,
 {
  let parent_path = PathBuf::from(&parent_filename);
- ensure_read_permission::<P>(state, &parent_path)?;
+ ensure_read_permission::<Env::P>(state, &parent_path)?;
  let resolver = state.borrow::<Rc<dyn RequireNpmResolver>>().clone();
- let permissions = state.borrow_mut::<P>();
+ let permissions = state.borrow_mut::<Env::P>();
  let pkg = PackageJson::load(
  &*resolver,
  permissions,

runtime/build.rs

@@ -218,6 +218,12 @@ mod startup_snapshot {
  }
  }
 
+ struct SnapshotNodeEnv;
+
+ impl deno_node::NodeEnv for SnapshotNodeEnv {
+ type P = Permissions;
+ }
+
  deno_core::extension!(runtime,
  deps = [
  deno_webidl,
@@ -319,7 +325,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::<Permissions>(None),
+ deno_node::deno_node::init_ops_and_esm::<SnapshotNodeEnv>(None),
  #[cfg(not(feature = "snapshot_from_snapshot"))]
  runtime_main::init_ops_and_esm(),
  ];

runtime/lib.rs

@@ -36,3 +36,8 @@ 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

@@ -441,7 +441,7 @@ impl WebWorker {
  deno_io::deno_io::init_ops(Some(options.stdio)),
  deno_fs::deno_fs::init_ops::<PermissionsContainer>(unstable),
  deno_flash::deno_flash::init_ops::<PermissionsContainer>(unstable),
- deno_node::deno_node::init_ops::<PermissionsContainer>(
+ deno_node::deno_node::init_ops::<crate::RuntimeNodeEnv>(
  options.npm_resolver,
  ),
  // Runtime ops that are always initialized for WebWorkers

runtime/worker.rs

@@ -265,7 +265,7 @@ impl MainWorker {
  deno_io::deno_io::init_ops(Some(options.stdio)),
  deno_fs::deno_fs::init_ops::<PermissionsContainer>(unstable),
  deno_flash::deno_flash::init_ops::<PermissionsContainer>(unstable),
- deno_node::deno_node::init_ops::<PermissionsContainer>(
+ deno_node::deno_node::init_ops::<crate::RuntimeNodeEnv>(
  options.npm_resolver,
  ),
  // Ops from this crate