From 22f4385301f68877502c9b6481fc05ac200ea50b Mon Sep 17 00:00:00 2001 From: Luca Casonato Date: Thu, 4 May 2023 14:36:38 +0200 Subject: [PATCH] refactor(ext/node): remove NodeEnv trait (#18986) --- cli/build.rs | 2 +- ext/node/lib.rs | 26 +++++++---------- ext/node/ops/require.rs | 65 ++++++++++++++++++++--------------------- runtime/build.rs | 8 +---- runtime/lib.rs | 5 ---- runtime/web_worker.rs | 2 +- runtime/worker.rs | 2 +- 7 files changed, 47 insertions(+), 63 deletions(-) diff --git a/cli/build.rs b/cli/build.rs index 7a3252e20b..21f8c229a1 100644 --- a/cli/build.rs +++ b/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_node::deno_node::init_ops::( None, Some(Arc::new(deno_node::RealFs)), ), diff --git a/ext/node/lib.rs b/ext/node/lib.rs index b5db83297e..128f3a2fea 100644 --- a/ext/node/lib.rs +++ b/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, + ops::require::op_require_node_module_paths

, 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, - ops::require::op_require_try_self, - ops::require::op_require_real_path, + ops::require::op_require_try_self_parent_path

, + ops::require::op_require_try_self

, + ops::require::op_require_real_path

, ops::require::op_require_path_is_absolute, ops::require::op_require_path_dirname, - ops::require::op_require_stat, + ops::require::op_require_stat

, ops::require::op_require_path_resolve, ops::require::op_require_path_basename, - ops::require::op_require_read_file, + ops::require::op_require_read_file

, ops::require::op_require_as_file_path, - ops::require::op_require_resolve_exports, - ops::require::op_require_read_closest_package_json, - ops::require::op_require_read_package_scope, - ops::require::op_require_package_imports_resolve, + ops::require::op_require_resolve_exports

, + ops::require::op_require_read_closest_package_json

, + ops::require::op_require_read_package_scope

, + ops::require::op_require_package_imports_resolve

, ops::require::op_require_break_on_next_statement, ], esm_entry_point = "ext:deno_node/02_init.js", diff --git a/ext/node/ops/require.rs b/ext/node/ops/require.rs index 1c8647bab7..4a2b97187a 100644 --- a/ext/node/ops/require.rs +++ b/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 { } #[op] -pub fn op_require_node_module_paths( +pub fn op_require_node_module_paths

