Skip to content

Commit

Permalink
feat(unstable): rename deno_modules to vendor (denoland#20065)
Browse files Browse the repository at this point in the history
Renames the unstable `deno_modules` directory and corresponding settings
to `vendor` after feedback. Also causes the vendoring of the
`node_modules` directory which can be disabled via
`--node-modules-dir=false` or `"nodeModulesDir": false`.
  • Loading branch information
dsherret authored Aug 7, 2023
1 parent 7b5bc87 commit b9b0386
Show file tree
Hide file tree
Showing 16 changed files with 126 additions and 114 deletions.
18 changes: 9 additions & 9 deletions cli/args/config_file.rs
Original file line number Diff line number Diff line change
Expand Up @@ -673,7 +673,7 @@ pub struct ConfigFileJson {
pub lock: Option<Value>,
pub exclude: Option<Value>,
pub node_modules_dir: Option<bool>,
pub deno_modules_dir: Option<bool>,
pub vendor: Option<bool>,
}

#[derive(Clone, Debug)]
Expand Down Expand Up @@ -855,24 +855,24 @@ impl ConfigFile {
self.json.import_map.clone()
}

pub fn node_modules_dir(&self) -> Option<bool> {
pub fn node_modules_dir_flag(&self) -> Option<bool> {
self.json.node_modules_dir
}

pub fn deno_modules_dir(&self) -> Option<bool> {
self.json.deno_modules_dir
pub fn vendor_dir_flag(&self) -> Option<bool> {
self.json.vendor
}

pub fn deno_modules_dir_path(&self) -> Option<PathBuf> {
if self.json.deno_modules_dir == Some(true) {
pub fn vendor_dir_path(&self) -> Option<PathBuf> {
if self.json.vendor == Some(true) {
Some(
self
.specifier
.to_file_path()
.unwrap()
.parent()
.unwrap()
.join("deno_modules"),
.join("vendor"),
)
} else {
None
Expand Down Expand Up @@ -903,8 +903,8 @@ impl ConfigFile {
Vec::new()
};

if self.deno_modules_dir() == Some(true) {
exclude.push("deno_modules".to_string());
if self.vendor_dir_flag() == Some(true) {
exclude.push("vendor".to_string());
}

let raw_files_config = SerializedFilesConfig {
Expand Down
42 changes: 18 additions & 24 deletions cli/args/flags.rs
Original file line number Diff line number Diff line change
Expand Up @@ -389,7 +389,7 @@ pub struct Flags {
pub type_check_mode: TypeCheckMode,
pub config_flag: ConfigFlag,
pub node_modules_dir: Option<bool>,
pub deno_modules_dir: Option<bool>,
pub vendor: Option<bool>,
pub enable_testing_features: bool,
pub ext: Option<String>,
pub ignore: Vec<PathBuf>,
Expand Down Expand Up @@ -1560,7 +1560,7 @@ TypeScript compiler cache: Subdirectory containing TS compiler output.",
.arg(config_arg())
.arg(import_map_arg())
.arg(node_modules_dir_arg())
.arg(deno_modules_dir_arg())
.arg(vendor_arg())
.arg(
Arg::new("json")
.long("json")
Expand Down Expand Up @@ -2107,7 +2107,7 @@ Remote modules and multiple modules may also be specified:
.arg(import_map_arg())
.arg(lock_arg())
.arg(node_modules_dir_arg())
.arg(deno_modules_dir_arg())
.arg(vendor_arg())
.arg(reload_arg())
.arg(ca_file_arg()))
}
Expand All @@ -2122,7 +2122,7 @@ fn compile_args_without_check_args(app: Command) -> Command {
.arg(no_remote_arg())
.arg(no_npm_arg())
.arg(node_modules_dir_arg())
.arg(deno_modules_dir_arg())
.arg(vendor_arg())
.arg(config_arg())
.arg(no_config_arg())
.arg(reload_arg())
Expand Down Expand Up @@ -2846,14 +2846,14 @@ fn node_modules_dir_arg() -> Arg {
.help("Enables or disables the use of a local node_modules folder for npm packages")
}

fn deno_modules_dir_arg() -> Arg {
Arg::new("deno-modules-dir")
.long("deno-modules-dir")
fn vendor_arg() -> Arg {
Arg::new("vendor")
.long("vendor")
.num_args(0..=1)
.value_parser(value_parser!(bool))
.default_missing_value("true")
.require_equals(true)
.help("UNSTABLE: Enables or disables the use of a local deno_modules folder for remote modules")
.help("UNSTABLE: Enables or disables the use of a local vendor folder for remote modules and node_modules folder for npm packages")
}

fn unsafely_ignore_certificate_errors_arg() -> Arg {
Expand Down Expand Up @@ -3143,7 +3143,7 @@ fn info_parse(flags: &mut Flags, matches: &mut ArgMatches) {
import_map_arg_parse(flags, matches);
location_arg_parse(flags, matches);
ca_file_arg_parse(flags, matches);
node_and_deno_modules_dir_arg_parse(flags, matches);
node_modules_and_vendor_dir_arg_parse(flags, matches);
lock_arg_parse(flags, matches);
no_lock_arg_parse(flags, matches);
no_remote_arg_parse(flags, matches);
Expand Down Expand Up @@ -3420,7 +3420,7 @@ fn vendor_parse(flags: &mut Flags, matches: &mut ArgMatches) {
config_args_parse(flags, matches);
import_map_arg_parse(flags, matches);
lock_arg_parse(flags, matches);
node_and_deno_modules_dir_arg_parse(flags, matches);
node_modules_and_vendor_dir_arg_parse(flags, matches);
reload_arg_parse(flags, matches);

flags.subcommand = DenoSubcommand::Vendor(VendorFlags {
Expand All @@ -3446,7 +3446,7 @@ fn compile_args_without_check_parse(
import_map_arg_parse(flags, matches);
no_remote_arg_parse(flags, matches);
no_npm_arg_parse(flags, matches);
node_and_deno_modules_dir_arg_parse(flags, matches);
node_modules_and_vendor_dir_arg_parse(flags, matches);
config_args_parse(flags, matches);
reload_arg_parse(flags, matches);
lock_args_parse(flags, matches);
Expand Down Expand Up @@ -3739,12 +3739,12 @@ fn no_npm_arg_parse(flags: &mut Flags, matches: &mut ArgMatches) {
}
}

fn node_and_deno_modules_dir_arg_parse(
fn node_modules_and_vendor_dir_arg_parse(
flags: &mut Flags,
matches: &mut ArgMatches,
) {
flags.node_modules_dir = matches.remove_one::<bool>("node-modules-dir");
flags.deno_modules_dir = matches.remove_one::<bool>("deno-modules-dir");
flags.vendor = matches.remove_one::<bool>("vendor");
}

fn reload_arg_validate(urlstr: &str) -> Result<String, String> {
Expand Down Expand Up @@ -6315,35 +6315,29 @@ mod tests {
}

#[test]
fn deno_modules_dir() {
let r =
flags_from_vec(svec!["deno", "run", "--deno-modules-dir", "script.ts"]);
fn vendor_flag() {
let r = flags_from_vec(svec!["deno", "run", "--vendor", "script.ts"]);
assert_eq!(
r.unwrap(),
Flags {
subcommand: DenoSubcommand::Run(RunFlags {
script: "script.ts".to_string(),
watch: Default::default(),
}),
deno_modules_dir: Some(true),
vendor: Some(true),
..Flags::default()
}
);

let r = flags_from_vec(svec![
"deno",
"run",
"--deno-modules-dir=false",
"script.ts"
]);
let r = flags_from_vec(svec!["deno", "run", "--vendor=false", "script.ts"]);
assert_eq!(
r.unwrap(),
Flags {
subcommand: DenoSubcommand::Run(RunFlags {
script: "script.ts".to_string(),
watch: Default::default(),
}),
deno_modules_dir: Some(false),
vendor: Some(false),
..Flags::default()
}
);
Expand Down
35 changes: 17 additions & 18 deletions cli/args/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -541,7 +541,7 @@ pub struct CliOptions {
flags: Flags,
initial_cwd: PathBuf,
maybe_node_modules_folder: Option<PathBuf>,
maybe_deno_modules_folder: Option<PathBuf>,
maybe_vendor_folder: Option<PathBuf>,
maybe_config_file: Option<ConfigFile>,
maybe_package_json: Option<PackageJson>,
maybe_lockfile: Option<Arc<Mutex<Lockfile>>>,
Expand Down Expand Up @@ -577,11 +577,8 @@ impl CliOptions {
maybe_package_json.as_ref(),
)
.with_context(|| "Resolving node_modules folder.")?;
let maybe_deno_modules_folder = resolve_deno_modules_folder(
&initial_cwd,
&flags,
maybe_config_file.as_ref(),
);
let maybe_vendor_folder =
resolve_vendor_folder(&initial_cwd, &flags, maybe_config_file.as_ref());

Ok(Self {
flags,
Expand All @@ -590,7 +587,7 @@ impl CliOptions {
maybe_lockfile,
maybe_package_json,
maybe_node_modules_folder,
maybe_deno_modules_folder,
maybe_vendor_folder,
overrides: Default::default(),
})
}
Expand Down Expand Up @@ -863,7 +860,7 @@ impl CliOptions {
self
.maybe_config_file
.as_ref()
.and_then(|c| c.node_modules_dir())
.and_then(|c| c.node_modules_dir_flag())
})
}

Expand All @@ -874,8 +871,8 @@ impl CliOptions {
.map(|path| ModuleSpecifier::from_directory_path(path).unwrap())
}

pub fn deno_modules_dir_path(&self) -> Option<&PathBuf> {
self.maybe_deno_modules_folder.as_ref()
pub fn vendor_dir_path(&self) -> Option<&PathBuf> {
self.maybe_vendor_folder.as_ref()
}

pub fn resolve_root_cert_store_provider(
Expand Down Expand Up @@ -1188,7 +1185,9 @@ fn resolve_node_modules_folder(
) -> Result<Option<PathBuf>, AnyError> {
let use_node_modules_dir = flags
.node_modules_dir
.or_else(|| maybe_config_file.and_then(|c| c.node_modules_dir()));
.or_else(|| maybe_config_file.and_then(|c| c.node_modules_dir_flag()))
.or(flags.vendor)
.or_else(|| maybe_config_file.and_then(|c| c.vendor_dir_flag()));
let path = if use_node_modules_dir == Some(false) {
return Ok(None);
} else if let Some(state) = &*NPM_PROCESS_STATE {
Expand All @@ -1209,28 +1208,28 @@ fn resolve_node_modules_folder(
Ok(Some(canonicalize_path_maybe_not_exists(&path)?))
}

fn resolve_deno_modules_folder(
fn resolve_vendor_folder(
cwd: &Path,
flags: &Flags,
maybe_config_file: Option<&ConfigFile>,
) -> Option<PathBuf> {
let use_deno_modules_dir = flags
.deno_modules_dir
.or_else(|| maybe_config_file.and_then(|c| c.deno_modules_dir()))
let use_vendor_dir = flags
.vendor
.or_else(|| maybe_config_file.and_then(|c| c.vendor_dir_flag()))
.unwrap_or(false);
// Unlike the node_modules directory, there is no need to canonicalize
// this directory because it's just used as a cache and the resolved
// specifier is not based on the canonicalized path (unlike the modules
// in the node_modules folder).
if !use_deno_modules_dir {
if !use_vendor_dir {
None
} else if let Some(config_path) = maybe_config_file
.as_ref()
.and_then(|c| c.specifier.to_file_path().ok())
{
Some(config_path.parent().unwrap().join("deno_modules"))
Some(config_path.parent().unwrap().join("vendor"))
} else {
Some(cwd.join("deno_modules"))
Some(cwd.join("vendor"))
}
}

Expand Down
2 changes: 1 addition & 1 deletion cli/cache/http_cache/local.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ use super::CachedUrlMetadata;
use super::HttpCache;
use super::HttpCacheItemKey;

/// A deno_modules http cache for the lsp that provides functionality
/// A vendor/ folder http cache for the lsp that provides functionality
/// for doing a reverse mapping.
#[derive(Debug)]
pub struct LocalLspHttpCache {
Expand Down
2 changes: 1 addition & 1 deletion cli/factory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ impl CliFactory {
pub fn http_cache(&self) -> Result<&Arc<dyn HttpCache>, AnyError> {
self.services.http_cache.get_or_try_init(|| {
let global_cache = self.global_http_cache()?.clone();
match self.options.deno_modules_dir_path() {
match self.options.vendor_dir_path() {
Some(local_path) => {
let local_cache =
LocalHttpCache::new(local_path.clone(), global_cache);
Expand Down
14 changes: 9 additions & 5 deletions cli/lsp/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -475,10 +475,8 @@ impl Config {
.and_then(|p| p.maybe_node_modules_dir.as_ref())
}

pub fn maybe_deno_modules_dir_path(&self) -> Option<PathBuf> {
self
.maybe_config_file()
.and_then(|c| c.deno_modules_dir_path())
pub fn maybe_vendor_dir_path(&self) -> Option<PathBuf> {
self.maybe_config_file().and_then(|c| c.vendor_dir_path())
}

pub fn maybe_config_file(&self) -> Option<&ConfigFile> {
Expand Down Expand Up @@ -816,7 +814,13 @@ fn resolve_node_modules_dir(config_file: &ConfigFile) -> Option<PathBuf> {
// `nodeModulesDir: true` setting in the deno.json file. This is to
// reduce the chance of modifying someone's node_modules directory
// without them having asked us to do so.
if config_file.node_modules_dir() != Some(true) {
let explicitly_disabled = config_file.node_modules_dir_flag() == Some(false);
if explicitly_disabled {
return None;
}
let enabled = config_file.node_modules_dir_flag() == Some(true)
|| config_file.vendor_dir_flag() == Some(true);
if !enabled {
return None;
}
if config_file.specifier.scheme() != "file" {
Expand Down
6 changes: 3 additions & 3 deletions cli/lsp/documents.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1184,7 +1184,7 @@ impl Documents {
document_preload_limit: usize,
maybe_import_map: Option<&import_map::ImportMap>,
maybe_jsx_config: Option<&JsxImportSourceConfig>,
maybe_deno_modules_dir: Option<bool>,
maybe_vendor_dir: Option<bool>,
maybe_package_json_deps: Option<&PackageJsonDeps>,
) -> u64 {
let mut hasher = FastInsecureHasher::default();
Expand All @@ -1199,7 +1199,7 @@ impl Documents {
hasher.write_str(&import_map.to_json());
hasher.write_str(import_map.base_url().as_str());
}
hasher.write_hashable(maybe_deno_modules_dir);
hasher.write_hashable(maybe_vendor_dir);
hasher.write_hashable(maybe_jsx_config);
if let Some(package_json_deps) = &maybe_package_json_deps {
// We need to ensure the hashing is deterministic so explicitly type
Expand Down Expand Up @@ -1234,7 +1234,7 @@ impl Documents {
options.document_preload_limit,
options.maybe_import_map.as_deref(),
maybe_jsx_config.as_ref(),
options.maybe_config_file.and_then(|c| c.deno_modules_dir()),
options.maybe_config_file.and_then(|c| c.vendor_dir_flag()),
maybe_package_json_deps.as_ref(),
);
let deps_provider =
Expand Down
2 changes: 1 addition & 1 deletion cli/lsp/language_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -906,7 +906,7 @@ impl Inner {
// update the cache path
let global_cache = Arc::new(GlobalHttpCache::new(dir.deps_folder_path()));
let maybe_local_cache =
self.config.maybe_deno_modules_dir_path().map(|local_path| {
self.config.maybe_vendor_dir_path().map(|local_path| {
Arc::new(LocalLspHttpCache::new(local_path, global_cache.clone()))
});
let cache: Arc<dyn HttpCache> = maybe_local_cache
Expand Down
6 changes: 3 additions & 3 deletions cli/schemas/config-file.v1.json
Original file line number Diff line number Diff line change
Expand Up @@ -429,11 +429,11 @@
}
},
"nodeModulesDir": {
"description": "Enables or disables the use of a local node_modules folder for npm packages. Alternatively, use the `--node-modules-dir` or `--node-modules-dir=false` flag. Requires Deno 1.34 or later.",
"description": "Enables or disables the use of a local node_modules folder for npm packages. Alternatively, use the `--node-modules-dir` flag or override the config via `--node-modules-dir=false`. Requires Deno 1.34 or later.",
"type": "boolean"
},
"denoModulesDir": {
"description": "UNSTABLE: Enables or disables the use of a local deno_modules folder as a local cache for remote modules. Alternatively, use the `--deno-modules-dir` or `--deno-modules-dir=false` flag. Requires Deno 1.36 or later.",
"vendor": {
"description": "UNSTABLE: Enables or disables the use of a local vendor folder as a local cache for remote modules and node_modules folder for npm packages. Alternatively, use the `--vendor` flag or override the config via `--vendor=false`. Requires Deno 1.36.1 or later.",
"type": "boolean"
},
"tasks": {
Expand Down
Loading

0 comments on commit b9b0386

Please sign in to comment.