From be97170a193e8cecc5ce03ecd3c1d0add4a06bf7 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Wed, 25 Oct 2023 14:39:00 -0400 Subject: [PATCH] feat(unstable): ability to `npm install` then `deno run main.ts` (#20967) This PR adds a new unstable "bring your own node_modules" (BYONM) functionality currently behind a `--unstable-byonm` flag (`"unstable": ["byonm"]` in a deno.json). This enables users to run a separate install command (ex. `npm install`, `pnpm install`) then run `deno run main.ts` and Deno will respect the layout of the node_modules directory as setup by the separate install command. It also works with npm/yarn/pnpm workspaces. For this PR, the behaviour is opted into by specifying `--unstable-byonm`/`"unstable": ["byonm"]`, but in the future we may make this the default behaviour as outlined in https://github.com/denoland/deno/issues/18967#issuecomment-1761248941 This is an extremely rough initial implementation. Errors are terrible in this and the LSP requires frequent restarts. Improvements will be done in follow up PRs. --- cli/args/flags.rs | 16 +- cli/args/mod.rs | 68 +++-- cli/cache/mod.rs | 29 +- cli/factory.rs | 41 ++- cli/graph_util.rs | 2 +- cli/lsp/analysis.rs | 2 +- cli/lsp/documents.rs | 71 +++-- cli/lsp/language_server.rs | 48 ++- cli/module_loader.rs | 4 +- cli/npm/byonm.rs | 274 ++++++++++++++++++ cli/npm/managed/mod.rs | 45 ++- cli/npm/managed/resolvers/local.rs | 23 +- cli/npm/managed/tarball.rs | 14 +- cli/npm/mod.rs | 21 +- cli/resolver.rs | 121 +++++++- cli/tests/integration/cert_tests.rs | 2 +- cli/tests/integration/compile_tests.rs | 18 +- cli/tests/integration/npm_tests.rs | 237 +++++++++++++++ .../1.0.0/cjs/index.cjs | 3 + .../1.0.0/esm/client/bar.js | 3 + .../1.0.0/esm/client/foo.js | 3 + .../1.0.0/esm/client/index.js | 3 + .../1.0.0/esm/client/m.js | 3 + .../1.0.0/esm/index.js | 3 + .../conditional-exports-strict/1.0.0/foo.js | 3 + .../1.0.0/package.json | 16 + .../1.0.0/esm/client/bar.js | 2 +- .../1.0.0/esm/client/foo.js | 2 +- .../1.0.0/esm/client/index.js | 2 +- .../conditional-exports/1.0.0/esm/client/m.js | 2 +- .../@denotest/dual-cjs-esm/1.0.0/main.d.cts | 1 + .../@denotest/dual-cjs-esm/1.0.0/main.d.mts | 1 + cli/tools/vendor/test.rs | 22 +- test_util/src/builders.rs | 15 +- test_util/src/lib.rs | 29 +- test_util/src/npm.rs | 2 +- tools/lint.js | 2 +- 37 files changed, 935 insertions(+), 218 deletions(-) create mode 100644 cli/npm/byonm.rs create mode 100644 cli/tests/testdata/npm/registry/@denotest/conditional-exports-strict/1.0.0/cjs/index.cjs create mode 100644 cli/tests/testdata/npm/registry/@denotest/conditional-exports-strict/1.0.0/esm/client/bar.js create mode 100644 cli/tests/testdata/npm/registry/@denotest/conditional-exports-strict/1.0.0/esm/client/foo.js create mode 100644 cli/tests/testdata/npm/registry/@denotest/conditional-exports-strict/1.0.0/esm/client/index.js create mode 100644 cli/tests/testdata/npm/registry/@denotest/conditional-exports-strict/1.0.0/esm/client/m.js create mode 100644 cli/tests/testdata/npm/registry/@denotest/conditional-exports-strict/1.0.0/esm/index.js create mode 100644 cli/tests/testdata/npm/registry/@denotest/conditional-exports-strict/1.0.0/foo.js create mode 100644 cli/tests/testdata/npm/registry/@denotest/conditional-exports-strict/1.0.0/package.json create mode 100644 cli/tests/testdata/npm/registry/@denotest/dual-cjs-esm/1.0.0/main.d.cts create mode 100644 cli/tests/testdata/npm/registry/@denotest/dual-cjs-esm/1.0.0/main.d.mts diff --git a/cli/args/flags.rs b/cli/args/flags.rs index 138d77359b..588f75dbbd 100644 --- a/cli/args/flags.rs +++ b/cli/args/flags.rs @@ -407,6 +407,7 @@ pub struct Flags { pub seed: Option, pub unstable: bool, pub unstable_bare_node_builtlins: bool, + pub unstable_byonm: bool, pub unsafely_ignore_certificate_errors: Option>, pub v8_flags: Vec, } @@ -803,9 +804,9 @@ pub fn flags_from_vec(args: Vec) -> clap::error::Result { flags.unstable = true; } - if matches.get_flag("unstable-bare-node-builtins") { - flags.unstable_bare_node_builtlins = true; - } + flags.unstable_bare_node_builtlins = + matches.get_flag("unstable-bare-node-builtins"); + flags.unstable_byonm = matches.get_flag("unstable-byonm"); if matches.get_flag("quiet") { flags.log_level = Some(Level::Error); @@ -911,6 +912,15 @@ fn clap_root() -> Command { .action(ArgAction::SetTrue) .global(true), ) + .arg( + Arg::new("unstable-byonm") + .long("unstable-byonm") + .help("Enable unstable 'bring your own node_modules' feature") + .env("DENO_UNSTABLE_BYONM") + .value_parser(FalseyValueParser::new()) + .action(ArgAction::SetTrue) + .global(true), + ) .arg( Arg::new("log-level") .short('L') diff --git a/cli/args/mod.rs b/cli/args/mod.rs index 390e16698c..ab8d6b503c 100644 --- a/cli/args/mod.rs +++ b/cli/args/mod.rs @@ -76,25 +76,29 @@ use deno_config::FmtConfig; use deno_config::LintConfig; use deno_config::TestConfig; -static NPM_REGISTRY_DEFAULT_URL: Lazy = Lazy::new(|| { - let env_var_name = "NPM_CONFIG_REGISTRY"; - if let Ok(registry_url) = std::env::var(env_var_name) { - // ensure there is a trailing slash for the directory - let registry_url = format!("{}/", registry_url.trim_end_matches('/')); - match Url::parse(®istry_url) { - Ok(url) => { - return url; - } - Err(err) => { - log::debug!("Invalid {} environment variable: {:#}", env_var_name, err,); +pub fn npm_registry_default_url() -> &'static Url { + static NPM_REGISTRY_DEFAULT_URL: Lazy = Lazy::new(|| { + let env_var_name = "NPM_CONFIG_REGISTRY"; + if let Ok(registry_url) = std::env::var(env_var_name) { + // ensure there is a trailing slash for the directory + let registry_url = format!("{}/", registry_url.trim_end_matches('/')); + match Url::parse(®istry_url) { + Ok(url) => { + return url; + } + Err(err) => { + log::debug!( + "Invalid {} environment variable: {:#}", + env_var_name, + err, + ); + } } } - } - Url::parse("https://registry.npmjs.org").unwrap() -}); + Url::parse("https://registry.npmjs.org").unwrap() + }); -pub fn npm_registry_default_url() -> &'static Url { &NPM_REGISTRY_DEFAULT_URL } @@ -570,10 +574,16 @@ pub fn get_root_cert_store( /// State provided to the process via an environment variable. #[derive(Clone, Debug, Serialize, Deserialize)] pub struct NpmProcessState { - pub snapshot: deno_npm::resolution::SerializedNpmResolutionSnapshot, + pub kind: NpmProcessStateKind, pub local_node_modules_path: Option, } +#[derive(Clone, Debug, Serialize, Deserialize)] +pub enum NpmProcessStateKind { + Snapshot(deno_npm::resolution::SerializedNpmResolutionSnapshot), + Byonm, +} + const RESOLUTION_STATE_ENV_VAR_NAME: &str = "DENO_DONT_USE_INTERNAL_NODE_COMPAT_STATE"; @@ -875,9 +885,11 @@ impl CliOptions { pub fn resolve_npm_resolution_snapshot( &self, ) -> Result, AnyError> { - if let Some(state) = &*NPM_PROCESS_STATE { + if let Some(NpmProcessStateKind::Snapshot(snapshot)) = + NPM_PROCESS_STATE.as_ref().map(|s| &s.kind) + { // TODO(bartlomieju): remove this clone - Ok(Some(state.snapshot.clone().into_valid()?)) + Ok(Some(snapshot.clone().into_valid()?)) } else { Ok(None) } @@ -926,13 +938,6 @@ impl CliOptions { }) } - pub fn node_modules_dir_specifier(&self) -> Option { - self - .maybe_node_modules_folder - .as_ref() - .map(|path| ModuleSpecifier::from_directory_path(path).unwrap()) - } - pub fn vendor_dir_path(&self) -> Option<&PathBuf> { self.maybe_vendor_folder.as_ref() } @@ -1226,6 +1231,19 @@ impl CliOptions { .unwrap_or(false) } + pub fn unstable_byonm(&self) -> bool { + self.flags.unstable_byonm + || NPM_PROCESS_STATE + .as_ref() + .map(|s| matches!(s.kind, NpmProcessStateKind::Byonm)) + .unwrap_or(false) + || self + .maybe_config_file() + .as_ref() + .map(|c| c.json.unstable.iter().any(|c| c == "byonm")) + .unwrap_or(false) + } + pub fn v8_flags(&self) -> &Vec { &self.flags.v8_flags } diff --git a/cli/cache/mod.rs b/cli/cache/mod.rs index 1d6a799634..5cc91f50fa 100644 --- a/cli/cache/mod.rs +++ b/cli/cache/mod.rs @@ -4,6 +4,7 @@ use crate::args::CacheSetting; use crate::errors::get_error_class_name; use crate::file_fetcher::FetchOptions; use crate::file_fetcher::FileFetcher; +use crate::npm::CliNpmResolver; use crate::util::fs::atomic_write_file; use deno_ast::MediaType; @@ -101,10 +102,10 @@ pub struct FetchCacher { file_fetcher: Arc, file_header_overrides: HashMap>, global_http_cache: Arc, + npm_resolver: Arc, parsed_source_cache: Arc, permissions: PermissionsContainer, cache_info_enabled: bool, - maybe_local_node_modules_url: Option, } impl FetchCacher { @@ -113,19 +114,19 @@ impl FetchCacher { file_fetcher: Arc, file_header_overrides: HashMap>, global_http_cache: Arc, + npm_resolver: Arc, parsed_source_cache: Arc, permissions: PermissionsContainer, - maybe_local_node_modules_url: Option, ) -> Self { Self { emit_cache, file_fetcher, file_header_overrides, global_http_cache, + npm_resolver, parsed_source_cache, permissions, cache_info_enabled: false, - maybe_local_node_modules_url, } } @@ -214,20 +215,18 @@ impl Loader for FetchCacher { ) -> LoadFuture { use deno_graph::source::CacheSetting as LoaderCacheSetting; - if let Some(node_modules_url) = self.maybe_local_node_modules_url.as_ref() { + if specifier.path().contains("/node_modules/") { // The specifier might be in a completely different symlinked tree than - // what the resolved node_modules_url is in (ex. `/my-project-1/node_modules` - // symlinked to `/my-project-2/node_modules`), so first check if the path - // is in a node_modules dir to avoid needlessly canonicalizing, then compare + // what the node_modules url is in (ex. `/my-project-1/node_modules` + // symlinked to `/my-project-2/node_modules`), so first we checked if the path + // is in a node_modules dir to avoid needlessly canonicalizing, then now compare // against the canonicalized specifier. - if specifier.path().contains("/node_modules/") { - let specifier = - crate::node::resolve_specifier_into_node_modules(specifier); - if specifier.as_str().starts_with(node_modules_url.as_str()) { - return Box::pin(futures::future::ready(Ok(Some( - LoadResponse::External { specifier }, - )))); - } + let specifier = + crate::node::resolve_specifier_into_node_modules(specifier); + if self.npm_resolver.in_npm_package(&specifier) { + return Box::pin(futures::future::ready(Ok(Some( + LoadResponse::External { specifier }, + )))); } } diff --git a/cli/factory.rs b/cli/factory.rs index 0c887b720a..b5240a85aa 100644 --- a/cli/factory.rs +++ b/cli/factory.rs @@ -32,6 +32,7 @@ use crate::node::CliCjsCodeAnalyzer; use crate::node::CliNodeCodeTranslator; use crate::npm::create_cli_npm_resolver; use crate::npm::CliNpmResolver; +use crate::npm::CliNpmResolverByonmCreateOptions; use crate::npm::CliNpmResolverCreateOptions; use crate::npm::CliNpmResolverManagedCreateOptions; use crate::npm::CliNpmResolverManagedPackageJsonInstallerOption; @@ -300,7 +301,14 @@ impl CliFactory { .npm_resolver .get_or_try_init_async(async { let fs = self.fs(); - create_cli_npm_resolver( + create_cli_npm_resolver(if self.options.unstable_byonm() { + CliNpmResolverCreateOptions::Byonm(CliNpmResolverByonmCreateOptions { + fs: fs.clone(), + // todo(byonm): actually resolve this properly because the package.json + // might be in an ancestor directory + root_node_modules_dir: self.options.initial_cwd().join("node_modules"), + }) + } else { CliNpmResolverCreateOptions::Managed(CliNpmResolverManagedCreateOptions { snapshot: match self.options.resolve_npm_resolution_snapshot()? { Some(snapshot) => { @@ -329,7 +337,7 @@ impl CliFactory { npm_system_info: self.options.npm_system_info(), npm_registry_url: crate::args::npm_registry_default_url().to_owned(), }) - ).await + }).await }) .await } @@ -365,24 +373,25 @@ impl CliFactory { .services .resolver .get_or_try_init_async(async { - Ok(Arc::new(CliGraphResolver::new( - if self.options.no_npm() { + Ok(Arc::new(CliGraphResolver::new(CliGraphResolverOptions { + fs: self.fs().clone(), + cjs_resolutions: Some(self.cjs_resolutions().clone()), + node_resolver: Some(self.node_resolver().await?.clone()), + npm_resolver: if self.options.no_npm() { None } else { Some(self.npm_resolver().await?.clone()) }, - self.package_json_deps_provider().clone(), - CliGraphResolverOptions { - maybe_jsx_import_source_config: self - .options - .to_maybe_jsx_import_source_config()?, - maybe_import_map: self.maybe_import_map().await?.clone(), - maybe_vendor_dir: self.options.vendor_dir_path(), - bare_node_builtins_enabled: self - .options - .unstable_bare_node_builtlins(), - }, - ))) + package_json_deps_provider: self.package_json_deps_provider().clone(), + maybe_jsx_import_source_config: self + .options + .to_maybe_jsx_import_source_config()?, + maybe_import_map: self.maybe_import_map().await?.clone(), + maybe_vendor_dir: self.options.vendor_dir_path(), + bare_node_builtins_enabled: self + .options + .unstable_bare_node_builtlins(), + }))) }) .await } diff --git a/cli/graph_util.rs b/cli/graph_util.rs index 9a2b805fd2..d6b5228cf8 100644 --- a/cli/graph_util.rs +++ b/cli/graph_util.rs @@ -435,9 +435,9 @@ impl ModuleGraphBuilder { self.file_fetcher.clone(), self.options.resolve_file_header_overrides(), self.global_http_cache.clone(), + self.npm_resolver.clone(), self.parsed_source_cache.clone(), permissions, - self.options.node_modules_dir_specifier(), ) } diff --git a/cli/lsp/analysis.rs b/cli/lsp/analysis.rs index 6515e7dc08..f8ace060a4 100644 --- a/cli/lsp/analysis.rs +++ b/cli/lsp/analysis.rs @@ -253,7 +253,7 @@ impl<'a> TsResponseImportMapper<'a> { let root_folder = self .npm_resolver .as_ref() - .and_then(|r| r.resolve_pkg_folder_from_specifier(specifier).ok()) + .and_then(|r| r.resolve_package_folder_from_path(specifier).ok()) .flatten()?; let package_json_path = root_folder.join("package.json"); let package_json_text = std::fs::read_to_string(&package_json_path).ok()?; diff --git a/cli/lsp/documents.rs b/cli/lsp/documents.rs index 57957780b7..e29ad785e6 100644 --- a/cli/lsp/documents.rs +++ b/cli/lsp/documents.rs @@ -37,9 +37,11 @@ use deno_core::ModuleSpecifier; use deno_graph::source::ResolutionMode; use deno_graph::GraphImport; use deno_graph::Resolution; +use deno_runtime::deno_fs::RealFs; use deno_runtime::deno_node; use deno_runtime::deno_node::NodeResolution; use deno_runtime::deno_node::NodeResolutionMode; +use deno_runtime::deno_node::NodeResolver; use deno_runtime::deno_node::PackageJson; use deno_runtime::permissions::PermissionsContainer; use deno_semver::npm::NpmPackageReqReference; @@ -899,6 +901,7 @@ pub struct UpdateDocumentConfigOptions<'a> { pub maybe_import_map: Option>, pub maybe_config_file: Option<&'a ConfigFile>, pub maybe_package_json: Option<&'a PackageJson>, + pub node_resolver: Option>, pub npm_resolver: Option>, } @@ -955,16 +958,17 @@ impl Documents { file_system_docs: Default::default(), resolver_config_hash: 0, imports: Default::default(), - resolver: Arc::new(CliGraphResolver::new( - None, - Arc::new(PackageJsonDepsProvider::default()), - CliGraphResolverOptions { - maybe_jsx_import_source_config: None, - maybe_import_map: None, - maybe_vendor_dir: None, - bare_node_builtins_enabled: false, - }, - )), + resolver: Arc::new(CliGraphResolver::new(CliGraphResolverOptions { + fs: Arc::new(RealFs), + node_resolver: None, + npm_resolver: None, + cjs_resolutions: None, + package_json_deps_provider: Arc::new(PackageJsonDepsProvider::default()), + maybe_jsx_import_source_config: None, + maybe_import_map: None, + maybe_vendor_dir: None, + bare_node_builtins_enabled: false, + })), npm_specifier_reqs: Default::default(), has_injected_types_node_package: false, specifier_resolver: Arc::new(SpecifierResolver::new(cache)), @@ -1329,7 +1333,7 @@ impl Documents { if let Some(package_json_deps) = &maybe_package_json_deps { // We need to ensure the hashing is deterministic so explicitly type // this in order to catch if the type of package_json_deps ever changes - // from a sorted/deterministic IndexMap to something else. + // from a deterministic IndexMap to something else. let package_json_deps: &IndexMap<_, _> = *package_json_deps; for (key, value) in package_json_deps { hasher.write_hashable(key); @@ -1364,27 +1368,28 @@ impl Documents { ); let deps_provider = Arc::new(PackageJsonDepsProvider::new(maybe_package_json_deps)); - self.resolver = Arc::new(CliGraphResolver::new( - options.npm_resolver, - deps_provider, - CliGraphResolverOptions { - maybe_jsx_import_source_config: maybe_jsx_config, - maybe_import_map: options.maybe_import_map, - maybe_vendor_dir: options - .maybe_config_file - .and_then(|c| c.vendor_dir_path()) - .as_ref(), - bare_node_builtins_enabled: options - .maybe_config_file - .map(|config| { - config - .json - .unstable - .contains(&"bare-node-builtins".to_string()) - }) - .unwrap_or(false), - }, - )); + self.resolver = Arc::new(CliGraphResolver::new(CliGraphResolverOptions { + fs: Arc::new(RealFs), + node_resolver: options.node_resolver, + npm_resolver: options.npm_resolver, + cjs_resolutions: None, // only used for runtime + package_json_deps_provider: deps_provider, + maybe_jsx_import_source_config: maybe_jsx_config, + maybe_import_map: options.maybe_import_map, + maybe_vendor_dir: options + .maybe_config_file + .and_then(|c| c.vendor_dir_path()) + .as_ref(), + bare_node_builtins_enabled: options + .maybe_config_file + .map(|config| { + config + .json + .unstable + .contains(&"bare-node-builtins".to_string()) + }) + .unwrap_or(false), + })); self.imports = Arc::new( if let Some(Ok(imports)) = options.maybe_config_file.map(|cf| cf.to_maybe_imports()) @@ -2194,6 +2199,7 @@ console.log(b, "hello deno"); maybe_import_map: Some(Arc::new(import_map)), maybe_config_file: None, maybe_package_json: None, + node_resolver: None, npm_resolver: None, }); @@ -2235,6 +2241,7 @@ console.log(b, "hello deno"); maybe_import_map: Some(Arc::new(import_map)), maybe_config_file: None, maybe_package_json: None, + node_resolver: None, npm_resolver: None, }); diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs index 8afcc7ffc0..8b275a650e 100644 --- a/cli/lsp/language_server.rs +++ b/cli/lsp/language_server.rs @@ -105,6 +105,7 @@ use crate::lsp::tsc::file_text_changes_to_workspace_edit; use crate::lsp::urls::LspUrlKind; use crate::npm::create_cli_npm_resolver_for_lsp; use crate::npm::CliNpmResolver; +use crate::npm::CliNpmResolverByonmCreateOptions; use crate::npm::CliNpmResolverCreateOptions; use crate::npm::CliNpmResolverManagedCreateOptions; use crate::npm::CliNpmResolverManagedPackageJsonInstallerOption; @@ -131,6 +132,8 @@ struct LspNpmServices { config_hash: LspNpmConfigHash, /// Npm's search api. search_api: CliNpmSearchApi, + /// Node resolver. + node_resolver: Option>, /// Resolver for npm packages. resolver: Option>, } @@ -495,6 +498,7 @@ impl Inner { npm: LspNpmServices { config_hash: LspNpmConfigHash(0), // this will be updated in initialize search_api: npm_search_api, + node_resolver: None, resolver: None, }, performance, @@ -815,15 +819,19 @@ impl Inner { return; // no need to do anything } - self.npm.resolver = Some( - create_npm_resolver( - &deno_dir, - &self.http_client, - self.config.maybe_lockfile(), - self.config.maybe_node_modules_dir_path().cloned(), - ) - .await, - ); + let npm_resolver = create_npm_resolver( + &deno_dir, + &self.http_client, + self.config.maybe_config_file(), + self.config.maybe_lockfile(), + self.config.maybe_node_modules_dir_path().cloned(), + ) + .await; + self.npm.node_resolver = Some(Arc::new(NodeResolver::new( + Arc::new(deno_fs::RealFs), + npm_resolver.clone().into_npm_resolver(), + ))); + self.npm.resolver = Some(npm_resolver); // update the hash self.npm.config_hash = config_hash; @@ -1059,11 +1067,24 @@ impl Inner { async fn create_npm_resolver( deno_dir: &DenoDir, http_client: &Arc, + maybe_config_file: Option<&ConfigFile>, maybe_lockfile: Option<&Arc>>, maybe_node_modules_dir_path: Option, ) -> Arc { - create_cli_npm_resolver_for_lsp(CliNpmResolverCreateOptions::Managed( - CliNpmResolverManagedCreateOptions { + let is_byonm = std::env::var("DENO_UNSTABLE_BYONM").as_deref() == Ok("1") + || maybe_config_file + .as_ref() + .map(|c| c.json.unstable.iter().any(|c| c == "byonm")) + .unwrap_or(false); + create_cli_npm_resolver_for_lsp(if is_byonm { + CliNpmResolverCreateOptions::Byonm(CliNpmResolverByonmCreateOptions { + fs: Arc::new(deno_fs::RealFs), + root_node_modules_dir: std::env::current_dir() + .unwrap() + .join("node_modules"), + }) + } else { + CliNpmResolverCreateOptions::Managed(CliNpmResolverManagedCreateOptions { http_client: http_client.clone(), snapshot: match maybe_lockfile { Some(lockfile) => { @@ -1090,8 +1111,8 @@ async fn create_npm_resolver( CliNpmResolverManagedPackageJsonInstallerOption::NoInstall, npm_registry_url: crate::args::npm_registry_default_url().to_owned(), npm_system_info: NpmSystemInfo::default(), - }, - )) + }) + }) .await } @@ -1250,6 +1271,7 @@ impl Inner { maybe_import_map: self.maybe_import_map.clone(), maybe_config_file: self.config.maybe_config_file(), maybe_package_json: self.maybe_package_json.as_ref(), + node_resolver: self.npm.node_resolver.clone(), npm_resolver: self.npm.resolver.clone(), }); diff --git a/cli/module_loader.rs b/cli/module_loader.rs index 3f5e82d8c5..f193c7e153 100644 --- a/cli/module_loader.rs +++ b/cli/module_loader.rs @@ -738,7 +738,7 @@ impl CliNodeResolver { .with_context(|| format!("Could not resolve '{}'.", req_ref)) } - fn resolve_package_sub_path( + pub fn resolve_package_sub_path( &self, package_folder: &Path, sub_path: Option<&str>, @@ -881,7 +881,7 @@ impl NpmModuleLoader { } /// Keeps track of what module specifiers were resolved as CJS. -#[derive(Default)] +#[derive(Debug, Default)] pub struct CjsResolutionStore(Mutex>); impl CjsResolutionStore { diff --git a/cli/npm/byonm.rs b/cli/npm/byonm.rs new file mode 100644 index 0000000000..7aba839159 --- /dev/null +++ b/cli/npm/byonm.rs @@ -0,0 +1,274 @@ +// Copyright 2018-2023 the Deno authors. All rights reserved. MIT license. + +use std::borrow::Cow; +use std::path::Path; +use std::path::PathBuf; +use std::sync::Arc; + +use deno_ast::ModuleSpecifier; +use deno_core::anyhow::bail; +use deno_core::error::AnyError; +use deno_core::serde_json; +use deno_runtime::deno_fs::FileSystem; +use deno_runtime::deno_node::NodePermissions; +use deno_runtime::deno_node::NodeResolutionMode; +use deno_runtime::deno_node::NpmResolver; +use deno_runtime::deno_node::PackageJson; +use deno_semver::package::PackageReq; + +use crate::args::package_json::get_local_package_json_version_reqs; +use crate::args::NpmProcessState; +use crate::args::NpmProcessStateKind; +use crate::util::fs::canonicalize_path_maybe_not_exists; +use crate::util::path::specifier_to_file_path; + +use super::common::types_package_name; +use super::CliNpmResolver; +use super::InnerCliNpmResolverRef; + +pub struct CliNpmResolverByonmCreateOptions { + pub fs: Arc, + pub root_node_modules_dir: PathBuf, +} + +pub fn create_byonm_npm_resolver( + options: CliNpmResolverByonmCreateOptions, +) -> Arc { + Arc::new(ByonmCliNpmResolver { + fs: options.fs, + root_node_modules_dir: options.root_node_modules_dir, + }) +} + +#[derive(Debug)] +pub struct ByonmCliNpmResolver { + fs: Arc, + root_node_modules_dir: PathBuf, +} + +impl NpmResolver for ByonmCliNpmResolver { + fn resolve_package_folder_from_package( + &self, + name: &str, + referrer: &ModuleSpecifier, + mode: NodeResolutionMode, + ) -> Result { + fn inner( + fs: &dyn FileSystem, + name: &str, + package_root_path: &Path, + referrer: &ModuleSpecifier, + mode: NodeResolutionMode, + ) -> Result { + let mut current_folder = package_root_path; + loop { + let node_modules_folder = if current_folder.ends_with("node_modules") { + Cow::Borrowed(current_folder) + } else { + Cow::Owned(current_folder.join("node_modules")) + }; + + // attempt to resolve the types package first, then fallback to the regular package + if mode.is_types() && !name.starts_with("@types/") { + let sub_dir = + join_package_name(&node_modules_folder, &types_package_name(name)); + if fs.is_dir_sync(&sub_dir) { + return Ok(sub_dir); + } + } + + let sub_dir = join_package_name(&node_modules_folder, name); + if fs.is_dir_sync(&sub_dir) { + return Ok(sub_dir); + } + + if let Some(parent) = current_folder.parent() { + current_folder = parent; + } else { + break; + } + } + + bail!( + "could not find package '{}' from referrer '{}'.", + name, + referrer + ); + } + + let package_root_path = + self.resolve_package_folder_from_path(referrer)?.unwrap(); // todo(byonm): don't unwrap + let path = inner(&*self.fs, name, &package_root_path, referrer, mode)?; + Ok(self.fs.realpath_sync(&path)?) + } + + fn resolve_package_folder_from_path( + &self, + specifier: &deno_core::ModuleSpecifier, + ) -> Result, AnyError> { + let path = specifier.to_file_path().unwrap(); // todo(byonm): don't unwrap + let path = self.fs.realpath_sync(&path)?; + if self.in_npm_package(specifier) { + let mut path = path.as_path(); + while let Some(parent) = path.parent() { + if parent + .file_name() + .and_then(|f| f.to_str()) + .map(|s| s.to_ascii_lowercase()) + .as_deref() + == Some("node_modules") + { + return Ok(Some(path.to_path_buf())); + } else { + path = parent; + } + } + } else { + // find the folder with a package.json + // todo(dsherret): not exactly correct, but good enough for a first pass + let mut path = path.as_path(); + while let Some(parent) = path.parent() { + if parent.join("package.json").exists() { + return Ok(Some(parent.to_path_buf())); + } else { + path = parent; + } + } + } + Ok(None) + } + + fn in_npm_package(&self, specifier: &ModuleSpecifier) -> bool { + specifier.scheme() == "file" + && specifier + .path() + .to_ascii_lowercase() + .contains("/node_modules/") + } + + fn ensure_read_permission( + &self, + permissions: &dyn NodePermissions, + path: &Path, + ) -> Result<(), AnyError> { + if !path + .components() + .any(|c| c.as_os_str().to_ascii_lowercase() == "node_modules") + { + permissions.check_read(path)?; + } + Ok(()) + } +} + +impl CliNpmResolver for ByonmCliNpmResolver { + fn into_npm_resolver(self: Arc) -> Arc { + self + } + + fn clone_snapshotted(&self) -> Arc { + Arc::new(Self { + fs: self.fs.clone(), + root_node_modules_dir: self.root_node_modules_dir.clone(), + }) + } + + fn as_inner(&self) -> InnerCliNpmResolverRef { + InnerCliNpmResolverRef::Byonm(self) + } + + fn root_node_modules_path(&self) -> Option { + Some(self.root_node_modules_dir.clone()) + } + + fn resolve_pkg_folder_from_deno_module_req( + &self, + req: &PackageReq, + referrer: &ModuleSpecifier, + ) -> Result { + fn resolve_from_package_json( + req: &PackageReq, + fs: &dyn FileSystem, + path: PathBuf, + ) -> Result { + let package_json = PackageJson::load_skip_read_permission(fs, path)?; + let deps = get_local_package_json_version_reqs(&package_json); + for (key, value) in deps { + if let Ok(value) = value { + if value.name == req.name + && value.version_req.intersects(&req.version_req) + { + let package_path = package_json + .path + .parent() + .unwrap() + .join("node_modules") + .join(key); + return Ok(canonicalize_path_maybe_not_exists(&package_path)?); + } + } + } + bail!( + concat!( + "Could not find a matching package for 'npm:{}' in '{}'. ", + "You must specify this as a package.json dependency when the ", + "node_modules folder is not managed by Deno.", + ), + req, + package_json.path.display() + ); + } + + // attempt to resolve the npm specifier from the referrer's package.json, + // but otherwise fallback to the project's package.json + if let Ok(file_path) = specifier_to_file_path(referrer) { + let mut current_path = file_path.as_path(); + while let Some(dir_path) = current_path.parent() { + let package_json_path = dir_path.join("package.json"); + if self.fs.exists_sync(&package_json_path) { + return resolve_from_package_json( + req, + self.fs.as_ref(), + package_json_path, + ); + } + current_path = dir_path; + } + } + + resolve_from_package_json( + req, + self.fs.as_ref(), + self + .root_node_modules_dir + .parent() + .unwrap() + .join("package.json"), + ) + } + + fn get_npm_process_state(&self) -> String { + serde_json::to_string(&NpmProcessState { + kind: NpmProcessStateKind::Byonm, + local_node_modules_path: Some( + self.root_node_modules_dir.to_string_lossy().to_string(), + ), + }) + .unwrap() + } + + fn check_state_hash(&self) -> Option { + // it is very difficult to determine the check state hash for byonm + // so we just return None to signify check caching is not supported + None + } +} + +fn join_package_name(path: &Path, package_name: &str) -> PathBuf { + let mut path = path.to_path_buf(); + // ensure backslashes are used on windows + for part in package_name.split('/') { + path = path.join(part); + } + path +} diff --git a/cli/npm/managed/mod.rs b/cli/npm/managed/mod.rs index b85f1130f6..68b5c2134e 100644 --- a/cli/npm/managed/mod.rs +++ b/cli/npm/managed/mod.rs @@ -27,6 +27,7 @@ use deno_semver::package::PackageReq; use crate::args::Lockfile; use crate::args::NpmProcessState; +use crate::args::NpmProcessStateKind; use crate::args::PackageJsonDepsProvider; use crate::cache::FastInsecureHasher; use crate::util::fs::canonicalize_path_maybe_not_exists_with_fs; @@ -508,7 +509,18 @@ impl NpmResolver for ManagedCliNpmResolver { &self, specifier: &ModuleSpecifier, ) -> Result, AnyError> { - self.resolve_pkg_folder_from_specifier(specifier) + let Some(path) = self + .fs_resolver + .resolve_package_folder_from_specifier(specifier)? + else { + return Ok(None); + }; + log::debug!( + "Resolved package folder of {} to {}", + specifier, + path.display() + ); + Ok(Some(path)) } fn in_npm_package(&self, specifier: &ModuleSpecifier) -> bool { @@ -568,27 +580,6 @@ impl CliNpmResolver for ManagedCliNpmResolver { self.fs_resolver.node_modules_path() } - /// Resolve the root folder of the package the provided specifier is in. - /// - /// This will error when the provided specifier is not in an npm package. - fn resolve_pkg_folder_from_specifier( - &self, - specifier: &ModuleSpecifier, - ) -> Result, AnyError> { - let Some(path) = self - .fs_resolver - .resolve_package_folder_from_specifier(specifier)? - else { - return Ok(None); - }; - log::debug!( - "Resolved package folder of {} to {}", - specifier, - path.display() - ); - Ok(Some(path)) - } - fn resolve_pkg_folder_from_deno_module_req( &self, req: &PackageReq, @@ -601,10 +592,12 @@ impl CliNpmResolver for ManagedCliNpmResolver { /// Gets the state of npm for the process. fn get_npm_process_state(&self) -> String { serde_json::to_string(&NpmProcessState { - snapshot: self - .resolution - .serialized_valid_snapshot() - .into_serialized(), + kind: NpmProcessStateKind::Snapshot( + self + .resolution + .serialized_valid_snapshot() + .into_serialized(), + ), local_node_modules_path: self .fs_resolver .node_modules_path() diff --git a/cli/npm/managed/resolvers/local.rs b/cli/npm/managed/resolvers/local.rs index 2d774518a7..a4a8550f12 100644 --- a/cli/npm/managed/resolvers/local.rs +++ b/cli/npm/managed/resolvers/local.rs @@ -36,7 +36,6 @@ use deno_runtime::deno_core::futures; use deno_runtime::deno_fs; use deno_runtime::deno_node::NodePermissions; use deno_runtime::deno_node::NodeResolutionMode; -use deno_runtime::deno_node::PackageJson; use deno_semver::package::PackageNv; use serde::Deserialize; use serde::Serialize; @@ -181,23 +180,8 @@ impl NpmPackageFsResolver for LocalNpmPackageResolver { } else { Cow::Owned(current_folder.join("node_modules")) }; - let sub_dir = join_package_name(&node_modules_folder, name); - if self.fs.is_dir_sync(&sub_dir) { - // if doing types resolution, only resolve the package if it specifies a types property - if mode.is_types() && !name.starts_with("@types/") { - let package_json = PackageJson::load_skip_read_permission( - &*self.fs, - sub_dir.join("package.json"), - )?; - if package_json.types.is_some() { - return Ok(sub_dir); - } - } else { - return Ok(sub_dir); - } - } - // if doing type resolution, check for the existence of a @types package + // attempt to resolve the types package first, then fallback to the regular package if mode.is_types() && !name.starts_with("@types/") { let sub_dir = join_package_name(&node_modules_folder, &types_package_name(name)); @@ -206,6 +190,11 @@ impl NpmPackageFsResolver for LocalNpmPackageResolver { } } + let sub_dir = join_package_name(&node_modules_folder, name); + if self.fs.is_dir_sync(&sub_dir) { + return Ok(sub_dir); + } + if current_folder == self.root_node_modules_path { bail!( "could not find package '{}' from referrer '{}'.", diff --git a/cli/npm/managed/tarball.rs b/cli/npm/managed/tarball.rs index e72b1afc85..17ab7711f8 100644 --- a/cli/npm/managed/tarball.rs +++ b/cli/npm/managed/tarball.rs @@ -52,15 +52,15 @@ fn verify_tarball_integrity( let mut hash_ctx = Context::new(algo); hash_ctx.update(data); let digest = hash_ctx.finish(); - let tarball_checksum = base64::encode(digest.as_ref()).to_lowercase(); - (tarball_checksum, base64_hash.to_lowercase()) + let tarball_checksum = base64::encode(digest.as_ref()); + (tarball_checksum, base64_hash) } NpmPackageVersionDistInfoIntegrity::LegacySha1Hex(hex) => { let mut hash_ctx = Context::new(&ring::digest::SHA1_FOR_LEGACY_USE_ONLY); hash_ctx.update(data); let digest = hash_ctx.finish(); - let tarball_checksum = hex::encode(digest.as_ref()).to_lowercase(); - (tarball_checksum, hex.to_lowercase()) + let tarball_checksum = hex::encode(digest.as_ref()); + (tarball_checksum, hex) } NpmPackageVersionDistInfoIntegrity::UnknownIntegrity(integrity) => { bail!( @@ -71,7 +71,7 @@ fn verify_tarball_integrity( } }; - if tarball_checksum != expected_checksum { + if tarball_checksum != *expected_checksum { bail!( "Tarball checksum did not match what was provided by npm registry for {}.\n\nExpected: {}\nActual: {}", package, @@ -158,7 +158,7 @@ mod test { version: Version::parse_from_npm("1.0.0").unwrap(), }; let actual_checksum = - "z4phnx7vul3xvchq1m2ab9yg5aulvxxcg/spidns6c5h0ne8xyxysp+dgnkhfuwvy7kxvudbeoglodj6+sfapg=="; + "z4PhNX7vuL3xVChQ1m2AB9Yg5AULVxXcg/SpIdNs6c5H0NE8XYXysP+DGNKHfuwvY7kxvUdBeoGlODJ6+SfaPg=="; assert_eq!( verify_tarball_integrity( &package, @@ -195,7 +195,7 @@ mod test { .to_string(), concat!( "Tarball checksum did not match what was provided by npm ", - "registry for package@1.0.0.\n\nExpected: test\nActual: 2jmj7l5rsw0yvb/vlwaykk/ybwk=", + "registry for package@1.0.0.\n\nExpected: test\nActual: 2jmj7l5rSw0yVb/vlWAYkK/YBwk=", ), ); assert_eq!( diff --git a/cli/npm/mod.rs b/cli/npm/mod.rs index 81f46419ed..761c99dba4 100644 --- a/cli/npm/mod.rs +++ b/cli/npm/mod.rs @@ -1,5 +1,6 @@ // Copyright 2018-2023 the Deno authors. All rights reserved. MIT license. +mod byonm; mod cache_dir; mod common; mod managed; @@ -12,17 +13,18 @@ use deno_core::error::AnyError; use deno_runtime::deno_node::NpmResolver; use deno_semver::package::PackageReq; +pub use self::byonm::CliNpmResolverByonmCreateOptions; pub use self::cache_dir::NpmCacheDir; pub use self::managed::CliNpmResolverManagedCreateOptions; pub use self::managed::CliNpmResolverManagedPackageJsonInstallerOption; pub use self::managed::CliNpmResolverManagedSnapshotOption; pub use self::managed::ManagedCliNpmResolver; +use self::byonm::ByonmCliNpmResolver; + pub enum CliNpmResolverCreateOptions { Managed(CliNpmResolverManagedCreateOptions), - // todo(dsherret): implement this - #[allow(dead_code)] - Byonm, + Byonm(CliNpmResolverByonmCreateOptions), } pub async fn create_cli_npm_resolver_for_lsp( @@ -33,7 +35,7 @@ pub async fn create_cli_npm_resolver_for_lsp( Managed(options) => { managed::create_managed_npm_resolver_for_lsp(options).await } - Byonm => todo!(), + Byonm(options) => byonm::create_byonm_npm_resolver(options), } } @@ -43,7 +45,7 @@ pub async fn create_cli_npm_resolver( use CliNpmResolverCreateOptions::*; match options { Managed(options) => managed::create_managed_npm_resolver(options).await, - Byonm => todo!(), + Byonm(options) => Ok(byonm::create_byonm_npm_resolver(options)), } } @@ -76,12 +78,6 @@ pub trait CliNpmResolver: NpmResolver { fn root_node_modules_path(&self) -> Option; - /// Resolve the root folder of the package the provided specifier is in. - fn resolve_pkg_folder_from_specifier( - &self, - specifier: &ModuleSpecifier, - ) -> Result, AnyError>; - fn resolve_pkg_folder_from_deno_module_req( &self, req: &PackageReq, @@ -95,6 +91,3 @@ pub trait CliNpmResolver: NpmResolver { /// or `None` if the state currently can't be determined. fn check_state_hash(&self) -> Option; } - -// todo(#18967): implement this -pub struct ByonmCliNpmResolver; diff --git a/cli/resolver.rs b/cli/resolver.rs index f48e62aae0..c4037a75a4 100644 --- a/cli/resolver.rs +++ b/cli/resolver.rs @@ -13,7 +13,13 @@ use deno_graph::source::ResolveError; use deno_graph::source::Resolver; use deno_graph::source::UnknownBuiltInNodeModuleError; use deno_graph::source::DEFAULT_JSX_IMPORT_SOURCE_MODULE; +use deno_runtime::deno_fs::FileSystem; use deno_runtime::deno_node::is_builtin_node_module; +use deno_runtime::deno_node::NodeResolution; +use deno_runtime::deno_node::NodeResolutionMode; +use deno_runtime::deno_node::NodeResolver; +use deno_runtime::permissions::PermissionsContainer; +use deno_semver::npm::NpmPackageReqReference; use deno_semver::package::PackageReq; use import_map::ImportMap; use std::path::PathBuf; @@ -22,6 +28,7 @@ use std::sync::Arc; use crate::args::package_json::PackageJsonDeps; use crate::args::JsxImportSourceConfig; use crate::args::PackageJsonDepsProvider; +use crate::module_loader::CjsResolutionStore; use crate::npm::CliNpmResolver; use crate::npm::InnerCliNpmResolverRef; use crate::util::sync::AtomicFlag; @@ -99,16 +106,24 @@ impl MappedSpecifierResolver { /// import map, JSX settings. #[derive(Debug)] pub struct CliGraphResolver { + fs: Arc, mapped_specifier_resolver: MappedSpecifierResolver, maybe_default_jsx_import_source: Option, maybe_jsx_import_source_module: Option, maybe_vendor_specifier: Option, + cjs_resolutions: Option>, + node_resolver: Option>, npm_resolver: Option>, found_package_json_dep_flag: Arc, bare_node_builtins_enabled: bool, } pub struct CliGraphResolverOptions<'a> { + pub fs: Arc, + pub cjs_resolutions: Option>, + pub node_resolver: Option>, + pub npm_resolver: Option>, + pub package_json_deps_provider: Arc, pub maybe_jsx_import_source_config: Option, pub maybe_import_map: Option>, pub maybe_vendor_dir: Option<&'a PathBuf>, @@ -116,15 +131,23 @@ pub struct CliGraphResolverOptions<'a> { } impl CliGraphResolver { - pub fn new( - npm_resolver: Option>, - package_json_deps_provider: Arc, - options: CliGraphResolverOptions, - ) -> Self { + pub fn new(options: CliGraphResolverOptions) -> Self { + let is_byonm = options + .npm_resolver + .as_ref() + .map(|n| n.as_byonm().is_some()) + .unwrap_or(false); Self { + fs: options.fs, + cjs_resolutions: options.cjs_resolutions, mapped_specifier_resolver: MappedSpecifierResolver::new( options.maybe_import_map, - package_json_deps_provider, + if is_byonm { + // don't resolve from the root package.json deps for byonm + Arc::new(PackageJsonDepsProvider::new(None)) + } else { + options.package_json_deps_provider + }, ), maybe_default_jsx_import_source: options .maybe_jsx_import_source_config @@ -136,7 +159,8 @@ impl CliGraphResolver { maybe_vendor_specifier: options .maybe_vendor_dir .and_then(|v| ModuleSpecifier::from_directory_path(v).ok()), - npm_resolver, + node_resolver: options.node_resolver, + npm_resolver: options.npm_resolver, found_package_json_dep_flag: Default::default(), bare_node_builtins_enabled: options.bare_node_builtins_enabled, } @@ -171,8 +195,15 @@ impl Resolver for CliGraphResolver { &self, specifier: &str, referrer: &ModuleSpecifier, - _mode: ResolutionMode, + mode: ResolutionMode, ) -> Result { + fn to_node_mode(mode: ResolutionMode) -> NodeResolutionMode { + match mode { + ResolutionMode::Execution => NodeResolutionMode::Execution, + ResolutionMode::Types => NodeResolutionMode::Types, + } + } + let result = match self .mapped_specifier_resolver .resolve(specifier, referrer)? @@ -200,6 +231,80 @@ impl Resolver for CliGraphResolver { } } + if let Some(resolver) = + self.npm_resolver.as_ref().and_then(|r| r.as_byonm()) + { + match &result { + Ok(specifier) => { + if let Ok(npm_req_ref) = + NpmPackageReqReference::from_specifier(specifier) + { + let package_folder = resolver + .resolve_pkg_folder_from_deno_module_req( + npm_req_ref.req(), + referrer, + )?; + let node_resolver = self.node_resolver.as_ref().unwrap(); + let package_json_path = package_folder.join("package.json"); + if !self.fs.exists_sync(&package_json_path) { + return Err(ResolveError::Other(anyhow!( + "Could not find '{}'. Maybe run `npm install`?", + package_json_path.display() + ))); + } + let maybe_resolution = node_resolver + .resolve_package_subpath_from_deno_module( + &package_folder, + npm_req_ref.sub_path(), + referrer, + to_node_mode(mode), + &PermissionsContainer::allow_all(), + )?; + match maybe_resolution { + Some(resolution) => { + if let Some(cjs_resolutions) = &self.cjs_resolutions { + if let NodeResolution::CommonJs(specifier) = &resolution { + // remember that this was a common js resolution + cjs_resolutions.insert(specifier.clone()); + } + } + + return Ok(resolution.into_url()); + } + None => { + return Err(ResolveError::Other(anyhow!( + "Failed resolving package subpath for '{}' in '{}'.", + npm_req_ref, + package_folder.display() + ))); + } + } + } + } + Err(_) => { + if referrer.scheme() == "file" { + if let Some(node_resolver) = &self.node_resolver { + let node_result = node_resolver.resolve( + specifier, + referrer, + to_node_mode(mode), + &PermissionsContainer::allow_all(), + ); + if let Ok(Some(resolution)) = node_result { + if let Some(cjs_resolutions) = &self.cjs_resolutions { + if let NodeResolution::CommonJs(specifier) = &resolution { + // remember that this was a common js resolution + cjs_resolutions.insert(specifier.clone()); + } + } + return Ok(resolution.into_url()); + } + } + } + } + } + } + result } } diff --git a/cli/tests/integration/cert_tests.rs b/cli/tests/integration/cert_tests.rs index ffd4b449d4..20bf4d80d0 100644 --- a/cli/tests/integration/cert_tests.rs +++ b/cli/tests/integration/cert_tests.rs @@ -118,7 +118,7 @@ fn cafile_compile() { context .new_command() - .command_name(output_exe) + .name(output_exe) .run() .assert_matches_text("[WILDCARD]\nHello\n"); } diff --git a/cli/tests/integration/compile_tests.rs b/cli/tests/integration/compile_tests.rs index 657d17d7d0..3ca57a35f4 100644 --- a/cli/tests/integration/compile_tests.rs +++ b/cli/tests/integration/compile_tests.rs @@ -29,7 +29,7 @@ fn compile_basic() { .run(); output.assert_exit_code(0); output.skip_output_check(); - let output = context.new_command().command_name(&exe).run(); + let output = context.new_command().name(&exe).run(); output.assert_matches_text("Welcome to Deno!\n"); } @@ -42,7 +42,7 @@ fn compile_basic() { .new_command() // it should fail creating this, but still work .env("DENO_DIR", readonly_sub_dir) - .command_name(exe) + .name(exe) .run(); output.assert_matches_text("Welcome to Deno!\n"); } @@ -688,11 +688,7 @@ fn workers_not_in_module_map() { output.assert_exit_code(0); output.skip_output_check(); - let output = context - .new_command() - .command_name(exe) - .env("NO_COLOR", "") - .run(); + let output = context.new_command().name(exe).env("NO_COLOR", "").run(); output.assert_exit_code(1); output.assert_matches_text(concat!( "error: Uncaught (in worker \"\") Module not found: [WILDCARD]", @@ -825,7 +821,7 @@ fn compile_npm_specifiers() { output.assert_exit_code(0); output.skip_output_check(); - let output = context.new_command().command_name(&binary_path).run(); + let output = context.new_command().name(&binary_path).run(); output.assert_matches_text( r#"Node esm importing node cjs =========================== @@ -881,7 +877,7 @@ testing[WILDCARD]this output.assert_exit_code(0); output.skip_output_check(); - let output = context.new_command().command_name(binary_path).run(); + let output = context.new_command().name(binary_path).run(); output.assert_matches_text("2\n"); } @@ -1050,7 +1046,7 @@ fn run_npm_bin_compile_test(opts: RunNpmBinCompileOptions) { }; let output = context .new_command() - .command_name(binary_path) + .name(binary_path) .args_vec(opts.run_args) .run(); output.assert_matches_file(opts.output_file); @@ -1114,6 +1110,6 @@ fn compile_node_modules_symlink_outside() { // run let binary_path = project_dir.join(if cfg!(windows) { "bin.exe" } else { "bin" }); - let output = context.new_command().command_name(binary_path).run(); + let output = context.new_command().name(binary_path).run(); output.assert_matches_file("compile/node_modules_symlink_outside/main.out"); } diff --git a/cli/tests/integration/npm_tests.rs b/cli/tests/integration/npm_tests.rs index ddbc79cec1..c3fe2949c2 100644 --- a/cli/tests/integration/npm_tests.rs +++ b/cli/tests/integration/npm_tests.rs @@ -2219,3 +2219,240 @@ itest!(require_resolve_url_paths { cwd: Some("npm/require_resolve_url/"), copy_temp_dir: Some("npm/require_resolve_url/"), }); + +#[test] +pub fn byonm_cjs_esm_packages() { + let test_context = TestContextBuilder::for_npm() + .env("DENO_UNSTABLE_BYONM", "1") + .use_temp_cwd() + .build(); + let dir = test_context.temp_dir(); + let run_npm = |args: &str| { + test_context + .new_command() + .name("npm") + .args(args) + .run() + .skip_output_check(); + }; + + run_npm("init -y"); + run_npm("install @denotest/esm-basic @denotest/cjs-default-export @denotest/dual-cjs-esm chalk@4 chai@4.3"); + + dir.write( + "main.ts", + r#" +import { getValue, setValue } from "@denotest/esm-basic"; + +setValue(2); +console.log(getValue()); + +import cjsDefault from "@denotest/cjs-default-export"; +console.log(cjsDefault.default()); +console.log(cjsDefault.named()); + +import { getKind } from "@denotest/dual-cjs-esm"; +console.log(getKind()); + + +"#, + ); + let output = test_context.new_command().args("run --check main.ts").run(); + output + .assert_matches_text("Check file:///[WILDCARD]/main.ts\n2\n1\n2\nesm\n"); + + // should not have created the .deno directory + assert!(!dir.path().join("node_modules/.deno").exists()); + + // try chai + dir.write( + "chai.ts", + r#"import { expect } from "chai"; + + const timeout = setTimeout(() => {}, 0); + expect(timeout).to.be.a("number"); + clearTimeout(timeout);"#, + ); + test_context.new_command().args("run chai.ts").run(); + + // try chalk cjs + dir.write( + "chalk.ts", + "import chalk from 'chalk'; console.log(chalk.green('chalk cjs loads'));", + ); + let output = test_context + .new_command() + .args("run --allow-read chalk.ts") + .run(); + output.assert_matches_text("chalk cjs loads\n"); + + // try using an npm specifier for chalk that matches the version we installed + dir.write( + "chalk.ts", + "import chalk from 'npm:chalk@4'; console.log(chalk.green('chalk cjs loads'));", + ); + let output = test_context + .new_command() + .args("run --allow-read chalk.ts") + .run(); + output.assert_matches_text("chalk cjs loads\n"); + + // try with one that doesn't match the package.json + dir.write( + "chalk.ts", + "import chalk from 'npm:chalk@5'; console.log(chalk.green('chalk cjs loads'));", + ); + let output = test_context + .new_command() + .args("run --allow-read chalk.ts") + .run(); + output.assert_matches_text( + r#"error: Could not find a matching package for 'npm:chalk@5' in '[WILDCARD]package.json'. You must specify this as a package.json dependency when the node_modules folder is not managed by Deno. + at file:///[WILDCARD]chalk.ts:1:19 +"#); + output.assert_exit_code(1); +} + +#[test] +pub fn byonm_package_npm_specifier_not_installed_and_invalid_subpath() { + let test_context = TestContextBuilder::for_npm() + .env("DENO_UNSTABLE_BYONM", "1") + .use_temp_cwd() + .build(); + let dir = test_context.temp_dir(); + dir.path().join("package.json").write_json(&json!({ + "dependencies": { + "chalk": "4", + "@denotest/conditional-exports-strict": "1" + } + })); + dir.write( + "main.ts", + "import chalk from 'npm:chalk'; console.log(chalk.green('hi'));", + ); + + // no npm install has been run, so this should give an informative error + let output = test_context.new_command().args("run main.ts").run(); + output.assert_matches_text( + r#"error: Could not find '[WILDCARD]package.json'. Maybe run `npm install`? + at file:///[WILDCARD]/main.ts:1:19 +"#, + ); + output.assert_exit_code(1); + + // now test for an invalid sub path after doing an npm install + dir.write( + "main.ts", + "import 'npm:@denotest/conditional-exports-strict/test';", + ); + + test_context + .new_command() + .name("npm") + .args("install") + .run() + .skip_output_check(); + + let output = test_context.new_command().args("run main.ts").run(); + output.assert_matches_text( + r#"error: Failed resolving package subpath './test' for '[WILDCARD]package.json' + at file:///[WILDCARD]/main.ts:1:8 +"#, + ); + output.assert_exit_code(1); +} + +#[test] +pub fn byonm_npm_workspaces() { + let test_context = TestContextBuilder::for_npm().use_temp_cwd().build(); + let dir = test_context.temp_dir(); + dir.write( + "deno.json", + r#"{ + "unstable": [ + "byonm" + ] + }"#, + ); + + dir.write( + "package.json", + r#"{ + "name": "my-workspace", + "workspaces": [ + "project-a", + "project-b" + ] +} +"#, + ); + + let project_a_dir = dir.path().join("project-a"); + project_a_dir.create_dir_all(); + project_a_dir.join("package.json").write_json(&json!({ + "name": "project-a", + "version": "1.0.0", + "main": "./index.js", + "type": "module", + "dependencies": { + "chai": "^4.2", + "project-b": "^1" + } + })); + project_a_dir.join("index.js").write( + r#" +import { expect } from "chai"; + +const timeout = setTimeout(() => {}, 0); +expect(timeout).to.be.a("number"); +clearTimeout(timeout); + +export function add(a, b) { + return a + b; +} +"#, + ); + project_a_dir + .join("index.d.ts") + .write("export function add(a: number, b: number): number;"); + + let project_b_dir = dir.path().join("project-b"); + project_b_dir.create_dir_all(); + project_b_dir.join("package.json").write_json(&json!({ + "name": "project-b", + "version": "1.0.0", + "type": "module", + "dependencies": { + "@denotest/esm-basic": "^1.0", + } + })); + project_b_dir.join("main.ts").write( + r#" +import { getValue, setValue } from "@denotest/esm-basic"; + +setValue(5); +console.log(getValue()); + +import { add } from "project-a"; +console.log(add(1, 2)); +"#, + ); + + test_context + .new_command() + .name("npm") + .args("install") + .run() + .skip_output_check(); + + let output = test_context + .new_command() + .args("run ./project-b/main.ts") + .run(); + output.assert_matches_text("5\n3\n"); + let output = test_context + .new_command() + .args("check ./project-b/main.ts") + .run(); + output.assert_matches_text("Check file:///[WILDCARD]/project-b/main.ts\n"); +} diff --git a/cli/tests/testdata/npm/registry/@denotest/conditional-exports-strict/1.0.0/cjs/index.cjs b/cli/tests/testdata/npm/registry/@denotest/conditional-exports-strict/1.0.0/cjs/index.cjs new file mode 100644 index 0000000000..16895e48ca --- /dev/null +++ b/cli/tests/testdata/npm/registry/@denotest/conditional-exports-strict/1.0.0/cjs/index.cjs @@ -0,0 +1,3 @@ +module.exports = { + hello: "from cjs" +}; \ No newline at end of file diff --git a/cli/tests/testdata/npm/registry/@denotest/conditional-exports-strict/1.0.0/esm/client/bar.js b/cli/tests/testdata/npm/registry/@denotest/conditional-exports-strict/1.0.0/esm/client/bar.js new file mode 100644 index 0000000000..1474f5d292 --- /dev/null +++ b/cli/tests/testdata/npm/registry/@denotest/conditional-exports-strict/1.0.0/esm/client/bar.js @@ -0,0 +1,3 @@ +export default { + hello: "from esm client bar", +} diff --git a/cli/tests/testdata/npm/registry/@denotest/conditional-exports-strict/1.0.0/esm/client/foo.js b/cli/tests/testdata/npm/registry/@denotest/conditional-exports-strict/1.0.0/esm/client/foo.js new file mode 100644 index 0000000000..bb5284b159 --- /dev/null +++ b/cli/tests/testdata/npm/registry/@denotest/conditional-exports-strict/1.0.0/esm/client/foo.js @@ -0,0 +1,3 @@ +export default { + hello: "from esm client foo", +} diff --git a/cli/tests/testdata/npm/registry/@denotest/conditional-exports-strict/1.0.0/esm/client/index.js b/cli/tests/testdata/npm/registry/@denotest/conditional-exports-strict/1.0.0/esm/client/index.js new file mode 100644 index 0000000000..dc1ec197d4 --- /dev/null +++ b/cli/tests/testdata/npm/registry/@denotest/conditional-exports-strict/1.0.0/esm/client/index.js @@ -0,0 +1,3 @@ +export default { + hello: "from esm client", +} diff --git a/cli/tests/testdata/npm/registry/@denotest/conditional-exports-strict/1.0.0/esm/client/m.js b/cli/tests/testdata/npm/registry/@denotest/conditional-exports-strict/1.0.0/esm/client/m.js new file mode 100644 index 0000000000..fec6807ac2 --- /dev/null +++ b/cli/tests/testdata/npm/registry/@denotest/conditional-exports-strict/1.0.0/esm/client/m.js @@ -0,0 +1,3 @@ +export default { + hello: "from esm client m", +} diff --git a/cli/tests/testdata/npm/registry/@denotest/conditional-exports-strict/1.0.0/esm/index.js b/cli/tests/testdata/npm/registry/@denotest/conditional-exports-strict/1.0.0/esm/index.js new file mode 100644 index 0000000000..38dae7d936 --- /dev/null +++ b/cli/tests/testdata/npm/registry/@denotest/conditional-exports-strict/1.0.0/esm/index.js @@ -0,0 +1,3 @@ +export default { + hello: "from esm", +} \ No newline at end of file diff --git a/cli/tests/testdata/npm/registry/@denotest/conditional-exports-strict/1.0.0/foo.js b/cli/tests/testdata/npm/registry/@denotest/conditional-exports-strict/1.0.0/foo.js new file mode 100644 index 0000000000..6060c8a673 --- /dev/null +++ b/cli/tests/testdata/npm/registry/@denotest/conditional-exports-strict/1.0.0/foo.js @@ -0,0 +1,3 @@ +export default { + hello: "from foo", +} diff --git a/cli/tests/testdata/npm/registry/@denotest/conditional-exports-strict/1.0.0/package.json b/cli/tests/testdata/npm/registry/@denotest/conditional-exports-strict/1.0.0/package.json new file mode 100644 index 0000000000..3576e48f83 --- /dev/null +++ b/cli/tests/testdata/npm/registry/@denotest/conditional-exports-strict/1.0.0/package.json @@ -0,0 +1,16 @@ +{ + "name": "@denotest/conditional-exports-strict", + "version": "1.0.0", + "type": "module", + "exports": { + ".": { + "types": "./types/src/index.d.ts", + "require": "./cjs/index.cjs", + "import": "./esm/index.js" + }, + "./client": { + "types": "./types/src/client/index.d.ts", + "import": "./esm/client/index.js" + } + } +} diff --git a/cli/tests/testdata/npm/registry/@denotest/conditional-exports/1.0.0/esm/client/bar.js b/cli/tests/testdata/npm/registry/@denotest/conditional-exports/1.0.0/esm/client/bar.js index 12352639d4..1474f5d292 100644 --- a/cli/tests/testdata/npm/registry/@denotest/conditional-exports/1.0.0/esm/client/bar.js +++ b/cli/tests/testdata/npm/registry/@denotest/conditional-exports/1.0.0/esm/client/bar.js @@ -1,3 +1,3 @@ export default { hello: "from esm client bar", -} \ No newline at end of file +} diff --git a/cli/tests/testdata/npm/registry/@denotest/conditional-exports/1.0.0/esm/client/foo.js b/cli/tests/testdata/npm/registry/@denotest/conditional-exports/1.0.0/esm/client/foo.js index 1ab5baf1b1..bb5284b159 100644 --- a/cli/tests/testdata/npm/registry/@denotest/conditional-exports/1.0.0/esm/client/foo.js +++ b/cli/tests/testdata/npm/registry/@denotest/conditional-exports/1.0.0/esm/client/foo.js @@ -1,3 +1,3 @@ export default { hello: "from esm client foo", -} \ No newline at end of file +} diff --git a/cli/tests/testdata/npm/registry/@denotest/conditional-exports/1.0.0/esm/client/index.js b/cli/tests/testdata/npm/registry/@denotest/conditional-exports/1.0.0/esm/client/index.js index 86f246be44..dc1ec197d4 100644 --- a/cli/tests/testdata/npm/registry/@denotest/conditional-exports/1.0.0/esm/client/index.js +++ b/cli/tests/testdata/npm/registry/@denotest/conditional-exports/1.0.0/esm/client/index.js @@ -1,3 +1,3 @@ export default { hello: "from esm client", -} \ No newline at end of file +} diff --git a/cli/tests/testdata/npm/registry/@denotest/conditional-exports/1.0.0/esm/client/m.js b/cli/tests/testdata/npm/registry/@denotest/conditional-exports/1.0.0/esm/client/m.js index 40e769031a..fec6807ac2 100644 --- a/cli/tests/testdata/npm/registry/@denotest/conditional-exports/1.0.0/esm/client/m.js +++ b/cli/tests/testdata/npm/registry/@denotest/conditional-exports/1.0.0/esm/client/m.js @@ -1,3 +1,3 @@ export default { hello: "from esm client m", -} \ No newline at end of file +} diff --git a/cli/tests/testdata/npm/registry/@denotest/dual-cjs-esm/1.0.0/main.d.cts b/cli/tests/testdata/npm/registry/@denotest/dual-cjs-esm/1.0.0/main.d.cts new file mode 100644 index 0000000000..f969ba9960 --- /dev/null +++ b/cli/tests/testdata/npm/registry/@denotest/dual-cjs-esm/1.0.0/main.d.cts @@ -0,0 +1 @@ +export function getKind(): string; diff --git a/cli/tests/testdata/npm/registry/@denotest/dual-cjs-esm/1.0.0/main.d.mts b/cli/tests/testdata/npm/registry/@denotest/dual-cjs-esm/1.0.0/main.d.mts new file mode 100644 index 0000000000..f969ba9960 --- /dev/null +++ b/cli/tests/testdata/npm/registry/@denotest/dual-cjs-esm/1.0.0/main.d.mts @@ -0,0 +1 @@ +export function getKind(): string; diff --git a/cli/tools/vendor/test.rs b/cli/tools/vendor/test.rs index 810daf4fb4..73a4324cfa 100644 --- a/cli/tools/vendor/test.rs +++ b/cli/tools/vendor/test.rs @@ -19,6 +19,7 @@ use deno_graph::source::LoadResponse; use deno_graph::source::Loader; use deno_graph::GraphKind; use deno_graph::ModuleGraph; +use deno_runtime::deno_fs::RealFs; use import_map::ImportMap; use crate::args::JsxImportSourceConfig; @@ -293,16 +294,17 @@ fn build_resolver( maybe_jsx_import_source_config: Option, original_import_map: Option, ) -> CliGraphResolver { - CliGraphResolver::new( - None, - Default::default(), - CliGraphResolverOptions { - maybe_jsx_import_source_config, - maybe_import_map: original_import_map.map(Arc::new), - maybe_vendor_dir: None, - bare_node_builtins_enabled: false, - }, - ) + CliGraphResolver::new(CliGraphResolverOptions { + fs: Arc::new(RealFs), + node_resolver: None, + npm_resolver: None, + cjs_resolutions: None, + package_json_deps_provider: Default::default(), + maybe_jsx_import_source_config, + maybe_import_map: original_import_map.map(Arc::new), + maybe_vendor_dir: None, + bare_node_builtins_enabled: false, + }) } async fn build_test_graph( diff --git a/test_util/src/builders.rs b/test_util/src/builders.rs index 17871baa1d..39771e0883 100644 --- a/test_util/src/builders.rs +++ b/test_util/src/builders.rs @@ -227,7 +227,7 @@ pub struct TestCommandBuilder { } impl TestCommandBuilder { - pub fn command_name(mut self, name: impl AsRef) -> Self { + pub fn name(mut self, name: impl AsRef) -> Self { self.command_name = name.as_ref().to_string_lossy().to_string(); self } @@ -306,7 +306,11 @@ impl TestCommandBuilder { } fn build_command_path(&self) -> PathRef { - let command_name = &self.command_name; + let command_name = if cfg!(windows) && self.command_name == "npm" { + "npm.cmd" + } else { + &self.command_name + }; if command_name == "deno" { deno_exe_path() } else { @@ -407,11 +411,11 @@ impl TestCommandBuilder { command.env_clear(); } command.env("DENO_DIR", self.context.deno_dir.path()); - let envs = self.build_envs(); + let mut envs = self.build_envs(); if !envs.contains_key("NPM_CONFIG_REGISTRY") { - command.env("NPM_CONFIG_REGISTRY", npm_registry_unset_url()); + envs.insert("NPM_CONFIG_REGISTRY".to_string(), npm_registry_unset_url()); } - command.envs(self.build_envs()); + command.envs(envs); command.current_dir(cwd); command.stdin(Stdio::piped()); @@ -527,6 +531,7 @@ impl Drop for TestCommandOutput { // now ensure the exit code was asserted if !*self.asserted_exit_code.borrow() && self.exit_code != Some(0) { + self.print_output(); panic!( "The non-zero exit code of the command was not asserted: {:?}", self.exit_code, diff --git a/test_util/src/lib.rs b/test_util/src/lib.rs index 07ed558227..9f764007cd 100644 --- a/test_util/src/lib.rs +++ b/test_util/src/lib.rs @@ -37,7 +37,9 @@ use std::env; use std::io; use std::io::Write; use std::mem::replace; +use std::net::Ipv6Addr; use std::net::SocketAddr; +use std::net::SocketAddrV6; use std::ops::Deref; use std::ops::DerefMut; use std::path::Path; @@ -1316,15 +1318,18 @@ async fn main_server( } _ => { let mut file_path = testdata_path().to_path_buf(); - file_path.push(&req.uri().path()[1..]); + file_path.push(&req.uri().path()[1..].replace("%2f", "/")); if let Ok(file) = tokio::fs::read(&file_path).await { let file_resp = custom_headers(req.uri().path(), file); return Ok(file_resp); } // serve npm registry files - if let Some(suffix) = - req.uri().path().strip_prefix("/npm/registry/@denotest/") + if let Some(suffix) = req + .uri() + .path() + .strip_prefix("/npm/registry/@denotest/") + .or_else(|| req.uri().path().strip_prefix("/npm/registry/@denotest%2f")) { // serve all requests to /npm/registry/@deno using the file system // at that path @@ -1571,10 +1576,22 @@ async fn wrap_abs_redirect_server() { } async fn wrap_main_server() { + let main_server_addr = SocketAddr::from(([127, 0, 0, 1], PORT)); + wrap_main_server_for_addr(&main_server_addr).await +} + +// necessary because on Windows the npm binary will resolve localhost to ::1 +async fn wrap_main_ipv6_server() { + let ipv6_loopback = Ipv6Addr::new(0, 0, 0, 0, 0, 0, 0, 1); + let main_server_addr = + SocketAddr::V6(SocketAddrV6::new(ipv6_loopback, PORT, 0, 0)); + wrap_main_server_for_addr(&main_server_addr).await +} + +async fn wrap_main_server_for_addr(main_server_addr: &SocketAddr) { let main_server_svc = make_service_fn(|_| async { Ok::<_, Infallible>(service_fn(main_server)) }); - let main_server_addr = SocketAddr::from(([127, 0, 0, 1], PORT)); - let main_server = Server::bind(&main_server_addr).serve(main_server_svc); + let main_server = Server::bind(main_server_addr).serve(main_server_svc); if let Err(e) = main_server.await { eprintln!("HTTP server error: {e:?}"); } @@ -1922,6 +1939,7 @@ pub async fn run_all_servers() { let tls_client_auth_server_fut = run_tls_client_auth_server(); let client_auth_server_https_fut = wrap_client_auth_https_server(); let main_server_fut = wrap_main_server(); + let main_server_ipv6_fut = wrap_main_ipv6_server(); let main_server_https_fut = wrap_main_https_server(); let h1_only_server_tls_fut = wrap_https_h1_only_tls_server(); let h2_only_server_tls_fut = wrap_https_h2_only_tls_server(); @@ -1945,6 +1963,7 @@ pub async fn run_all_servers() { double_redirects_server_fut, abs_redirect_server_fut, main_server_fut, + main_server_ipv6_fut, main_server_https_fut, client_auth_server_https_fut, h1_only_server_tls_fut, diff --git a/test_util/src/npm.rs b/test_util/src/npm.rs index 98308ae210..9cbadad5cb 100644 --- a/test_util/src/npm.rs +++ b/test_util/src/npm.rs @@ -104,7 +104,7 @@ fn get_npm_package(package_name: &str) -> Result> { let mut hash_ctx = Context::new(&SHA512); hash_ctx.update(&tarball_bytes); let digest = hash_ctx.finish(); - let tarball_checksum = base64::encode(digest.as_ref()).to_lowercase(); + let tarball_checksum = base64::encode(digest.as_ref()); // create the registry file JSON for this version let mut dist = serde_json::Map::new(); diff --git a/tools/lint.js b/tools/lint.js index 396a68767e..4f10ec5324 100755 --- a/tools/lint.js +++ b/tools/lint.js @@ -21,7 +21,7 @@ if (rs) { promises.push(clippy()); } -if (!js && !rs) { +if (js && rs) { promises.push(checkCopyright()); }