1
0
Fork 0
mirror of https://github.com/denoland/deno.git synced 2024-12-24 08:09:08 -05:00

fix(npm): improved optional dependency support (#19135)

Note: If the package information has already been cached, then this
requires running with `--reload` or for the registry information to be
fetched some other way (ex. the cache busting).

Closes #15544

---------

Co-authored-by: Bartek Iwańczuk <biwanczuk@gmail.com>
This commit is contained in:
David Sherret 2023-05-17 17:38:50 -04:00 committed by GitHub
parent ad22336245
commit 41f618a1df
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
25 changed files with 300 additions and 61 deletions

5
Cargo.lock generated
View file

@ -1217,9 +1217,9 @@ dependencies = [
[[package]]
name = "deno_npm"
version = "0.3.0"
version = "0.4.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "eab571ab55247090bb78bc0466fdf01bd331c98e9e2c4a236bfa93479d906c5c"
checksum = "007cdc59079448de7af94510f7e9652bc6f17e9029741b26f3754f86a3cc2dd0"
dependencies = [
"anyhow",
"async-trait",
@ -1228,7 +1228,6 @@ dependencies = [
"log",
"monch",
"once_cell",
"parking_lot 0.12.1",
"serde",
"thiserror",
]

View file

@ -53,7 +53,7 @@ deno_bench_util = { version = "0.98.0", path = "./bench_util" }
test_util = { path = "./test_util" }
deno_lockfile = "0.14.0"
deno_media_type = { version = "0.1.0", features = ["module_specifier"] }
deno_npm = "0.3.0"
deno_npm = "0.4.0"
deno_semver = "0.2.1"
# exts

View file

@ -107,8 +107,12 @@ pub async fn snapshot_from_lockfile(
packages.push(SerializedNpmResolutionSnapshotPackage {
pkg_id,
dist: Default::default(), // temporarily empty
dependencies,
// temporarily empty
os: Default::default(),
cpu: Default::default(),
dist: Default::default(),
optional_dependencies: Default::default(),
});
}
(root_packages, packages)
@ -131,11 +135,17 @@ pub async fn snapshot_from_lockfile(
}))
};
let mut version_infos = get_version_infos();
let mut i = 0;
while let Some(result) = version_infos.next().await {
packages[i].dist = match result {
Ok(version_info) => version_info.dist,
match result {
Ok(version_info) => {
let mut package = &mut packages[i];
package.dist = version_info.dist;
package.cpu = version_info.cpu;
package.os = version_info.os;
package.optional_dependencies =
version_info.optional_dependencies.into_keys().collect();
}
Err(err) => {
if api.mark_force_reload() {
// reset and try again
@ -146,7 +156,7 @@ pub async fn snapshot_from_lockfile(
return Err(err);
}
}
};
}
i += 1;
}

View file

@ -13,6 +13,7 @@ use self::package_json::PackageJsonDeps;
use ::import_map::ImportMap;
use deno_core::resolve_url_or_path;
use deno_npm::resolution::ValidSerializedNpmResolutionSnapshot;
use deno_npm::NpmSystemInfo;
use deno_runtime::deno_tls::RootCertStoreProvider;
use deno_semver::npm::NpmPackageReqReference;
use indexmap::IndexMap;
@ -688,6 +689,42 @@ impl CliOptions {
}
}
pub fn npm_system_info(&self) -> NpmSystemInfo {
match self.sub_command() {
DenoSubcommand::Compile(CompileFlags {
target: Some(target),
..
}) => {
// the values of NpmSystemInfo align with the possible values for the
// `arch` and `platform` fields of Node.js' `process` global:
// https://nodejs.org/api/process.html
match target.as_str() {
"aarch64-apple-darwin" => NpmSystemInfo {
os: "darwin".to_string(),
cpu: "arm64".to_string(),
},
"x86_64-apple-darwin" => NpmSystemInfo {
os: "darwin".to_string(),
cpu: "x64".to_string(),
},
"x86_64-unknown-linux-gnu" => NpmSystemInfo {
os: "linux".to_string(),
cpu: "x64".to_string(),
},
"x86_64-pc-windows-msvc" => NpmSystemInfo {
os: "win32".to_string(),
cpu: "x64".to_string(),
},
value => {
log::warn!("Not implemented NPM system info for target '{value}'. Using current system default. This may impact NPM ");
NpmSystemInfo::default()
}
}
}
_ => NpmSystemInfo::default(),
}
}
pub fn resolve_deno_dir(&self) -> Result<DenoDir, AnyError> {
Ok(DenoDir::new(self.maybe_custom_root())?)
}

