summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorBartek IwaƄczuk <biwanczuk@gmail.com>2023-02-22 23:21:05 +0100
committerGitHub <noreply@github.com>2023-02-22 23:21:05 +0100
commit1c14127c4f54d815b3e1be48bddd5198dcb33a50 (patch)
tree56dbbea5e9a39fc95d3c722811b47bce08e21b59
parentc18e0d1d37878bb4441f7f8d339cc23ac8e68448 (diff)
feat: support bare specifier resolution with package.json (#17864)
This commit enables resolution of "bare specifiers" (eg. "import express from 'express';") if a "package.json" file is discovered. It's a step towards being able to run projects authored for Node.js without any changes. With this commit we are able to successfully run Vite projects without any changes to the user code. --------- Co-authored-by: David Sherret <dsherret@gmail.com>
-rw-r--r--cli/cache/mod.rs14
-rw-r--r--cli/graph_util.rs5
-rw-r--r--cli/lsp/language_server.rs2
-rw-r--r--cli/proc_state.rs18
-rw-r--r--cli/resolver.rs19
-rw-r--r--cli/tests/integration/run_tests.rs3
-rw-r--r--cli/tests/testdata/run/with_package_json/no_deno_json/main.out6
-rw-r--r--cli/tests/testdata/run/with_package_json/no_deno_json/main.ts6
-rw-r--r--cli/tests/testdata/run/with_package_json/no_deno_json/package.json3
-rw-r--r--cli/tests/testdata/run/with_package_json/with_stop/main.out3
-rw-r--r--cli/tests/testdata/run/with_package_json/with_stop/some/nested/dir/main.ts8
-rw-r--r--cli/tools/info.rs2
12 files changed, 63 insertions, 26 deletions
diff --git a/cli/cache/mod.rs b/cli/cache/mod.rs
index eead9c419..6c50c4c8b 100644
--- a/cli/cache/mod.rs
+++ b/cli/cache/mod.rs
@@ -3,6 +3,7 @@
use crate::errors::get_error_class_name;
use crate::file_fetcher::FileFetcher;
+use deno_core::futures;
use deno_core::futures::FutureExt;
use deno_core::ModuleSpecifier;
use deno_graph::source::CacheInfo;
@@ -44,6 +45,7 @@ pub struct FetchCacher {
file_fetcher: Arc<FileFetcher>,
root_permissions: PermissionsContainer,
cache_info_enabled: bool,
+ maybe_local_node_modules_url: Option<ModuleSpecifier>,
}
impl FetchCacher {
@@ -52,6 +54,7 @@ impl FetchCacher {
file_fetcher: Arc<FileFetcher>,
root_permissions: PermissionsContainer,
dynamic_permissions: PermissionsContainer,
+ maybe_local_node_modules_url: Option<ModuleSpecifier>,
) -> Self {
Self {
emit_cache,
@@ -59,6 +62,7 @@ impl FetchCacher {
file_fetcher,
root_permissions,
cache_info_enabled: false,
+ maybe_local_node_modules_url,
}
}
@@ -96,6 +100,16 @@ impl Loader for FetchCacher {
specifier: &ModuleSpecifier,
is_dynamic: bool,
) -> LoadFuture {
+ if let Some(node_modules_url) = self.maybe_local_node_modules_url.as_ref() {
+ if specifier.as_str().starts_with(node_modules_url.as_str()) {
+ return Box::pin(futures::future::ready(Ok(Some(
+ LoadResponse::External {
+ specifier: specifier.clone(),
+ },
+ ))));
+ }
+ }
+
let permissions = if is_dynamic {
self.dynamic_permissions.clone()
} else {
diff --git a/cli/graph_util.rs b/cli/graph_util.rs
index ef1e0f59a..b5726b943 100644
--- a/cli/graph_util.rs
+++ b/cli/graph_util.rs
@@ -14,6 +14,7 @@ use crate::resolver::CliGraphResolver;
use crate::tools::check;
use deno_core::anyhow::bail;
+use deno_core::anyhow::Context;
use deno_core::error::custom_error;
use deno_core::error::AnyError;
use deno_core::ModuleSpecifier;
@@ -152,6 +153,10 @@ pub async fn create_graph_and_maybe_check(
ps.file_fetcher.clone(),
PermissionsContainer::allow_all(),
PermissionsContainer::allow_all(),
+ ps.options
+ .resolve_local_node_modules_folder()
+ .with_context(|| "Resolving local node_modules folder.")?
+ .map(|path| ModuleSpecifier::from_file_path(path).unwrap()),
);
let maybe_imports = ps.options.to_maybe_imports()?;
let maybe_package_json_deps = ps.options.maybe_package_json_deps()?;
diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs
index 37c36e228..d056afbc0 100644
--- a/cli/lsp/language_server.rs
+++ b/cli/lsp/language_server.rs
@@ -168,7 +168,7 @@ impl LanguageServer {
.map(|d| (d.specifier().clone(), d))
.collect::<HashMap<_, _>>();
let ps = ProcState::from_options(Arc::new(cli_options)).await?;
- let mut inner_loader = ps.create_graph_loader();
+ let mut inner_loader = ps.create_graph_loader()?;
let mut loader = crate::lsp::documents::OpenDocumentsGraphLoader {
inner_loader: &mut inner_loader,
open_docs: &open_docs,
diff --git a/cli/proc_state.rs b/cli/proc_state.rs
index 86af11ec3..9f1e7320c 100644
--- a/cli/proc_state.rs
+++ b/cli/proc_state.rs
@@ -329,6 +329,11 @@ impl ProcState {
self.file_fetcher.clone(),
root_permissions,
dynamic_permissions,
+ self
+ .options
+ .resolve_local_node_modules_folder()
+ .with_context(|| "Resolving local node_modules folder.")?
+ .map(|path| ModuleSpecifier::from_file_path(path).unwrap()),
);
let maybe_imports = self.options.to_maybe_imports()?;
let graph_resolver = self.resolver.as_graph_resolver();
@@ -627,20 +632,25 @@ impl ProcState {
}
/// Creates the default loader used for creating a graph.
- pub fn create_graph_loader(&self) -> cache::FetchCacher {
- cache::FetchCacher::new(
+ pub fn create_graph_loader(&self) -> Result<cache::FetchCacher, AnyError> {
+ Ok(cache::FetchCacher::new(
self.emit_cache.clone(),
self.file_fetcher.clone(),
PermissionsContainer::allow_all(),
PermissionsContainer::allow_all(),
- )
+ self
+ .options
+ .resolve_local_node_modules_folder()
+ .with_context(|| "Resolving local node_modules folder.")?
+ .map(|path| ModuleSpecifier::from_file_path(path).unwrap()),
+ ))
}
pub async fn create_graph(
&self,
roots: Vec<ModuleSpecifier>,
) -> Result<deno_graph::ModuleGraph, AnyError> {
- let mut cache = self.create_graph_loader();
+ let mut cache = self.create_graph_loader()?;
self.create_graph_with_loader(roots, &mut cache).await
}
diff --git a/cli/resolver.rs b/cli/resolver.rs
index 76e1715b5..ce0d5d5cf 100644
--- a/cli/resolver.rs
+++ b/cli/resolver.rs
@@ -26,9 +26,6 @@ use crate::npm::NpmResolution;
#[derive(Debug, Clone)]
pub struct CliGraphResolver {
maybe_import_map: Option<Arc<ImportMap>>,
- // TODO(bartlomieju): actually use in `resolver`, once
- // deno_graph refactors and upgrades land.
- #[allow(dead_code)]
maybe_package_json_deps: Option<HashMap<String, NpmPackageReq>>,
maybe_default_jsx_import_source: Option<String>,
maybe_jsx_import_source_module: Option<String>,
@@ -116,15 +113,19 @@ impl Resolver for CliGraphResolver {
specifier: &str,
referrer: &ModuleSpecifier,
) -> Result<ModuleSpecifier, AnyError> {
- // TODO(bartlomieju): actually use `maybe_package_json_deps` here, once
- // deno_graph refactors and upgrades land.
if let Some(import_map) = &self.maybe_import_map {
- import_map
+ return import_map
.resolve(specifier, referrer)
- .map_err(|err| err.into())
- } else {
- deno_graph::resolve_import(specifier, referrer).map_err(|err| err.into())
+ .map_err(|err| err.into());
+ }
+
+ if let Some(deps) = self.maybe_package_json_deps.as_ref() {
+ if let Some(req) = deps.get(specifier) {
+ return Ok(ModuleSpecifier::parse(&format!("npm:{req}")).unwrap());
+ }
}
+
+ deno_graph::resolve_import(specifier, referrer).map_err(|err| err.into())
}
}
diff --git a/cli/tests/integration/run_tests.rs b/cli/tests/integration/run_tests.rs
index 10ff6d2e0..f30e7ce69 100644
--- a/cli/tests/integration/run_tests.rs
+++ b/cli/tests/integration/run_tests.rs
@@ -2750,7 +2750,7 @@ itest!(config_not_auto_discovered_for_remote_script {
});
itest!(package_json_auto_discovered_for_local_script_log {
- args: "run -L debug no_deno_json/main.ts",
+ args: "run -L debug -A no_deno_json/main.ts",
output: "run/with_package_json/no_deno_json/main.out",
maybe_cwd: Some("run/with_package_json/"),
envs: env_vars_for_npm_tests_no_sync_download(),
@@ -2766,6 +2766,7 @@ itest!(
maybe_cwd: Some("run/with_package_json/"),
envs: env_vars_for_npm_tests_no_sync_download(),
http_server: true,
+ exit_code: 1,
}
);
diff --git a/cli/tests/testdata/run/with_package_json/no_deno_json/main.out b/cli/tests/testdata/run/with_package_json/no_deno_json/main.out
index c0dab77d0..a41c8787a 100644
--- a/cli/tests/testdata/run/with_package_json/no_deno_json/main.out
+++ b/cli/tests/testdata/run/with_package_json/no_deno_json/main.out
@@ -1,3 +1,9 @@
[WILDCARD]package.json file found at '[WILDCARD]with_package_json[WILDCARD]package.json'
[WILDCARD]
ok
+[Chalk] {
+ constructor: [Function],
+ Instance: [Class: ChalkClass],
+ supportsColor: false,
+ stderr: [Chalk] { constructor: [Function], Instance: [Class: ChalkClass], supportsColor: false }
+}
diff --git a/cli/tests/testdata/run/with_package_json/no_deno_json/main.ts b/cli/tests/testdata/run/with_package_json/no_deno_json/main.ts
index daefa8f60..1e6e50040 100644
--- a/cli/tests/testdata/run/with_package_json/no_deno_json/main.ts
+++ b/cli/tests/testdata/run/with_package_json/no_deno_json/main.ts
@@ -1,6 +1,4 @@
-// TODO(bartlomieju): currently we don't support actual bare specifier
-// imports; this will be done in a follow up PR.
-// import express from "express";
+import chalk from "chalk";
-// console.log(express);
console.log("ok");
+console.log(chalk);
diff --git a/cli/tests/testdata/run/with_package_json/no_deno_json/package.json b/cli/tests/testdata/run/with_package_json/no_deno_json/package.json
index 9ee3f39a8..a85b890a8 100644
--- a/cli/tests/testdata/run/with_package_json/no_deno_json/package.json
+++ b/cli/tests/testdata/run/with_package_json/no_deno_json/package.json
@@ -1,6 +1,7 @@
{
"dependencies": {
- "@denotest/check-error": "1.0.0"
+ "@denotest/check-error": "1.0.0",
+ "chalk": "4"
},
"devDependencies": {
"@denotest/cjs-default-export": "1.0.0"
diff --git a/cli/tests/testdata/run/with_package_json/with_stop/main.out b/cli/tests/testdata/run/with_package_json/with_stop/main.out
index e7ef053e4..b199faf8d 100644
--- a/cli/tests/testdata/run/with_package_json/with_stop/main.out
+++ b/cli/tests/testdata/run/with_package_json/with_stop/main.out
@@ -1,4 +1,5 @@
[WILDCARD]Config file found at '[WILDCARD]with_package_json[WILDCARD]with_stop[WILDCARD]some[WILDCARD]nested[WILDCARD]deno.json'
[WILDCARD]No package.json file found
[WILDCARD]
-ok
+error: Relative import path "chalk" not prefixed with / or ./ or ../
+ at file:///[WILDCARD]with_package_json/with_stop/some/nested/dir/main.ts:3:19
diff --git a/cli/tests/testdata/run/with_package_json/with_stop/some/nested/dir/main.ts b/cli/tests/testdata/run/with_package_json/with_stop/some/nested/dir/main.ts
index daefa8f60..6016470a1 100644
--- a/cli/tests/testdata/run/with_package_json/with_stop/some/nested/dir/main.ts
+++ b/cli/tests/testdata/run/with_package_json/with_stop/some/nested/dir/main.ts
@@ -1,6 +1,6 @@
-// TODO(bartlomieju): currently we don't support actual bare specifier
-// imports; this will be done in a follow up PR.
-// import express from "express";
+// This import should fail, because `package.json` is not discovered, as we're
+// stopping the discovery when encountering `deno.json`.
+import chalk from "chalk";
-// console.log(express);
console.log("ok");
+console.log(chalk);
diff --git a/cli/tools/info.rs b/cli/tools/info.rs
index 8a7f4b6b9..d120cb89f 100644
--- a/cli/tools/info.rs
+++ b/cli/tools/info.rs
@@ -34,7 +34,7 @@ pub async fn info(flags: Flags, info_flags: InfoFlags) -> Result<(), AnyError> {
let ps = ProcState::build(flags).await?;
if let Some(specifier) = info_flags.file {
let specifier = resolve_url_or_path(&specifier)?;
- let mut loader = ps.create_graph_loader();
+ let mut loader = ps.create_graph_loader()?;
loader.enable_loading_cache_info(); // for displaying the cache information
let graph = ps
.create_graph_with_loader(vec![specifier], &mut loader)