mirror of
https://github.com/denoland/deno.git
synced 2024-12-25 08:39:09 -05:00
fix(node): inspect ancestor directories when resolving cjs re-exports during analysis (#21104)
If a CJS re-export can't be resolved, it will check the ancestor directories, which is more similar to what `require` does at runtime.
This commit is contained in:
parent
50e4806a2d
commit
9201198efd
7 changed files with 119 additions and 23 deletions
|
@ -924,7 +924,7 @@ impl CliOptions {
|
||||||
}
|
}
|
||||||
|
|
||||||
pub fn has_node_modules_dir(&self) -> bool {
|
pub fn has_node_modules_dir(&self) -> bool {
|
||||||
self.maybe_node_modules_folder.is_some()
|
self.maybe_node_modules_folder.is_some() || self.unstable_byonm()
|
||||||
}
|
}
|
||||||
|
|
||||||
pub fn node_modules_dir_path(&self) -> Option<PathBuf> {
|
pub fn node_modules_dir_path(&self) -> Option<PathBuf> {
|
||||||
|
|
|
@ -2483,6 +2483,73 @@ console.log(add(1, 2));
|
||||||
output.assert_matches_text("Check file:///[WILDCARD]/project-b/main.ts\n");
|
output.assert_matches_text("Check file:///[WILDCARD]/project-b/main.ts\n");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
pub fn byonm_cjs_export_analysis_require_re_export() {
|
||||||
|
let test_context = TestContextBuilder::for_npm().use_temp_cwd().build();
|
||||||
|
let dir = test_context.temp_dir();
|
||||||
|
dir.write(
|
||||||
|
"deno.json",
|
||||||
|
r#"{
|
||||||
|
"unstable": [
|
||||||
|
"byonm"
|
||||||
|
]
|
||||||
|
}"#,
|
||||||
|
);
|
||||||
|
|
||||||
|
dir.write(
|
||||||
|
"package.json",
|
||||||
|
r#"{
|
||||||
|
"name": "test",
|
||||||
|
"packages": {
|
||||||
|
"my-package": "1.0.0"
|
||||||
|
}
|
||||||
|
}
|
||||||
|
"#,
|
||||||
|
);
|
||||||
|
dir.write(
|
||||||
|
"main.js",
|
||||||
|
"import { value1, value2 } from 'my-package';\nconsole.log(value1);\nconsole.log(value2)\n",
|
||||||
|
);
|
||||||
|
|
||||||
|
let node_modules_dir = dir.path().join("node_modules");
|
||||||
|
|
||||||
|
// create a package at node_modules/.multipart/name/nested without a package.json
|
||||||
|
{
|
||||||
|
let pkg_dir = node_modules_dir
|
||||||
|
.join(".multipart")
|
||||||
|
.join("name")
|
||||||
|
.join("nested");
|
||||||
|
pkg_dir.create_dir_all();
|
||||||
|
pkg_dir.join("index.js").write("module.exports.value1 = 5;");
|
||||||
|
}
|
||||||
|
// create a package at node_modules/.multipart/other with a package.json
|
||||||
|
{
|
||||||
|
let pkg_dir = node_modules_dir.join(".multipart").join("other");
|
||||||
|
pkg_dir.create_dir_all();
|
||||||
|
pkg_dir.join("index.js").write("module.exports.value2 = 6;");
|
||||||
|
}
|
||||||
|
// create a package at node_modules/my-package that requires them both
|
||||||
|
{
|
||||||
|
let pkg_dir = node_modules_dir.join("my-package");
|
||||||
|
pkg_dir.create_dir_all();
|
||||||
|
pkg_dir.join("package.json").write_json(&json!({
|
||||||
|
"name": "my-package",
|
||||||
|
"version": "1.0.0",
|
||||||
|
}));
|
||||||
|
pkg_dir
|
||||||
|
.join("index.js")
|
||||||
|
.write("module.exports = { ...require('.multipart/name/nested/index'), ...require('.multipart/other/index.js') }");
|
||||||
|
}
|
||||||
|
|
||||||
|
// the cjs export analysis was preivously failing, but it should
|
||||||
|
// resolve these exports similar to require
|
||||||
|
let output = test_context
|
||||||
|
.new_command()
|
||||||
|
.args("run --allow-read main.js")
|
||||||
|
.run();
|
||||||
|
output.assert_matches_text("5\n6\n");
|
||||||
|
}
|
||||||
|
|
||||||
itest!(imports_package_json {
|
itest!(imports_package_json {
|
||||||
args: "run --node-modules-dir=false npm/imports_package_json/main.js",
|
args: "run --node-modules-dir=false npm/imports_package_json/main.js",
|
||||||
output: "npm/imports_package_json/main.out",
|
output: "npm/imports_package_json/main.out",
|
||||||
|
|
|
@ -193,7 +193,6 @@ impl<TCjsCodeAnalyzer: CjsCodeAnalyzer> NodeCodeTranslator<TCjsCodeAnalyzer> {
|
||||||
}
|
}
|
||||||
|
|
||||||
// We've got a bare specifier or maybe bare_specifier/blah.js"
|
// We've got a bare specifier or maybe bare_specifier/blah.js"
|
||||||
|
|
||||||
let (package_specifier, package_subpath) =
|
let (package_specifier, package_subpath) =
|
||||||
parse_specifier(specifier).unwrap();
|
parse_specifier(specifier).unwrap();
|
||||||
|
|
||||||
|
@ -205,14 +204,13 @@ impl<TCjsCodeAnalyzer: CjsCodeAnalyzer> NodeCodeTranslator<TCjsCodeAnalyzer> {
|
||||||
)?;
|
)?;
|
||||||
|
|
||||||
let package_json_path = module_dir.join("package.json");
|
let package_json_path = module_dir.join("package.json");
|
||||||
if self.fs.exists_sync(&package_json_path) {
|
|
||||||
let package_json = PackageJson::load(
|
let package_json = PackageJson::load(
|
||||||
&*self.fs,
|
&*self.fs,
|
||||||
&*self.npm_resolver,
|
&*self.npm_resolver,
|
||||||
permissions,
|
permissions,
|
||||||
package_json_path.clone(),
|
package_json_path.clone(),
|
||||||
)?;
|
)?;
|
||||||
|
if package_json.exists {
|
||||||
if let Some(exports) = &package_json.exports {
|
if let Some(exports) = &package_json.exports {
|
||||||
return self.node_resolver.package_exports_resolve(
|
return self.node_resolver.package_exports_resolve(
|
||||||
&package_json_path,
|
&package_json_path,
|
||||||
|
@ -232,13 +230,13 @@ impl<TCjsCodeAnalyzer: CjsCodeAnalyzer> NodeCodeTranslator<TCjsCodeAnalyzer> {
|
||||||
if self.fs.is_dir_sync(&d) {
|
if self.fs.is_dir_sync(&d) {
|
||||||
// subdir might have a package.json that specifies the entrypoint
|
// subdir might have a package.json that specifies the entrypoint
|
||||||
let package_json_path = d.join("package.json");
|
let package_json_path = d.join("package.json");
|
||||||
if self.fs.exists_sync(&package_json_path) {
|
|
||||||
let package_json = PackageJson::load(
|
let package_json = PackageJson::load(
|
||||||
&*self.fs,
|
&*self.fs,
|
||||||
&*self.npm_resolver,
|
&*self.npm_resolver,
|
||||||
permissions,
|
permissions,
|
||||||
package_json_path,
|
package_json_path,
|
||||||
)?;
|
)?;
|
||||||
|
if package_json.exists {
|
||||||
if let Some(main) = package_json.main(NodeModuleKind::Cjs) {
|
if let Some(main) = package_json.main(NodeModuleKind::Cjs) {
|
||||||
return Ok(d.join(main).clean());
|
return Ok(d.join(main).clean());
|
||||||
}
|
}
|
||||||
|
@ -253,6 +251,24 @@ impl<TCjsCodeAnalyzer: CjsCodeAnalyzer> NodeCodeTranslator<TCjsCodeAnalyzer> {
|
||||||
return Ok(module_dir.join("index.js").clean());
|
return Ok(module_dir.join("index.js").clean());
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// as a fallback, attempt to resolve it via the ancestor directories
|
||||||
|
let mut last = referrer_path.as_path();
|
||||||
|
while let Some(parent) = last.parent() {
|
||||||
|
if !self.npm_resolver.in_npm_package_at_dir_path(parent) {
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
let path = if parent.ends_with("node_modules") {
|
||||||
|
parent.join(specifier)
|
||||||
|
} else {
|
||||||
|
parent.join("node_modules").join(specifier)
|
||||||
|
};
|
||||||
|
if let Ok(path) = self.file_extension_probe(path, &referrer_path) {
|
||||||
|
return Ok(path);
|
||||||
|
}
|
||||||
|
last = parent;
|
||||||
|
}
|
||||||
|
|
||||||
Err(not_found(specifier, &referrer_path))
|
Err(not_found(specifier, &referrer_path))
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -45,7 +45,6 @@ pub fn err_invalid_package_config(
|
||||||
generic_error(msg)
|
generic_error(msg)
|
||||||
}
|
}
|
||||||
|
|
||||||
#[allow(unused)]
|
|
||||||
pub fn err_module_not_found(path: &str, base: &str, typ: &str) -> AnyError {
|
pub fn err_module_not_found(path: &str, base: &str, typ: &str) -> AnyError {
|
||||||
generic_error(format!(
|
generic_error(format!(
|
||||||
"[ERR_MODULE_NOT_FOUND] Cannot find {typ} \"{path}\" imported from \"{base}\""
|
"[ERR_MODULE_NOT_FOUND] Cannot find {typ} \"{path}\" imported from \"{base}\""
|
||||||
|
|
|
@ -83,7 +83,16 @@ pub trait NpmResolver: std::fmt::Debug + MaybeSend + MaybeSync {
|
||||||
|
|
||||||
fn in_npm_package(&self, specifier: &ModuleSpecifier) -> bool;
|
fn in_npm_package(&self, specifier: &ModuleSpecifier) -> bool;
|
||||||
|
|
||||||
fn in_npm_package_at_path(&self, path: &Path) -> bool {
|
fn in_npm_package_at_dir_path(&self, path: &Path) -> bool {
|
||||||
|
let specifier =
|
||||||
|
match ModuleSpecifier::from_directory_path(path.to_path_buf().clean()) {
|
||||||
|
Ok(p) => p,
|
||||||
|
Err(_) => return false,
|
||||||
|
};
|
||||||
|
self.in_npm_package(&specifier)
|
||||||
|
}
|
||||||
|
|
||||||
|
fn in_npm_package_at_file_path(&self, path: &Path) -> bool {
|
||||||
let specifier =
|
let specifier =
|
||||||
match ModuleSpecifier::from_file_path(path.to_path_buf().clean()) {
|
match ModuleSpecifier::from_file_path(path.to_path_buf().clean()) {
|
||||||
Ok(p) => p,
|
Ok(p) => p,
|
||||||
|
|
|
@ -209,7 +209,7 @@ pub fn op_require_is_deno_dir_package(
|
||||||
#[string] path: String,
|
#[string] path: String,
|
||||||
) -> bool {
|
) -> bool {
|
||||||
let resolver = state.borrow::<NpmResolverRc>();
|
let resolver = state.borrow::<NpmResolverRc>();
|
||||||
resolver.in_npm_package_at_path(&PathBuf::from(path))
|
resolver.in_npm_package_at_file_path(&PathBuf::from(path))
|
||||||
}
|
}
|
||||||
|
|
||||||
#[op2]
|
#[op2]
|
||||||
|
@ -484,7 +484,7 @@ where
|
||||||
let permissions = state.borrow::<P>();
|
let permissions = state.borrow::<P>();
|
||||||
|
|
||||||
let pkg_path = if npm_resolver
|
let pkg_path = if npm_resolver
|
||||||
.in_npm_package_at_path(&PathBuf::from(&modules_path))
|
.in_npm_package_at_file_path(&PathBuf::from(&modules_path))
|
||||||
&& !uses_local_node_modules_dir
|
&& !uses_local_node_modules_dir
|
||||||
{
|
{
|
||||||
modules_path
|
modules_path
|
||||||
|
|
|
@ -110,8 +110,13 @@ impl PackageJson {
|
||||||
path: PathBuf,
|
path: PathBuf,
|
||||||
source: String,
|
source: String,
|
||||||
) -> Result<PackageJson, AnyError> {
|
) -> Result<PackageJson, AnyError> {
|
||||||
let package_json: Value = serde_json::from_str(&source)
|
let package_json: Value = serde_json::from_str(&source).map_err(|err| {
|
||||||
.map_err(|err| anyhow::anyhow!("malformed package.json {}", err))?;
|
anyhow::anyhow!(
|
||||||
|
"malformed package.json: {}\n at {}",
|
||||||
|
err,
|
||||||
|
path.display()
|
||||||
|
)
|
||||||
|
})?;
|
||||||
Self::load_from_value(path, package_json)
|
Self::load_from_value(path, package_json)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
Loading…
Reference in a new issue