View file

@ -313,6 +313,7 @@ impl CliFactory {
CliNpmRegistryApi::default_url().to_owned(),
npm_resolution.clone(),
self.options.node_modules_dir_path(),
self.options.npm_system_info(),
);
Ok(Arc::new(CliNpmResolver::new(
fs.clone(),
@ -557,8 +558,9 @@ impl CliFactory {
self.deno_dir()?,
self.npm_api()?,
self.npm_cache()?,
self.npm_resolver().await?,
self.npm_resolution().await?,
self.npm_resolver().await?,
self.options.npm_system_info(),
self.package_json_deps_provider(),
))
}

View file

@ -10,6 +10,7 @@ use deno_core::serde_json::json;
use deno_core::serde_json::Value;
use deno_core::task::spawn;
use deno_core::ModuleSpecifier;
use deno_npm::NpmSystemInfo;
use deno_runtime::deno_fs;
use deno_runtime::deno_node::NodeResolver;
use deno_runtime::deno_node::PackageJson;
@ -467,6 +468,7 @@ fn create_lsp_structs(
registry_url.clone(),
resolution.clone(),
None,
NpmSystemInfo::default(),
);
(
api,
@ -725,6 +727,7 @@ impl Inner {
self.npm_api.base_url().clone(),
npm_resolution,
None,
NpmSystemInfo::default(),
),
None,
));
@ -1242,11 +1245,12 @@ impl Inner {
async fn refresh_npm_specifiers(&mut self) {
let package_reqs = self.documents.npm_package_reqs();
if let Err(err) = self
.npm_resolver
.set_package_reqs((*package_reqs).clone())
.await
{
let npm_resolver = self.npm_resolver.clone();
// spawn to avoid the LSP's Send requirements
let handle = spawn(async move {
npm_resolver.set_package_reqs((*package_reqs).clone()).await
});
if let Err(err) = handle.await.unwrap() {
lsp_warn!("Could not set npm package requirements. {:#}", err);
}
}

View file

@ -23,6 +23,7 @@ use deno_npm::resolution::ValidSerializedNpmResolutionSnapshot;
use deno_npm::NpmPackageCacheFolderId;
use deno_npm::NpmPackageId;
use deno_npm::NpmResolutionPackage;
use deno_npm::NpmSystemInfo;
use deno_semver::npm::NpmPackageNv;
use deno_semver::npm::NpmPackageNvReference;
use deno_semver::npm::NpmPackageReq;
@ -237,12 +238,21 @@ impl NpmResolution {
Ok(nv)
}
pub fn all_packages(&self) -> Vec<NpmResolutionPackage> {
self.snapshot.read().all_packages()
pub fn all_system_packages(
&self,
system_info: &NpmSystemInfo,
) -> Vec<NpmResolutionPackage> {
self.snapshot.read().all_system_packages(system_info)
}
pub fn all_packages_partitioned(&self) -> NpmPackagesPartitioned {
self.snapshot.read().all_packages_partitioned()
pub fn all_system_packages_partitioned(
&self,
system_info: &NpmSystemInfo,
) -> NpmPackagesPartitioned {
self
.snapshot
.read()
.all_system_packages_partitioned(system_info)
}
pub fn has_packages(&self) -> bool {
@ -322,7 +332,7 @@ fn populate_lockfile_from_snapshot(
.as_serialized(),
);
}
for package in snapshot.all_packages() {
for package in snapshot.all_packages_for_every_system() {
lockfile
.check_or_insert_npm_package(npm_package_to_lockfile_info(package))?;
}
@ -330,13 +340,13 @@ fn populate_lockfile_from_snapshot(
}
fn npm_package_to_lockfile_info(
pkg: NpmResolutionPackage,
pkg: &NpmResolutionPackage,
) -> NpmPackageLockfileInfo {
let dependencies = pkg
.dependencies
.into_iter()
.iter()
.map(|(name, id)| NpmPackageDependencyLockfileInfo {
name,
name: name.clone(),
id: id.as_serialized(),
})
.collect();

View file

@ -69,7 +69,6 @@ pub async fn cache_packages(
let mut handles = Vec::with_capacity(packages.len());
for package in packages {
assert_eq!(package.copy_index, 0); // the caller should not provide any of these
let cache = cache.clone();
let registry_url = registry_url.clone();
let handle = spawn(async move {

View file

@ -14,6 +14,7 @@ use deno_npm::resolution::PackageNotFoundFromReferrerError;
use deno_npm::NpmPackageCacheFolderId;
use deno_npm::NpmPackageId;
use deno_npm::NpmResolutionPackage;
use deno_npm::NpmSystemInfo;
use deno_runtime::deno_fs::FileSystem;
use deno_runtime::deno_node::NodePermissions;
use deno_runtime::deno_node::NodeResolutionMode;
@ -33,6 +34,7 @@ pub struct GlobalNpmPackageResolver {
cache: Arc<NpmCache>,
resolution: Arc<NpmResolution>,
registry_url: Url,
system_info: NpmSystemInfo,
}
impl GlobalNpmPackageResolver {
@ -41,12 +43,14 @@ impl GlobalNpmPackageResolver {
cache: Arc<NpmCache>,
registry_url: Url,
resolution: Arc<NpmResolution>,
system_info: NpmSystemInfo,
) -> Self {
Self {
fs,
cache,
resolution,
registry_url,
system_info,
}
}
@ -125,7 +129,26 @@ impl NpmPackageFsResolver for GlobalNpmPackageResolver {
}
async fn cache_packages(&self) -> Result<(), AnyError> {
cache_packages_in_resolver(self).await
let package_partitions = self
.resolution
.all_system_packages_partitioned(&self.system_info);
cache_packages(
package_partitions.packages,
&self.cache,
&self.registry_url,
)
.await?;
// create the copy package folders
for copy in package_partitions.copy_packages {
self.cache.ensure_copy_package(
&copy.get_package_cache_folder_id(),
&self.registry_url,
)?;
}
Ok(())
}
fn ensure_read_permission(
@ -137,26 +160,3 @@ impl NpmPackageFsResolver for GlobalNpmPackageResolver {
ensure_registry_read_permission(&self.fs, permissions, &registry_path, path)
}
}
async fn cache_packages_in_resolver(
resolver: &GlobalNpmPackageResolver,
) -> Result<(), AnyError> {
let package_partitions = resolver.resolution.all_packages_partitioned();
cache_packages(
package_partitions.packages,
&resolver.cache,
&resolver.registry_url,
)
.await?;
// create the copy package folders
for copy in package_partitions.copy_packages {
resolver.cache.ensure_copy_package(
&copy.get_package_cache_folder_id(),
&resolver.registry_url,
)?;
}
Ok(())
}

View file

@ -24,6 +24,7 @@ use deno_core::url::Url;
use deno_npm::resolution::NpmResolutionSnapshot;
use deno_npm::NpmPackageCacheFolderId;
use deno_npm::NpmPackageId;
use deno_npm::NpmSystemInfo;
use deno_runtime::deno_core::futures;
use deno_runtime::deno_fs;
use deno_runtime::deno_node::NodePermissions;
@ -52,6 +53,7 @@ pub struct LocalNpmPackageResolver {
registry_url: Url,
root_node_modules_path: PathBuf,
root_node_modules_url: Url,
system_info: NpmSystemInfo,
}
impl LocalNpmPackageResolver {
@ -62,6 +64,7 @@ impl LocalNpmPackageResolver {
registry_url: Url,
node_modules_folder: PathBuf,
resolution: Arc<NpmResolution>,
system_info: NpmSystemInfo,
) -> Self {
Self {
fs,
@ -72,6 +75,7 @@ impl LocalNpmPackageResolver {
root_node_modules_url: Url::from_directory_path(&node_modules_folder)
.unwrap(),
root_node_modules_path: node_modules_folder,
system_info,
}
}
@ -205,6 +209,7 @@ impl NpmPackageFsResolver for LocalNpmPackageResolver {
&self.progress_bar,
&self.registry_url,
&self.root_node_modules_path,
&self.system_info,
)
.await
}
@ -230,6 +235,7 @@ async fn sync_resolution_with_fs(
progress_bar: &ProgressBar,
registry_url: &Url,
root_node_modules_dir_path: &Path,
system_info: &NpmSystemInfo,
) -> Result<(), AnyError> {
if snapshot.is_empty() {
return Ok(()); // don't create the directory
@ -254,7 +260,8 @@ async fn sync_resolution_with_fs(
// Copy (hardlink in future) <global_registry_cache>/<package_id>/ to
// node_modules/.deno/<package_folder_id_folder_name>/node_modules/<package_name>
let sync_download = should_sync_download();
let mut package_partitions = snapshot.all_packages_partitioned();
let mut package_partitions =
snapshot.all_system_packages_partitioned(system_info);
if sync_download {
// we're running the tests not with --quiet
// and we want the output to be deterministic

View file

@ -18,6 +18,7 @@ use deno_npm::resolution::NpmResolutionSnapshot;
use deno_npm::resolution::PackageReqNotFoundError;
use deno_npm::resolution::SerializedNpmResolutionSnapshot;
use deno_npm::NpmPackageId;
use deno_npm::NpmSystemInfo;
use deno_runtime::deno_fs::FileSystem;
use deno_runtime::deno_node::NodePermissions;
use deno_runtime::deno_node::NodeResolutionMode;
@ -289,6 +290,7 @@ pub fn create_npm_fs_resolver(
registry_url: Url,
resolution: Arc<NpmResolution>,
maybe_node_modules_path: Option<PathBuf>,
system_info: NpmSystemInfo,
) -> Arc<dyn NpmPackageFsResolver> {
match maybe_node_modules_path {
Some(node_modules_folder) => Arc::new(LocalNpmPackageResolver::new(
@ -298,12 +300,14 @@ pub fn create_npm_fs_resolver(
registry_url,
node_modules_folder,
resolution,
system_info,
)),
None => Arc::new(GlobalNpmPackageResolver::new(
fs,
cache,
registry_url,
resolution,
system_info,
)),
}
}

View file

@ -19,6 +19,7 @@ use deno_core::serde_json;
use deno_core::url::Url;
use deno_npm::registry::PackageDepNpmSchemeValueParseError;
use deno_npm::resolution::SerializedNpmResolutionSnapshot;
use deno_npm::NpmSystemInfo;
use deno_runtime::permissions::PermissionsOptions;
use deno_semver::npm::NpmPackageReq;
use deno_semver::npm::NpmVersionReqSpecifierParseError;
@ -343,8 +344,9 @@ pub struct DenoCompileBinaryWriter<'a> {
deno_dir: &'a DenoDir,
npm_api: &'a CliNpmRegistryApi,
npm_cache: &'a NpmCache,
npm_resolution: &'a NpmResolution,
npm_resolver: &'a CliNpmResolver,
resolution: &'a NpmResolution,
npm_system_info: NpmSystemInfo,
package_json_deps_provider: &'a PackageJsonDepsProvider,
}
@ -356,8 +358,9 @@ impl<'a> DenoCompileBinaryWriter<'a> {
deno_dir: &'a DenoDir,
npm_api: &'a CliNpmRegistryApi,
npm_cache: &'a NpmCache,
npm_resolution: &'a NpmResolution,
npm_resolver: &'a CliNpmResolver,
resolution: &'a NpmResolution,
npm_system_info: NpmSystemInfo,
package_json_deps_provider: &'a PackageJsonDepsProvider,
) -> Self {
Self {
@ -367,7 +370,8 @@ impl<'a> DenoCompileBinaryWriter<'a> {
npm_api,
npm_cache,
npm_resolver,
resolution,
npm_system_info,
npm_resolution,
package_json_deps_provider,
}
}
@ -488,9 +492,10 @@ impl<'a> DenoCompileBinaryWriter<'a> {
.resolve_import_map(self.file_fetcher)
.await?
.map(|import_map| (import_map.base_url().clone(), import_map.to_json()));
let (npm_snapshot, npm_vfs, npm_files) = if self.resolution.has_packages() {
let (npm_snapshot, npm_vfs, npm_files) =
if self.npm_resolution.has_packages() {
let (root_dir, files) = self.build_vfs()?.into_dir_and_files();
let snapshot = self.resolution.serialized_snapshot();
let snapshot = self.npm_resolution.serialized_snapshot();
(Some(snapshot), Some(root_dir), files)
} else {
(None, None, Vec::new())
@ -540,7 +545,10 @@ impl<'a> DenoCompileBinaryWriter<'a> {
let registry_url = self.npm_api.base_url();
let root_path = self.npm_cache.registry_folder(registry_url);
let mut builder = VfsBuilder::new(root_path);
for package in self.resolution.all_packages() {
for package in self
.npm_resolution
.all_system_packages(&self.npm_system_info)
{
let folder = self
.npm_resolver
.resolve_pkg_folder_from_pkg_id(&package.pkg_id)?;

View file

@ -37,6 +37,7 @@ use deno_core::ModuleLoader;
use deno_core::ModuleSpecifier;
use deno_core::ModuleType;
use deno_core::ResolutionKind;
use deno_npm::NpmSystemInfo;
use deno_runtime::deno_fs;
use deno_runtime::deno_node;
use deno_runtime::deno_node::analyze::NodeCodeTranslator;
@ -350,6 +351,7 @@ pub async fn run(
npm_registry_url,
npm_resolution.clone(),
node_modules_path,
NpmSystemInfo::default(),
);
let npm_resolver = Arc::new(CliNpmResolver::new(
fs.clone(),

View file

@ -1799,3 +1799,84 @@ fn reload_info_not_found_cache_but_exists_remote() {
output.assert_exit_code(0);
}
}
#[test]
fn binary_package_with_optional_dependencies() {
let context = TestContextBuilder::for_npm()
.use_sync_npm_download()
.use_separate_deno_dir() // the "npm" folder means something in the deno dir, so use a separate folder
.use_copy_temp_dir("npm/binary_package")
.cwd("npm/binary_package")
.build();
let temp_dir = context.temp_dir();
let temp_dir_path = temp_dir.path();
let project_path = temp_dir_path.join("npm/binary_package");
// write empty config file so a lockfile gets created
temp_dir.write("npm/binary_package/deno.json", "{}");
// run it twice, with the first time creating the lockfile and the second using it
for i in 0..2 {
if i == 1 {
assert!(project_path.join("deno.lock").exists());
}
let output = context
.new_command()
.args("run -A --node-modules-dir main.js")
.run();
#[cfg(target_os = "windows")]
{
output.assert_exit_code(0);
output.assert_matches_text(
"[WILDCARD]Hello from binary package on windows[WILDCARD]",
);
assert!(project_path
.join("node_modules/.deno/@denotest+binary-package-windows@1.0.0")
.exists());
assert!(!project_path
.join("node_modules/.deno/@denotest+binary-package-linux@1.0.0")
.exists());
assert!(!project_path
.join("node_modules/.deno/@denotest+binary-package-mac@1.0.0")
.exists());
}
#[cfg(target_os = "macos")]
{
output.assert_exit_code(0);
output.assert_matches_text(
"[WILDCARD]Hello from binary package on mac[WILDCARD]",
);
assert!(!project_path
.join("node_modules/.deno/@denotest+binary-package-windows@1.0.0")
.exists());
assert!(!project_path
.join("node_modules/.deno/@denotest+binary-package-linux@1.0.0")
.exists());
assert!(project_path
.join("node_modules/.deno/@denotest+binary-package-mac@1.0.0")
.exists());
}
#[cfg(target_os = "linux")]
{
output.assert_exit_code(0);
output.assert_matches_text(
"[WILDCARD]Hello from binary package on linux[WILDCARD]",
);
assert!(!project_path
.join("node_modules/.deno/@denotest+binary-package-windows@1.0.0")
.exists());
assert!(project_path
.join("node_modules/.deno/@denotest+binary-package-linux@1.0.0")
.exists());
assert!(!project_path
.join("node_modules/.deno/@denotest+binary-package-mac@1.0.0")
.exists());
}
}
}

View file

@ -0,0 +1 @@
import "npm:@denotest/binary-package";

View file

@ -0,0 +1 @@
console.log("Hello from binary package on linux");

View file

@ -0,0 +1,8 @@
{
"name": "@denotest/binary-package-linux",
"version": "1.0.0",
"main": "index.js",
"os": [
"linux"
]
}

View file

@ -0,0 +1 @@
console.log("Hello from binary package on mac");

View file

@ -0,0 +1,8 @@
{
"name": "@denotest/binary-package-linux",
"version": "1.0.0",
"main": "index.js",
"os": [
"darwin"
]
}

View file

@ -0,0 +1 @@
console.log("Hello from binary package on windows");

View file

@ -0,0 +1,8 @@
{
"name": "@denotest/binary-package-windows",
"version": "1.0.0",
"main": "index.js",
"os": [
"win32"
]
}

View file

@ -0,0 +1,13 @@
const packageByOs = {
"darwin": "@denotest/binary-package-mac",
"linux": "@denotest/binary-package-linux",
"win32": "@denotest/binary-package-windows",
}
const selectedPackage = packageByOs[process.platform];
if (!selectedPackage) {
throw new Error("trying to run on unsupported platform");
}
require(selectedPackage);

View file

@ -0,0 +1,10 @@
{
"name": "@denotest/binary-package",
"version": "1.0.0",
"main": "index.js",
"optionalDependencies": {
"@denotest/binary-package-linux": "1.0.0",
"@denotest/binary-package-mac": "1.0.0",
"@denotest/binary-package-windows": "1.0.0"
}
}

View file

@ -217,7 +217,8 @@ fn add_npm_packages_to_json(
}
}
let mut sorted_packages = snapshot.all_packages();
let mut sorted_packages =
snapshot.all_packages_for_every_system().collect::<Vec<_>>();
sorted_packages.sort_by(|a, b| a.pkg_id.cmp(&b.pkg_id));
let mut json_packages = serde_json::Map::with_capacity(sorted_packages.len());
for pkg in sorted_packages {

View file

@ -136,6 +136,30 @@ fn get_npm_package(package_name: &str) -> Result<Option<CustomNpmPackage>> {
let mut version_info: serde_json::Map<String, serde_json::Value> =
serde_json::from_str(&package_json_text)?;
version_info.insert("dist".to_string(), dist.into());
if let Some(maybe_optional_deps) = version_info.get("optionalDependencies")
{
if let Some(optional_deps) = maybe_optional_deps.as_object() {
if let Some(maybe_deps) = version_info.get("dependencies") {
if let Some(deps) = maybe_deps.as_object() {
let mut cloned_deps = deps.to_owned();
for (key, value) in optional_deps {
cloned_deps.insert(key.to_string(), value.to_owned());
}
version_info.insert(
"dependencies".to_string(),
serde_json::to_value(cloned_deps).unwrap(),
);
}
} else {
version_info.insert(
"dependencies".to_string(),
serde_json::to_value(optional_deps).unwrap(),
);
}
}
}
versions.insert(version.clone(), version_info.into());
let version = semver::Version::parse(&version)?;
if version.cmp(&latest_version).is_gt() {