fix(npm): fix require resolution if using --node-modules-dir (#17087)

In our `require()` implementation we use a special logic to resolve
"base path" when looking for matching packages, however this logic
is in contradiction to what needs to happen if there's a local
"node_modules"
directory used. This commit changes require implementation to be aware
if we're running off of global node modules cache or a local one.
This commit is contained in:
Bartek Iwańczuk 2022-12-16 23:41:51 +01:00 committed by GitHub
parent 058610b458
commit efcb93f8b9
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 35 additions and 9 deletions

View file

@ -280,6 +280,10 @@ impl CliOptions {
self.overrides.import_map_specifier = Some(path);
}
pub fn node_modules_dir(&self) -> bool {
self.flags.node_modules_dir
}
/// Resolves the path to use for a local node_modules folder.
pub fn resolve_local_node_modules_folder(
&self,

View file

@ -391,14 +391,19 @@ static RESERVED_WORDS: Lazy<HashSet<&str>> = Lazy::new(|| {
pub async fn initialize_runtime(
js_runtime: &mut JsRuntime,
uses_local_node_modules_dir: bool,
) -> Result<(), AnyError> {
let source_code = &format!(
r#"(async function loadBuiltinNodeModules(moduleAllUrl, nodeGlobalThisName) {{
r#"(async function loadBuiltinNodeModules(moduleAllUrl, nodeGlobalThisName, usesLocalNodeModulesDir) {{
const moduleAll = await import(moduleAllUrl);
Deno[Deno.internal].node.initialize(moduleAll.default, nodeGlobalThisName);
}})('{}', '{}');"#,
if (usesLocalNodeModulesDir) {{
Deno[Deno.internal].require.setUsesLocalNodeModulesDir();
}}
}})('{}', '{}', {});"#,
MODULE_ALL_URL.as_str(),
NODE_GLOBAL_THIS_NAME.as_str(),
uses_local_node_modules_dir,
);
let value =

View file

@ -456,7 +456,11 @@ impl ReplSession {
if !npm_imports.is_empty() {
if !self.has_initialized_node_runtime {
self.proc_state.prepare_node_std_graph().await?;
crate::node::initialize_runtime(&mut self.worker.js_runtime).await?;
crate::node::initialize_runtime(
&mut self.worker.js_runtime,
self.proc_state.options.node_modules_dir(),
)
.await?;
self.has_initialized_node_runtime = true;
}

View file

@ -309,7 +309,11 @@ impl CliMainWorker {
async fn initialize_main_module_for_node(&mut self) -> Result<(), AnyError> {
self.ps.prepare_node_std_graph().await?;
node::initialize_runtime(&mut self.worker.js_runtime).await?;
node::initialize_runtime(
&mut self.worker.js_runtime,
self.ps.options.node_modules_dir(),
)
.await?;
if let DenoSubcommand::Run(flags) = self.ps.options.sub_command() {
if let Ok(pkg_ref) = NpmPackageReference::from_str(&flags.script) {
// if the user ran a binary command, we'll need to set process.argv[0]
@ -621,7 +625,11 @@ fn create_web_worker_pre_execute_module_callback(
let fut = async move {
// this will be up to date after pre-load
if ps.npm_resolver.has_packages() {
node::initialize_runtime(&mut worker.js_runtime).await?;
node::initialize_runtime(
&mut worker.js_runtime,
ps.options.node_modules_dir(),
)
.await?;
}
Ok(worker)

View file

@ -71,6 +71,8 @@
let mainModule = null;
let hasBrokenOnInspectBrk = false;
let hasInspectBrk = false;
// Are we running with --node-modules-dir flag?
let usesLocalNodeModulesDir = false;
function stat(filename) {
// TODO: required only on windows
@ -359,10 +361,10 @@
const isRelative = ops.op_require_is_request_relative(
request,
);
// TODO(bartlomieju): could be a single op
const basePath = (isDenoDirPackage && !isRelative)
? pathResolve(curPath, packageSpecifierSubPath(request))
: pathResolve(curPath, request);
const basePath =
(isDenoDirPackage && !isRelative && !usesLocalNodeModulesDir)
? pathResolve(curPath, packageSpecifierSubPath(request))
: pathResolve(curPath, request);
let filename;
const rc = stat(basePath);
@ -915,6 +917,9 @@
window.__bootstrap.internals = {
...window.__bootstrap.internals ?? {},
require: {
setUsesLocalNodeModulesDir() {
usesLocalNodeModulesDir = true;
},
setInspectBrk() {
hasInspectBrk = true;
},