Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: auto-discover package.json for npm dependencies #17272

Merged
merged 88 commits into from
Feb 20, 2023
Merged
Show file tree
Hide file tree
Changes from 85 commits
Commits
Show all changes
88 commits
Select commit Hold shift + click to select a range
419ce99
feat: support node built-in module imports
bartlomieju Jan 4, 2023
80adb4d
feat: support node built-in module imports
bartlomieju Jan 4, 2023
44bddac
[WIP] experiment: read package.json to discover dependencies for bare…
bartlomieju Jan 5, 2023
23287d5
temp
bartlomieju Jan 6, 2023
528e0c0
Merge branch 'main' into bare_specifier_package_json
bartlomieju Jan 7, 2023
8ffe400
check if is cjs
bartlomieju Jan 7, 2023
f7a6e8b
Merge branch 'main' into builtin_node_modules
dsherret Jan 10, 2023
26cdd98
Add half of typescript support.
dsherret Jan 10, 2023
d40b7e0
Adding automatic @types/node package resolution - Note: Not working i…
dsherret Jan 11, 2023
31aac2f
Merge branch 'main' into builtin_node_modules
dsherret Jan 11, 2023
1082f86
Update name.
dsherret Jan 11, 2023
cb2d000
move NodeModulePolyfill to ext/node
bartlomieju Jan 11, 2023
b76d475
Merge branch 'main' into builtin_node_modules
bartlomieju Jan 11, 2023
ed26cf9
Merge branch 'main' into builtin_node_modules
dsherret Jan 12, 2023
8f88662
Merge branch 'builtin_node_modules' of https://github.com/bartlomieju…
dsherret Jan 12, 2023
8a30ab1
Merge branch 'main' into builtin_node_modules
dsherret Jan 13, 2023
08a661d
Make node: specifiers use the @types/node ambient module for `deno ch…
dsherret Jan 13, 2023
7ba3623
Merge remote-tracking branch 'upstream/main' into builtin_node_modules
bartlomieju Jan 15, 2023
c7b1514
add some tests
bartlomieju Jan 15, 2023
3971ae0
Make work in lsp.
dsherret Jan 16, 2023
84cee59
Merge branch 'main' into builtin_node_modules
dsherret Jan 16, 2023
7c38994
Updates for rebase/merge
dsherret Jan 16, 2023
dfbb9ea
Stop log spam
dsherret Jan 16, 2023
8f1865a
Merge branch 'main' into bare_specifier_package_json
bartlomieju Jan 16, 2023
01566b1
default to ESM instead of CJS
bartlomieju Jan 16, 2023
907291c
Diagnostics for uncached @types/node package and invalid node: specifier
dsherret Jan 16, 2023
1a35654
Merge branch 'main' into builtin_node_modules
dsherret Jan 16, 2023
b631b0a
Synthetic reqs barely done... committing for historical reasons befor…
dsherret Jan 16, 2023
b0e9551
Revert "Synthetic reqs barely done... committing for historical reaso…
dsherret Jan 16, 2023
10d75c5
Refactor creating resolver with lockfile
dsherret Jan 17, 2023
9b22fe3
Merge branch 'main' into builtin_node_modules
dsherret Jan 17, 2023
a3bc377
Fix bug introduced by latest refactor
dsherret Jan 17, 2023
8c16c3f
Don't store @types/node in the lockfile
dsherret Jan 17, 2023
af0bf90
Merge branch 'main' into bare_specifier_package_json
bartlomieju Jan 18, 2023
2778bba
Merge branch 'main' into bare_specifier_package_json
bartlomieju Jan 19, 2023
b8b4d39
wip to autodiscover
bartlomieju Jan 19, 2023
94ca058
Merge branch 'builtin_node_modules' into bare_specifier_package_json
bartlomieju Jan 19, 2023
f9e4c03
wip
bartlomieju Jan 19, 2023
704b95f
debug
bartlomieju Jan 19, 2023
50e135d
temp
bartlomieju Jan 19, 2023
b1177af
Debug
bartlomieju Jan 19, 2023
dbbb682
Merge branch 'main' into bare_specifier_package_json
bartlomieju Jan 28, 2023
27c610c
Merge branch 'bare_specifier_package_json_2' into bare_specifier_pack…
bartlomieju Jan 28, 2023
4319bd5
Merge branch 'main' into bare_specifier_package_json
bartlomieju Jan 28, 2023
739a885
lint
bartlomieju Jan 28, 2023
b86b279
works with simple ESM express app
bartlomieju Jan 28, 2023
c16fdd7
Merge branch 'main' into bare_specifier_package_json
dsherret Jan 30, 2023
c46ab64
revert some changes
bartlomieju Jan 30, 2023
43e1d8a
revert some changes 2
bartlomieju Jan 30, 2023
f87271e
working!
bartlomieju Jan 30, 2023
9cac655
Merge branch 'main' into bare_specifier_package_json
bartlomieju Jan 31, 2023
45a72d2
Merge branch 'main' into bare_specifier_package_json
dsherret Feb 1, 2023
54beff7
Add `NpmPackageReq::from_dependency_entry`.
dsherret Feb 1, 2023
c32042d
Updates
dsherret Feb 1, 2023
b1ac2f7
Fix test.
dsherret Feb 1, 2023
92b05cd
Make `--node-modules-dir` true when a package.json is autodiscovered
dsherret Feb 1, 2023
06b8698
update discovery algorithm
bartlomieju Feb 1, 2023
983cf68
Merge branch 'main' into bare_specifier_package_json
dsherret Feb 2, 2023
2d13028
Add tests.
dsherret Feb 2, 2023
c83f24c
log discovered package.json in preparation for tests
bartlomieju Feb 2, 2023
c19ebf9
Merge branch 'main' into bare_specifier_package_json
bartlomieju Feb 12, 2023
1ba68ec
make progress debugging vite project
bartlomieju Feb 13, 2023
92cc13c
more debug output, plus something is actually getting resolved
bartlomieju Feb 13, 2023
e01bc92
Merge branch 'main' into bare_specifier_package_json
dsherret Feb 13, 2023
f178a81
Remove unused code.
dsherret Feb 13, 2023
ef5e681
Merge branch 'main' into bare_specifier_package_json
bartlomieju Feb 16, 2023
50ba728
fix package json error
bartlomieju Feb 16, 2023
18de119
revert Cargo.toml
bartlomieju Feb 16, 2023
ff27653
remove debug logs
bartlomieju Feb 16, 2023
babe74d
fix some tests
bartlomieju Feb 17, 2023
7d233a9
Merge remote-tracking branch 'upstream/main' into bare_specifier_pack…
bartlomieju Feb 17, 2023
fb79ae9
don't use package.json deps for now
bartlomieju Feb 17, 2023
f24eab7
address some todos
bartlomieju Feb 17, 2023
70617aa
remove a panic
bartlomieju Feb 17, 2023
2e28368
wip basic test
bartlomieju Feb 17, 2023
e86421c
improve tests
bartlomieju Feb 18, 2023
da7da75
update assertion
bartlomieju Feb 18, 2023
b792073
fix remaining test
bartlomieju Feb 19, 2023
8d2bce5
don't forward NpmProcessState
bartlomieju Feb 19, 2023
d0f29ac
remove pub
bartlomieju Feb 19, 2023
a43a91c
remove node_modules dir
bartlomieju Feb 19, 2023
ed1e019
add git ignore
bartlomieju Feb 19, 2023
b7e2897
remove node_modules dir
bartlomieju Feb 19, 2023
07ea6f0
fix error when parsing
bartlomieju Feb 20, 2023
370b95c
Merge branch 'main' into bare_specifier_package_json
bartlomieju Feb 20, 2023
b76362f
Merge branch 'main' into bare_specifier_package_json
bartlomieju Feb 20, 2023
594d6ff
review
bartlomieju Feb 20, 2023
9f08858
remove a TODO
bartlomieju Feb 20, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 53 additions & 6 deletions cli/args/flags.rs
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,7 @@ pub struct Flags {
pub cached_only: bool,
pub type_check_mode: TypeCheckMode,
pub config_flag: ConfigFlag,
pub node_modules_dir: bool,
pub node_modules_dir: Option<bool>,
pub coverage_dir: Option<String>,
pub enable_testing_features: bool,
pub ignore: Vec<PathBuf>,
Expand Down Expand Up @@ -503,6 +503,33 @@ impl Flags {
}
}

/// Extract path argument for `package.json` search paths.
/// If it returns Some(path), the `package.json` should be discovered
/// from the `path` dir.
/// If it returns None, the `package.json` file shouldn't be discovered at
/// all.
pub fn package_json_arg(&self) -> Option<PathBuf> {
use DenoSubcommand::*;

if let Run(RunFlags { script }) = &self.subcommand {
if let Ok(module_specifier) = deno_core::resolve_url_or_path(script) {
if module_specifier.scheme() == "file" {
let p = module_specifier
.to_file_path()
.unwrap()
.parent()?
.to_owned();
return Some(p);
} else if module_specifier.scheme() == "npm" {
let p = std::env::current_dir().unwrap();
return Some(p);
}
}
}

None
}

pub fn has_permission(&self) -> bool {
self.allow_all
|| self.allow_hrtime
Expand Down Expand Up @@ -2309,7 +2336,12 @@ fn no_npm_arg<'a>() -> Arg<'a> {
fn local_npm_arg<'a>() -> Arg<'a> {
Arg::new("node-modules-dir")
.long("node-modules-dir")
.help("Creates a local node_modules folder")
.min_values(0)
.max_values(1)
.takes_value(true)
.require_equals(true)
.possible_values(["true", "false"])
.help("Creates a local node_modules folder. This option is implicitly true when a package.json is auto-discovered.")
}

fn unsafely_ignore_certificate_errors_arg<'a>() -> Arg<'a> {
Expand Down Expand Up @@ -3247,9 +3279,7 @@ fn no_npm_arg_parse(flags: &mut Flags, matches: &clap::ArgMatches) {
}

fn local_npm_args_parse(flags: &mut Flags, matches: &ArgMatches) {
if matches.is_present("node-modules-dir") {
flags.node_modules_dir = true;
}
flags.node_modules_dir = optional_bool_parse(matches, "node-modules-dir");
}

fn inspect_arg_validate(val: &str) -> Result<(), String> {
Expand Down Expand Up @@ -5448,7 +5478,24 @@ mod tests {
subcommand: DenoSubcommand::Run(RunFlags {
script: "script.ts".to_string(),
}),
node_modules_dir: true,
node_modules_dir: Some(true),
..Flags::default()
}
);

let r = flags_from_vec(svec![
"deno",
"run",
"--node-modules-dir=false",
"script.ts"
]);
assert_eq!(
r.unwrap(),
Flags {
subcommand: DenoSubcommand::Run(RunFlags {
script: "script.ts".to_string(),
}),
node_modules_dir: Some(false),
..Flags::default()
}
);
Expand Down
185 changes: 180 additions & 5 deletions cli/args/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,13 @@ mod flags;
mod flags_allow_net;
mod import_map;
mod lockfile;
pub mod package_json;

pub use self::import_map::resolve_import_map_from_specifier;
use ::import_map::ImportMap;

use crate::npm::NpmResolutionSnapshot;
use crate::util::fs::canonicalize_path;
pub use config_file::BenchConfig;
pub use config_file::CompilerOptions;
pub use config_file::ConfigFile;
Expand All @@ -32,26 +36,36 @@ use deno_core::anyhow::Context;
use deno_core::error::AnyError;
use deno_core::normalize_path;
use deno_core::parking_lot::Mutex;
use deno_core::serde_json;
use deno_core::url::Url;
use deno_graph::npm::NpmPackageReq;
use deno_runtime::colors;
use deno_runtime::deno_node::PackageJson;
use deno_runtime::deno_tls::rustls;
use deno_runtime::deno_tls::rustls::RootCertStore;
use deno_runtime::deno_tls::rustls_native_certs::load_native_certs;
use deno_runtime::deno_tls::rustls_pemfile;
use deno_runtime::deno_tls::webpki_roots;
use deno_runtime::inspector_server::InspectorServer;
use deno_runtime::permissions::PermissionsOptions;
use once_cell::sync::Lazy;
use std::collections::BTreeMap;
use std::collections::HashMap;
use std::collections::HashSet;
use std::env;
use std::io::BufReader;
use std::io::Cursor;
use std::net::SocketAddr;
use std::num::NonZeroUsize;
use std::path::Path;
use std::path::PathBuf;
use std::sync::Arc;

use crate::cache::DenoDir;
use crate::file_fetcher::FileFetcher;
// TODO(bartlomieju): refactor this bit; we have a circular dependency
// between this file and `cli/npm/resolver/mod.rs`.
use crate::npm::NpmProcessState;
bartlomieju marked this conversation as resolved.
Show resolved Hide resolved
use crate::util::fs::canonicalize_path_maybe_not_exists;
use crate::version;

Expand Down Expand Up @@ -375,6 +389,74 @@ fn resolve_lint_rules_options(
}
}

/// Discover `package.json` file. If `maybe_stop_at` is provided, we will stop
/// crawling up the directory tree at that path.
fn discover_package_json(
flags: &Flags,
maybe_stop_at: Option<PathBuf>,
) -> Result<Option<PackageJson>, AnyError> {
pub fn discover_from(
start: &Path,
checked: &mut HashSet<PathBuf>,
maybe_stop_at: Option<PathBuf>,
) -> Result<Option<PackageJson>, AnyError> {
const PACKAGE_JSON_NAME: &str = "package.json";

for ancestor in start.ancestors() {
if checked.insert(ancestor.to_path_buf()) {
let path = ancestor.join(PACKAGE_JSON_NAME);

let source = match std::fs::read_to_string(&path) {
Ok(source) => source,
Err(err) if err.kind() == std::io::ErrorKind::NotFound => {
if let Some(stop_at) = maybe_stop_at.as_ref() {
if ancestor == stop_at {
break;
}
}
continue;
}
Err(err) => bail!(
"Error loading package.json at {}. {:#}",
path.display(),
err
),
};

let package_json = PackageJson::load_from_string(path.clone(), source)?;
log::debug!("package.json file found at '{}'", path.display());
return Ok(Some(package_json));
}
}
// No config file found.
log::debug!("No package.json file found");
Ok(None)
}

// TODO(bartlomieju): discover for all subcommands, but print warnings that
// `package.json` is ignored in bundle/compile/etc.

if let Some(package_json_arg) = flags.package_json_arg() {
return discover_from(
&package_json_arg,
&mut HashSet::new(),
maybe_stop_at,
);
} else if let crate::args::DenoSubcommand::Task(TaskFlags {
cwd: Some(path),
..
}) = &flags.subcommand
{
// attempt to resolve the config file from the task subcommand's
// `--cwd` when specified
let task_cwd = canonicalize_path(&PathBuf::from(path))?;
return discover_from(&task_cwd, &mut HashSet::new(), None);
}

log::debug!("No package.json file found");
Ok(None)
}

/// Create and populate a root cert store based on the passed options and
/// environment.
pub fn get_root_cert_store(
Expand Down Expand Up @@ -459,6 +541,12 @@ pub fn get_root_cert_store(
Ok(root_cert_store)
}

const RESOLUTION_STATE_ENV_VAR_NAME: &str =
"DENO_DONT_USE_INTERNAL_NODE_COMPAT_STATE";

static IS_NPM_MAIN: Lazy<bool> =
Lazy::new(|| std::env::var(RESOLUTION_STATE_ENV_VAR_NAME).is_ok());

/// Overrides for the options below that when set will
/// use these values over the values derived from the
/// CLI flags or config file.
Expand All @@ -474,15 +562,18 @@ pub struct CliOptions {
// application need not concern itself with, so keep these private
flags: Flags,
maybe_config_file: Option<ConfigFile>,
maybe_package_json: Option<PackageJson>,
maybe_lockfile: Option<Arc<Mutex<Lockfile>>>,
overrides: CliOptionOverrides,
npm_process_state: Arc<Mutex<Option<NpmProcessState>>>,
bartlomieju marked this conversation as resolved.
Show resolved Hide resolved
}

impl CliOptions {
pub fn new(
flags: Flags,
maybe_config_file: Option<ConfigFile>,
maybe_lockfile: Option<Lockfile>,
maybe_package_json: Option<PackageJson>,
) -> Self {
if let Some(insecure_allowlist) =
flags.unsafely_ignore_certificate_errors.as_ref()
Expand All @@ -503,16 +594,39 @@ impl CliOptions {
Self {
maybe_config_file,
maybe_lockfile,
maybe_package_json,
flags,
overrides: Default::default(),
npm_process_state: Arc::new(Mutex::new(None)),
}
}

pub fn from_flags(flags: Flags) -> Result<Self, AnyError> {
let maybe_config_file = ConfigFile::discover(&flags)?;

let mut maybe_package_json = None;
if let Some(config_file) = &maybe_config_file {
let specifier = config_file.specifier.clone();
if specifier.scheme() == "file" {
let maybe_stop_at = specifier
.to_file_path()
.unwrap()
.parent()
.map(|p| p.to_path_buf());

maybe_package_json = discover_package_json(&flags, maybe_stop_at)?;
}
} else {
maybe_package_json = discover_package_json(&flags, None)?;
}
let maybe_lock_file =
lockfile::discover(&flags, maybe_config_file.as_ref())?;
Ok(Self::new(flags, maybe_config_file, maybe_lock_file))
Ok(Self::new(
flags,
maybe_config_file,
maybe_lock_file,
maybe_package_json,
))
}

pub fn maybe_config_file_specifier(&self) -> Option<ModuleSpecifier> {
Expand Down Expand Up @@ -576,7 +690,7 @@ impl CliOptions {
};
resolve_import_map_from_specifier(
&import_map_specifier,
self.get_maybe_config_file().as_ref(),
self.maybe_config_file().as_ref(),
file_fetcher,
)
.await
Expand All @@ -586,21 +700,68 @@ impl CliOptions {
.map(Some)
}

fn get_npm_process_state(&self) -> Option<NpmProcessState> {
if !self.is_npm_main() {
return None;
}

let mut maybe_state = self.npm_process_state.lock();

// TODO(bartlomieju): remove this clone, return a reference maybe?
if maybe_state.is_some() {
return maybe_state.clone();
}

let state = std::env::var(RESOLUTION_STATE_ENV_VAR_NAME).ok()?;
let state: NpmProcessState = serde_json::from_str(&state).ok()?;
// remove the environment variable so that sub processes
// that are spawned do not also use this.
std::env::remove_var(RESOLUTION_STATE_ENV_VAR_NAME);
*maybe_state = Some(state.clone());
Some(state)
}

pub fn get_npm_resolution_snapshot(&self) -> Option<NpmResolutionSnapshot> {
if let Some(state) = self.get_npm_process_state() {
return Some(state.snapshot);
}

None
}

// If the main module should be treated as being in an npm package.
// This is triggered via a secret environment variable which is used
// for functionality like child_process.fork. Users should NOT depend
// on this functionality.
pub fn is_npm_main(&self) -> bool {
*IS_NPM_MAIN
}

/// Overrides the import map specifier to use.
pub fn set_import_map_specifier(&mut self, path: Option<ModuleSpecifier>) {
self.overrides.import_map_specifier = Some(path);
}

pub fn node_modules_dir(&self) -> bool {
self.flags.node_modules_dir
if let Some(node_modules_dir) = self.flags.node_modules_dir {
return node_modules_dir;
}

if let Some(npm_process_state) = self.get_npm_process_state() {
dsherret marked this conversation as resolved.
Show resolved Hide resolved
return npm_process_state.local_node_modules_path.is_some();
}

self.maybe_package_json.is_some()
}

/// Resolves the path to use for a local node_modules folder.
pub fn resolve_local_node_modules_folder(
&self,
) -> Result<Option<PathBuf>, AnyError> {
let path = if !self.flags.node_modules_dir {
let path = if !self.node_modules_dir() {
return Ok(None);
} else if let Some(state) = self.get_npm_process_state() {
return Ok(state.local_node_modules_path.as_ref().map(PathBuf::from));
} else if let Some(config_path) = self
.maybe_config_file
.as_ref()
Expand Down Expand Up @@ -699,10 +860,24 @@ impl CliOptions {
}
}

pub fn get_maybe_config_file(&self) -> &Option<ConfigFile> {
pub fn maybe_config_file(&self) -> &Option<ConfigFile> {
&self.maybe_config_file
}

pub fn maybe_package_json(&self) -> &Option<PackageJson> {
&self.maybe_package_json
}

pub fn maybe_package_json_deps(
&self,
) -> Result<Option<HashMap<String, NpmPackageReq>>, AnyError> {
if let Some(package_json) = self.maybe_package_json() {
package_json::get_local_package_json_version_reqs(package_json).map(Some)
} else {
Ok(None)
}
}

pub fn resolve_fmt_options(
&self,
fmt_flags: FmtFlags,
Expand Down
Loading