summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorNathan Whitaker <17734409+nathanwhit@users.noreply.github.com>2024-09-19 18:37:36 -0700
committerGitHub <noreply@github.com>2024-09-19 18:37:36 -0700
commitf1ba26661346a83b6e7fe5e7ffeed4553a9571ae (patch)
tree2aaa1c5fa8896b0dddb3219f627f32e3ad73afcc
parent5bcea1a9f4eb95cd764047869a17ed74df8f60d1 (diff)
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.
-rw-r--r--cli/node.rs21
-rw-r--r--ext/node_resolver/analyze.rs80
-rw-r--r--tests/specs/run/cjs_reexport_non_analyzable/__test__.jsonc9
-rw-r--r--tests/specs/run/cjs_reexport_non_analyzable/deno.json3
-rw-r--r--tests/specs/run/cjs_reexport_non_analyzable/main.ts3
-rw-r--r--tests/specs/run/cjs_reexport_non_analyzable/node_modules/foo.cjs5
6 files changed, 88 insertions, 33 deletions
diff --git a/cli/node.rs b/cli/node.rs
index 0fd18e299..766da1c01 100644
--- a/cli/node.rs
+++ b/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?;
diff --git a/ext/node_resolver/analyze.rs b/ext/node_resolver/analyze.rs
index 8231cdc8a..aea33e29a 100644
--- a/ext/node_resolver/analyze.rs
+++ b/ext/node_resolver/analyze.rs
@@ -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
- .file_extension_probe(parent.join(specifier), &referrer_path)
- .map(|p| to_file_specifier(&p));
+ return Some(
+ self
+ .file_extension_probe(parent.join(specifier), &referrer_path)
+ .map(|p| to_file_specifier(&p)),
+ )
+ .transpose();
} else {
todo!();
}
@@ -311,28 +315,41 @@ 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
- .node_resolver
- .package_exports_resolve(
- &package_json_path,
- &package_subpath,
- exports,
- Some(referrer),
- NodeModuleKind::Esm,
- conditions,
- mode,
- )
- .map_err(AnyError::from);
+ return Some(
+ self
+ .node_resolver
+ .package_exports_resolve(
+ &package_json_path,
+ &package_subpath,
+ exports,
+ Some(referrer),
+ NodeModuleKind::Esm,
+ conditions,
+ mode,
+ )
+ .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
- .file_extension_probe(d, &referrer_path)
- .map(|p| to_file_specifier(&p));
+ return Some(
+ self
+ .file_extension_probe(d, &referrer_path)
+ .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;
}
diff --git a/tests/specs/run/cjs_reexport_non_analyzable/__test__.jsonc b/tests/specs/run/cjs_reexport_non_analyzable/__test__.jsonc
new file mode 100644
index 000000000..81f29b685
--- /dev/null
+++ b/tests/specs/run/cjs_reexport_non_analyzable/__test__.jsonc
@@ -0,0 +1,9 @@
+{
+ "tempDir": true,
+ "steps": [
+ {
+ "args": "run -A main.ts",
+ "output": ""
+ }
+ ]
+}
diff --git a/tests/specs/run/cjs_reexport_non_analyzable/deno.json b/tests/specs/run/cjs_reexport_non_analyzable/deno.json
new file mode 100644
index 000000000..fde86a1ef
--- /dev/null
+++ b/tests/specs/run/cjs_reexport_non_analyzable/deno.json
@@ -0,0 +1,3 @@
+{
+ "nodeModulesDir": "manual"
+}
diff --git a/tests/specs/run/cjs_reexport_non_analyzable/main.ts b/tests/specs/run/cjs_reexport_non_analyzable/main.ts
new file mode 100644
index 000000000..a9e32af4e
--- /dev/null
+++ b/tests/specs/run/cjs_reexport_non_analyzable/main.ts
@@ -0,0 +1,3 @@
+import assert from "./node_modules/foo.cjs";
+
+assert.equal(1 + 1, 2);
diff --git a/tests/specs/run/cjs_reexport_non_analyzable/node_modules/foo.cjs b/tests/specs/run/cjs_reexport_non_analyzable/node_modules/foo.cjs
new file mode 100644
index 000000000..924d8a96a
--- /dev/null
+++ b/tests/specs/run/cjs_reexport_non_analyzable/node_modules/foo.cjs
@@ -0,0 +1,5 @@
+try {
+ module.exports = require("nonexistent");
+} catch(_e) {
+ module.exports = require("assert");
+}