( state: &mut OpState, from: String, ) -> Result, AnyError> where - Env: NodeEnv + 'static, + P: NodePermissions + 'static, { let fs = state.borrow::>(); // Guarantee that "from" is absolute. @@ -105,7 +104,7 @@ where .to_file_path() .unwrap(); - ensure_read_permission::(state, &from)?; + ensure_read_permission::

(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( +fn op_require_stat

( state: &mut OpState, path: String, ) -> Result where - Env: NodeEnv + 'static, + P: NodePermissions + 'static, { let path = PathBuf::from(path); - ensure_read_permission::(state, &path)?; + ensure_read_permission::

(state, &path)?; let fs = state.borrow::>(); if let Ok(metadata) = fs.metadata(&path) { if metadata.is_file { @@ -277,15 +276,15 @@ where } #[op] -fn op_require_real_path( +fn op_require_real_path

( state: &mut OpState, request: String, ) -> Result where - Env: NodeEnv + 'static, + P: NodePermissions + 'static, { let path = PathBuf::from(request); - ensure_read_permission::(state, &path)?; + ensure_read_permission::

(state, &path)?; let fs = state.borrow::>(); 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 { } #[op] -fn op_require_try_self_parent_path( +fn op_require_try_self_parent_path

( state: &mut OpState, has_parent: bool, maybe_parent_filename: Option, maybe_parent_id: Option, ) -> Result, AnyError> where - Env: NodeEnv + 'static, + P: NodePermissions + 'static, { if !has_parent { return Ok(None); @@ -349,7 +348,7 @@ where if parent_id == "" || parent_id == "internal/preload" { let fs = state.borrow::>(); if let Ok(cwd) = fs.current_dir() { - ensure_read_permission::(state, &cwd)?; + ensure_read_permission::

(state, &cwd)?; return Ok(Some(cwd.to_string_lossy().to_string())); } } @@ -358,20 +357,20 @@ where } #[op] -fn op_require_try_self( +fn op_require_try_self

( state: &mut OpState, parent_path: Option, request: String, ) -> Result, AnyError> where - Env: NodeEnv + 'static, + P: NodePermissions + 'static, { if parent_path.is_none() { return Ok(None); } let node_resolver = state.borrow::>(); - let permissions = state.borrow::(); + let permissions = state.borrow::

(); 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( +fn op_require_read_file

( state: &mut OpState, file_path: String, ) -> Result where - Env: NodeEnv + 'static, + P: NodePermissions + 'static, { let file_path = PathBuf::from(file_path); - ensure_read_permission::(state, &file_path)?; + ensure_read_permission::

(state, &file_path)?; let fs = state.borrow::>(); 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( +fn op_require_resolve_exports

( state: &mut OpState, uses_local_node_modules_dir: bool, modules_path: String, @@ -456,12 +455,12 @@ fn op_require_resolve_exports( parent_path: String, ) -> Result, AnyError> where - Env: NodeEnv + 'static, + P: NodePermissions + 'static, { let fs = state.borrow::>(); let npm_resolver = state.borrow::>(); let node_resolver = state.borrow::>(); - let permissions = state.borrow::(); + let permissions = state.borrow::

(); let pkg_path = if npm_resolver .in_npm_package_at_path(&PathBuf::from(&modules_path)) @@ -502,19 +501,19 @@ where } #[op] -fn op_require_read_closest_package_json( +fn op_require_read_closest_package_json

( state: &mut OpState, filename: String, ) -> Result where - Env: NodeEnv + 'static, + P: NodePermissions + 'static, { - ensure_read_permission::( + ensure_read_permission::

( state, PathBuf::from(&filename).parent().unwrap(), )?; let node_resolver = state.borrow::>(); - let permissions = state.borrow::(); + let permissions = state.borrow::

(); node_resolver.get_closest_package_json( &Url::from_file_path(filename).unwrap(), permissions, @@ -522,15 +521,15 @@ where } #[op] -fn op_require_read_package_scope( +fn op_require_read_package_scope

( state: &mut OpState, package_json_path: String, ) -> Option where - Env: NodeEnv + 'static, + P: NodePermissions + 'static, { let node_resolver = state.borrow::>(); - let permissions = state.borrow::(); + let permissions = state.borrow::

(); let package_json_path = PathBuf::from(package_json_path); node_resolver .load_package_json(permissions, package_json_path) @@ -538,18 +537,18 @@ where } #[op] -fn op_require_package_imports_resolve( +fn op_require_package_imports_resolve

( state: &mut OpState, parent_filename: String, request: String, ) -> Result, AnyError> where - Env: NodeEnv + 'static, + P: NodePermissions + 'static, { let parent_path = PathBuf::from(&parent_filename); - ensure_read_permission::(state, &parent_path)?; + ensure_read_permission::

(state, &parent_path)?; let node_resolver = state.borrow::>(); - let permissions = state.borrow::(); + let permissions = state.borrow::

(); let pkg = node_resolver .load_package_json(permissions, parent_path.join("package.json"))?; diff --git a/runtime/build.rs b/runtime/build.rs index 2f3b125959..bba2eae551 100644 --- a/runtime/build.rs +++ b/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::(None, None), + deno_node::deno_node::init_ops_and_esm::(None, None), #[cfg(not(feature = "snapshot_from_snapshot"))] runtime_main::init_ops_and_esm(), ]; diff --git a/runtime/lib.rs b/runtime/lib.rs index 878171913f..50822d373e 100644 --- a/runtime/lib.rs +++ b/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; -} diff --git a/runtime/web_worker.rs b/runtime/web_worker.rs index b688aae8b3..1b3dd28096 100644 --- a/runtime/web_worker.rs +++ b/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::( + deno_node::deno_node::init_ops::( options.npm_resolver, options.node_fs, ), diff --git a/runtime/worker.rs b/runtime/worker.rs index 0d68a4b51e..ac67011f0d 100644 --- a/runtime/worker.rs +++ b/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::( + deno_node::deno_node::init_ops::( options.npm_resolver, options.node_fs, ),