foster/denoland-deno
1
0
Fork 0
mirror of https://github.com/denoland/deno.git synced 2024-11-22 15:06:54 -05:00
Code Issues Packages 1 Wiki Activity

fix(npm): only include top level packages in top level node_modules directory (#18824)

Browse source 
We were indeterministically including packages in the top level
`node_modules/` folder when using a local node_modules directory. This
change aligns with pnpm and only includes top level packages in this
folder. This should be faster for initializing the folder, but may
expose issues in packages that reference other packages not defined in
their dependencies. That said, the behaviour previously was previously
broken.

This has exposed a bug in the require implementation where it doesn't
find a package (which is the main underlying issue here). There is a
failing test already for this in the test suite after this change.

Closes #18822

---------

Co-authored-by: Bartek Iwańczuk <biwanczuk@gmail.com>
This commit is contained in:
David Sherret 2023-04-24 17:08:11 -04:00 committed by GitHub
parent be9e3c430f
commit 0e97fa4d5f
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 22 additions and 20 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

14
cli/npm/resolvers/local.rs
View file

@ -4,7 +4,6 @@
use std::borrow::Cow; use std::borrow::Cow;
use std::collections::HashSet; use std::collections::HashSet;
use std::collections::VecDeque;
use std::fs; use std::fs;
use std::path::Path; use std::path::Path;
use std::path::PathBuf; use std::path::PathBuf;
@ -365,18 +364,16 @@ async fn sync_resolution_with_fs(
} }
} }
// 4. Create all the packages in the node_modules folder, which are symlinks. // 4. Create all the top level packages in the node_modules folder, which are symlinks.
// //
// Symlink node_modules/<package_name> to // Symlink node_modules/<package_name> to
// node_modules/.deno/<package_id>/node_modules/<package_name> // node_modules/.deno/<package_id>/node_modules/<package_name>
let mut found_names = HashSet::new(); let mut found_names = HashSet::new();
let mut pending_packages = VecDeque::new(); let mut ids = snapshot.top_level_packages().collect::<Vec<_>>();
pending_packages.extend(snapshot.top_level_packages().map(|id| (id, true))); ids.sort_by(|a, b| b.cmp(a)); // create determinism and only include the latest version
while let Some((id, is_top_level)) = pending_packages.pop_front() { for id in ids {
let root_folder_name = if found_names.insert(id.nv.name.clone()) { let root_folder_name = if found_names.insert(id.nv.name.clone()) {
id.nv.name.clone() id.nv.name.clone()
} else if is_top_level {
id.nv.to_string()
} else { } else {
continue; // skip, already handled continue; // skip, already handled
}; };
@ -394,9 +391,6 @@ async fn sync_resolution_with_fs(
&local_registry_package_path, &local_registry_package_path,
&join_package_name(root_node_modules_dir_path, &root_folder_name), &join_package_name(root_node_modules_dir_path, &root_folder_name),
)?; )?;
for id in package.dependencies.values() {
pending_packages.push_back((id, false));
}
} }
drop(single_process_lock); drop(single_process_lock);

28
ext/node/polyfills/01_require.js
View file

@ -557,15 +557,21 @@ Module._findPath = function (request, paths, isMain, parentPath) {
} }
} }
const isDenoDirPackage = ops.op_require_is_deno_dir_package( let basePath;
curPath,
); if (usesLocalNodeModulesDir) {
const isRelative = ops.op_require_is_request_relative( basePath = pathResolve(curPath, request);
request, } else {
); const isDenoDirPackage = ops.op_require_is_deno_dir_package(
const basePath = (isDenoDirPackage && !isRelative) curPath,
? pathResolve(curPath, packageSpecifierSubPath(request)) );
: pathResolve(curPath, request); const isRelative = ops.op_require_is_request_relative(
request,
);
basePath = (isDenoDirPackage && !isRelative)
? pathResolve(curPath, packageSpecifierSubPath(request))
: pathResolve(curPath, request);
}
let filename; let filename;
const rc = stat(basePath); const rc = stat(basePath);
@ -615,7 +621,9 @@ Module._resolveLookupPaths = function (request, parent) {
return paths; return paths;
} }
if (parent?.filename && parent.filename.length > 0) { if (
!usesLocalNodeModulesDir && parent?.filename && parent.filename.length > 0
) {
const denoDirPath = ops.op_require_resolve_deno_dir( const denoDirPath = ops.op_require_resolve_deno_dir(
request, request,
parent.filename, parent.filename,