mirror of
https://github.com/denoland/deno
synced 2024-08-27 12:00:17 +00:00
fix(unstable): disable importing from the vendor directory (#20067)
Some people might get think they need to import from this directory, which could cause confusion and duplicate dependencies. Additionally, the `vendor` directory has special behaviour in the language server, so importing from the folder will definitely cause confusion and issues there.
This commit is contained in:
parent
6082e51094
commit
f343391a9f
|
@ -38,6 +38,7 @@ use crate::npm::NpmPackageFsResolver;
|
|||
use crate::npm::NpmResolution;
|
||||
use crate::npm::PackageJsonDepsInstaller;
|
||||
use crate::resolver::CliGraphResolver;
|
||||
use crate::resolver::CliGraphResolverOptions;
|
||||
use crate::standalone::DenoCompileBinaryWriter;
|
||||
use crate::tools::check::TypeChecker;
|
||||
use crate::util::progress_bar::ProgressBar;
|
||||
|
@ -424,13 +425,18 @@ impl CliFactory {
|
|||
.resolver
|
||||
.get_or_try_init_async(async {
|
||||
Ok(Arc::new(CliGraphResolver::new(
|
||||
self.options.to_maybe_jsx_import_source_config()?,
|
||||
self.maybe_import_map().await?.clone(),
|
||||
self.options.no_npm(),
|
||||
self.npm_api()?.clone(),
|
||||
self.npm_resolution().await?.clone(),
|
||||
self.package_json_deps_provider().clone(),
|
||||
self.package_json_deps_installer().await?.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(),
|
||||
no_npm: self.options.no_npm(),
|
||||
},
|
||||
)))
|
||||
})
|
||||
.await
|
||||
|
|
|
@ -21,6 +21,7 @@ use crate::npm::CliNpmRegistryApi;
|
|||
use crate::npm::NpmResolution;
|
||||
use crate::npm::PackageJsonDepsInstaller;
|
||||
use crate::resolver::CliGraphResolver;
|
||||
use crate::resolver::CliGraphResolverOptions;
|
||||
use crate::util::glob;
|
||||
use crate::util::path::specifier_to_file_path;
|
||||
use crate::util::text_encoding;
|
||||
|
@ -1241,13 +1242,19 @@ impl Documents {
|
|||
Arc::new(PackageJsonDepsProvider::new(maybe_package_json_deps));
|
||||
let deps_installer = Arc::new(PackageJsonDepsInstaller::no_op());
|
||||
self.resolver = Arc::new(CliGraphResolver::new(
|
||||
maybe_jsx_config,
|
||||
options.maybe_import_map,
|
||||
false,
|
||||
options.npm_registry_api,
|
||||
options.npm_resolution,
|
||||
deps_provider,
|
||||
deps_installer,
|
||||
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(),
|
||||
no_npm: false,
|
||||
},
|
||||
));
|
||||
self.imports = Arc::new(
|
||||
if let Some(Ok(imports)) =
|
||||
|
|
|
@ -1,6 +1,7 @@
|
|||
// Copyright 2018-2023 the Deno authors. All rights reserved. MIT license.
|
||||
|
||||
use deno_core::anyhow::anyhow;
|
||||
use deno_core::anyhow::bail;
|
||||
use deno_core::error::AnyError;
|
||||
use deno_core::futures::future;
|
||||
use deno_core::futures::future::LocalBoxFuture;
|
||||
|
@ -16,6 +17,7 @@ use deno_npm::registry::NpmRegistryApi;
|
|||
use deno_runtime::deno_node::is_builtin_node_module;
|
||||
use deno_semver::npm::NpmPackageReq;
|
||||
use import_map::ImportMap;
|
||||
use std::path::PathBuf;
|
||||
use std::sync::Arc;
|
||||
|
||||
use crate::args::package_json::PackageJsonDeps;
|
||||
|
@ -102,6 +104,7 @@ pub struct CliGraphResolver {
|
|||
mapped_specifier_resolver: MappedSpecifierResolver,
|
||||
maybe_default_jsx_import_source: Option<String>,
|
||||
maybe_jsx_import_source_module: Option<String>,
|
||||
maybe_vendor_specifier: Option<ModuleSpecifier>,
|
||||
no_npm: bool,
|
||||
npm_registry_api: Arc<CliNpmRegistryApi>,
|
||||
npm_resolution: Arc<NpmResolution>,
|
||||
|
@ -125,8 +128,9 @@ impl Default for CliGraphResolver {
|
|||
maybe_import_map: Default::default(),
|
||||
package_json_deps_provider: Default::default(),
|
||||
},
|
||||
maybe_default_jsx_import_source: Default::default(),
|
||||
maybe_jsx_import_source_module: Default::default(),
|
||||
maybe_default_jsx_import_source: None,
|
||||
maybe_jsx_import_source_module: None,
|
||||
maybe_vendor_specifier: None,
|
||||
no_npm: false,
|
||||
npm_registry_api,
|
||||
npm_resolution,
|
||||
|
@ -137,27 +141,37 @@ impl Default for CliGraphResolver {
|
|||
}
|
||||
}
|
||||
|
||||
pub struct CliGraphResolverOptions<'a> {
|
||||
pub maybe_jsx_import_source_config: Option<JsxImportSourceConfig>,
|
||||
pub maybe_import_map: Option<Arc<ImportMap>>,
|
||||
pub maybe_vendor_dir: Option<&'a PathBuf>,
|
||||
pub no_npm: bool,
|
||||
}
|
||||
|
||||
impl CliGraphResolver {
|
||||
pub fn new(
|
||||
maybe_jsx_import_source_config: Option<JsxImportSourceConfig>,
|
||||
maybe_import_map: Option<Arc<ImportMap>>,
|
||||
no_npm: bool,
|
||||
npm_registry_api: Arc<CliNpmRegistryApi>,
|
||||
npm_resolution: Arc<NpmResolution>,
|
||||
package_json_deps_provider: Arc<PackageJsonDepsProvider>,
|
||||
package_json_deps_installer: Arc<PackageJsonDepsInstaller>,
|
||||
options: CliGraphResolverOptions,
|
||||
) -> Self {
|
||||
Self {
|
||||
mapped_specifier_resolver: MappedSpecifierResolver {
|
||||
maybe_import_map,
|
||||
maybe_import_map: options.maybe_import_map,
|
||||
package_json_deps_provider,
|
||||
},
|
||||
maybe_default_jsx_import_source: maybe_jsx_import_source_config
|
||||
maybe_default_jsx_import_source: options
|
||||
.maybe_jsx_import_source_config
|
||||
.as_ref()
|
||||
.and_then(|c| c.default_specifier.clone()),
|
||||
maybe_jsx_import_source_module: maybe_jsx_import_source_config
|
||||
maybe_jsx_import_source_module: options
|
||||
.maybe_jsx_import_source_config
|
||||
.map(|c| c.module),
|
||||
no_npm,
|
||||
maybe_vendor_specifier: options
|
||||
.maybe_vendor_dir
|
||||
.and_then(|v| ModuleSpecifier::from_directory_path(v).ok()),
|
||||
no_npm: options.no_npm,
|
||||
npm_registry_api,
|
||||
npm_resolution,
|
||||
package_json_deps_installer,
|
||||
|
@ -219,7 +233,7 @@ impl Resolver for CliGraphResolver {
|
|||
referrer: &ModuleSpecifier,
|
||||
) -> Result<ModuleSpecifier, AnyError> {
|
||||
use MappedResolution::*;
|
||||
match self
|
||||
let result = match self
|
||||
.mapped_specifier_resolver
|
||||
.resolve(specifier, referrer)?
|
||||
{
|
||||
|
@ -232,10 +246,24 @@ impl Resolver for CliGraphResolver {
|
|||
}
|
||||
None => deno_graph::resolve_import(specifier, referrer)
|
||||
.map_err(|err| err.into()),
|
||||
};
|
||||
|
||||
// When the user is vendoring, don't allow them to import directly from the vendor/ directory
|
||||
// as it might cause them confusion or duplicate dependencies. Additionally, this folder has
|
||||
// special treatment in the language server so it will definitely cause issues/confusion there
|
||||
// if they do this.
|
||||
if let Some(vendor_specifier) = &self.maybe_vendor_specifier {
|
||||
if let Ok(specifier) = &result {
|
||||
if specifier.as_str().starts_with(vendor_specifier.as_str()) {
|
||||
bail!("Importing from the vendor directory is not permitted. Use a remote specifier instead or disable vendoring.");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
result
|
||||
}
|
||||
}
|
||||
|
||||
fn resolve_package_json_dep(
|
||||
specifier: &str,
|
||||
deps: &PackageJsonDeps,
|
||||
|
|
|
@ -8958,5 +8958,47 @@ fn lsp_vendor_dir() {
|
|||
);
|
||||
assert_eq!(diagnostics.all().len(), 2);
|
||||
|
||||
// now try doing a relative import into the vendor directory
|
||||
client.write_notification(
|
||||
"textDocument/didChange",
|
||||
json!({
|
||||
"textDocument": {
|
||||
"uri": local_file_uri,
|
||||
"version": 2
|
||||
},
|
||||
"contentChanges": [
|
||||
{
|
||||
"range": {
|
||||
"start": { "line": 0, "character": 0 },
|
||||
"end": { "line": 2, "character": 0 },
|
||||
},
|
||||
"text": "import { returnsHi } from './vendor/subdir/mod1.ts';\nconst test: string = returnsHi();\nconsole.log(test);"
|
||||
}
|
||||
]
|
||||
}),
|
||||
);
|
||||
|
||||
let diagnostics = client.read_diagnostics();
|
||||
|
||||
assert_eq!(
|
||||
json!(
|
||||
diagnostics
|
||||
.messages_with_file_and_source(local_file_uri.as_str(), "deno")
|
||||
.diagnostics
|
||||
),
|
||||
json!([
|
||||
{
|
||||
"range": {
|
||||
"start": { "line": 0, "character": 26 },
|
||||
"end": { "line": 0, "character": 51 }
|
||||
},
|
||||
"severity": 1,
|
||||
"code": "resolver-error",
|
||||
"source": "deno",
|
||||
"message": "Importing from the vendor directory is not permitted. Use a remote specifier instead or disable vendoring."
|
||||
}
|
||||
]),
|
||||
);
|
||||
|
||||
client.shutdown();
|
||||
}
|
||||
|
|
|
@ -4580,4 +4580,17 @@ console.log(returnsHi());"#,
|
|||
.args("run --vendor http://localhost:4545/subdir/CAPITALS/hello_there.ts")
|
||||
.run()
|
||||
.assert_matches_text("hello there\n");
|
||||
|
||||
// now try importing directly from the vendor folder
|
||||
temp_dir.write(
|
||||
"main.ts",
|
||||
r#"import { returnsHi } from './vendor/http_localhost_4545/subdir/mod1.ts';
|
||||
console.log(returnsHi());"#,
|
||||
);
|
||||
deno_run_cmd
|
||||
.run()
|
||||
.assert_matches_text("error: Importing from the vendor directory is not permitted. Use a remote specifier instead or disable vendoring.
|
||||
at [WILDCARD]/main.ts:1:27
|
||||
")
|
||||
.assert_exit_code(1);
|
||||
}
|
||||
|
|
12
cli/tools/vendor/test.rs
vendored
12
cli/tools/vendor/test.rs
vendored
|
@ -26,6 +26,7 @@ use crate::cache::ParsedSourceCache;
|
|||
use crate::npm::CliNpmRegistryApi;
|
||||
use crate::npm::NpmResolution;
|
||||
use crate::resolver::CliGraphResolver;
|
||||
use crate::resolver::CliGraphResolverOptions;
|
||||
|
||||
use super::build::VendorEnvironment;
|
||||
|
||||
|
@ -290,7 +291,7 @@ impl VendorTestBuilder {
|
|||
}
|
||||
|
||||
fn build_resolver(
|
||||
jsx_import_source_config: Option<JsxImportSourceConfig>,
|
||||
maybe_jsx_import_source_config: Option<JsxImportSourceConfig>,
|
||||
original_import_map: Option<ImportMap>,
|
||||
) -> CliGraphResolver {
|
||||
let npm_registry_api = Arc::new(CliNpmRegistryApi::new_uninitialized());
|
||||
|
@ -300,13 +301,16 @@ fn build_resolver(
|
|||
None,
|
||||
));
|
||||
CliGraphResolver::new(
|
||||
jsx_import_source_config,
|
||||
original_import_map.map(Arc::new),
|
||||
false,
|
||||
npm_registry_api,
|
||||
npm_resolution,
|
||||
Default::default(),
|
||||
Default::default(),
|
||||
CliGraphResolverOptions {
|
||||
maybe_jsx_import_source_config,
|
||||
maybe_import_map: original_import_map.map(Arc::new),
|
||||
maybe_vendor_dir: None,
|
||||
no_npm: false,
|
||||
},
|
||||
)
|
||||
}
|
||||
|
||||
|
|
|
@ -998,6 +998,7 @@ impl CollectedDiagnostics {
|
|||
.unwrap()
|
||||
}
|
||||
|
||||
#[track_caller]
|
||||
pub fn messages_with_file_and_source(
|
||||
&self,
|
||||
specifier: &str,
|
||||
|
|
Loading…
Reference in a new issue