mirror of
https://github.com/denoland/deno.git
synced 2024-11-21 15:04:11 -05:00
fix(node): Don't error out if we fail to statically analyze CJS re-export (#25748)
Fixes rsbuild running in deno. You can look at the test to see what was failing, the gist is that we were trying to statically analyze the re-exports of a CJS script, and if we couldn't find the source for the re-exported file we would fail. Instead, we should just treat these as if they were too dynamic to analyze, and let it fail (or succeed) at runtime. This aligns with node's behavior.
This commit is contained in:
parent
5bcea1a9f4
commit
f1ba266613
6 changed files with 88 additions and 33 deletions
21
cli/node.rs
21
cli/node.rs
|
@ -125,10 +125,23 @@ impl CjsCodeAnalyzer for CliCjsCodeAnalyzer {
|
|||
let source = match source {
|
||||
Some(source) => source,
|
||||
None => {
|
||||
self
|
||||
.fs
|
||||
.read_text_file_lossy_async(specifier.to_file_path().unwrap(), None)
|
||||
.await?
|
||||
if let Ok(path) = specifier.to_file_path() {
|
||||
if let Ok(source_from_file) =
|
||||
self.fs.read_text_file_lossy_async(path, None).await
|
||||
{
|
||||
source_from_file
|
||||
} else {
|
||||
return Ok(ExtNodeCjsAnalysis::Cjs(CjsAnalysisExports {
|
||||
exports: vec![],
|
||||
reexports: vec![],
|
||||
}));
|
||||
}
|
||||
} else {
|
||||
return Ok(ExtNodeCjsAnalysis::Cjs(CjsAnalysisExports {
|
||||
exports: vec![],
|
||||
reexports: vec![],
|
||||
}));
|
||||
}
|
||||
}
|
||||
};
|
||||
let analysis = self.inner_cjs_analysis(specifier, &source).await?;
|
||||
|
|
|
@ -200,7 +200,8 @@ impl<TCjsCodeAnalyzer: CjsCodeAnalyzer, TNodeResolverEnv: NodeResolverEnv>
|
|||
NodeResolutionMode::Execution,
|
||||
);
|
||||
let reexport_specifier = match result {
|
||||
Ok(specifier) => specifier,
|
||||
Ok(Some(specifier)) => specifier,
|
||||
Ok(None) => continue,
|
||||
Err(err) => {
|
||||
errors.push(err);
|
||||
continue;
|
||||
|
@ -291,7 +292,7 @@ impl<TCjsCodeAnalyzer: CjsCodeAnalyzer, TNodeResolverEnv: NodeResolverEnv>
|
|||
referrer: &Url,
|
||||
conditions: &[&str],
|
||||
mode: NodeResolutionMode,
|
||||
) -> Result<Url, AnyError> {
|
||||
) -> Result<Option<Url>, AnyError> {
|
||||
if specifier.starts_with('/') {
|
||||
todo!();
|
||||
}
|
||||
|
@ -299,9 +300,12 @@ impl<TCjsCodeAnalyzer: CjsCodeAnalyzer, TNodeResolverEnv: NodeResolverEnv>
|
|||
let referrer_path = referrer.to_file_path().unwrap();
|
||||
if specifier.starts_with("./") || specifier.starts_with("../") {
|
||||
if let Some(parent) = referrer_path.parent() {
|
||||
return self
|
||||
return Some(
|
||||
self
|
||||
.file_extension_probe(parent.join(specifier), &referrer_path)
|
||||
.map(|p| to_file_specifier(&p));
|
||||
.map(|p| to_file_specifier(&p)),
|
||||
)
|
||||
.transpose();
|
||||
} else {
|
||||
todo!();
|
||||
}
|
||||
|
@ -311,17 +315,28 @@ impl<TCjsCodeAnalyzer: CjsCodeAnalyzer, TNodeResolverEnv: NodeResolverEnv>
|
|||
let (package_specifier, package_subpath) =
|
||||
parse_specifier(specifier).unwrap();
|
||||
|
||||
let module_dir = self.npm_resolver.resolve_package_folder_from_package(
|
||||
package_specifier.as_str(),
|
||||
referrer,
|
||||
)?;
|
||||
let module_dir = match self
|
||||
.npm_resolver
|
||||
.resolve_package_folder_from_package(package_specifier.as_str(), referrer)
|
||||
{
|
||||
Err(err)
|
||||
if matches!(
|
||||
err.as_kind(),
|
||||
crate::errors::PackageFolderResolveErrorKind::PackageNotFound(..)
|
||||
) =>
|
||||
{
|
||||
return Ok(None);
|
||||
}
|
||||
other => other,
|
||||
}?;
|
||||
|
||||
let package_json_path = module_dir.join("package.json");
|
||||
let maybe_package_json =
|
||||
load_pkg_json(self.env.pkg_json_fs(), &package_json_path)?;
|
||||
if let Some(package_json) = maybe_package_json {
|
||||
if let Some(exports) = &package_json.exports {
|
||||
return self
|
||||
return Some(
|
||||
self
|
||||
.node_resolver
|
||||
.package_exports_resolve(
|
||||
&package_json_path,
|
||||
|
@ -332,7 +347,9 @@ impl<TCjsCodeAnalyzer: CjsCodeAnalyzer, TNodeResolverEnv: NodeResolverEnv>
|
|||
conditions,
|
||||
mode,
|
||||
)
|
||||
.map_err(AnyError::from);
|
||||
.map_err(AnyError::from),
|
||||
)
|
||||
.transpose();
|
||||
}
|
||||
|
||||
// old school
|
||||
|
@ -345,19 +362,24 @@ impl<TCjsCodeAnalyzer: CjsCodeAnalyzer, TNodeResolverEnv: NodeResolverEnv>
|
|||
load_pkg_json(self.env.pkg_json_fs(), &package_json_path)?;
|
||||
if let Some(package_json) = maybe_package_json {
|
||||
if let Some(main) = package_json.main(NodeModuleKind::Cjs) {
|
||||
return Ok(to_file_specifier(&d.join(main).clean()));
|
||||
return Ok(Some(to_file_specifier(&d.join(main).clean())));
|
||||
}
|
||||
}
|
||||
|
||||
return Ok(to_file_specifier(&d.join("index.js").clean()));
|
||||
return Ok(Some(to_file_specifier(&d.join("index.js").clean())));
|
||||
}
|
||||
return self
|
||||
return Some(
|
||||
self
|
||||
.file_extension_probe(d, &referrer_path)
|
||||
.map(|p| to_file_specifier(&p));
|
||||
.map(|p| to_file_specifier(&p)),
|
||||
)
|
||||
.transpose();
|
||||
} else if let Some(main) = package_json.main(NodeModuleKind::Cjs) {
|
||||
return Ok(to_file_specifier(&module_dir.join(main).clean()));
|
||||
return Ok(Some(to_file_specifier(&module_dir.join(main).clean())));
|
||||
} else {
|
||||
return Ok(to_file_specifier(&module_dir.join("index.js").clean()));
|
||||
return Ok(Some(to_file_specifier(
|
||||
&module_dir.join("index.js").clean(),
|
||||
)));
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -373,7 +395,7 @@ impl<TCjsCodeAnalyzer: CjsCodeAnalyzer, TNodeResolverEnv: NodeResolverEnv>
|
|||
parent.join("node_modules").join(specifier)
|
||||
};
|
||||
if let Ok(path) = self.file_extension_probe(path, &referrer_path) {
|
||||
return Ok(to_file_specifier(&path));
|
||||
return Ok(Some(to_file_specifier(&path)));
|
||||
}
|
||||
last = parent;
|
||||
}
|
||||
|
|
|
@ -0,0 +1,9 @@
|
|||
{
|
||||
"tempDir": true,
|
||||
"steps": [
|
||||
{
|
||||
"args": "run -A main.ts",
|
||||
"output": ""
|
||||
}
|
||||
]
|
||||
}
|
3
tests/specs/run/cjs_reexport_non_analyzable/deno.json
Normal file
3
tests/specs/run/cjs_reexport_non_analyzable/deno.json
Normal file
|
@ -0,0 +1,3 @@
|
|||
{
|
||||
"nodeModulesDir": "manual"
|
||||
}
|
3
tests/specs/run/cjs_reexport_non_analyzable/main.ts
Normal file
3
tests/specs/run/cjs_reexport_non_analyzable/main.ts
Normal file
|
@ -0,0 +1,3 @@
|
|||
import assert from "./node_modules/foo.cjs";
|
||||
|
||||
assert.equal(1 + 1, 2);
|
5
tests/specs/run/cjs_reexport_non_analyzable/node_modules/foo.cjs
generated
vendored
Normal file
5
tests/specs/run/cjs_reexport_non_analyzable/node_modules/foo.cjs
generated
vendored
Normal file
|
@ -0,0 +1,5 @@
|
|||
try {
|
||||
module.exports = require("nonexistent");
|
||||
} catch(_e) {
|
||||
module.exports = require("assert");
|
||||
}
|
Loading…
Reference in a new issue