Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(node): load ES modules defined as CJS #22945

Merged
merged 10 commits into from
Mar 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 9 additions & 10 deletions cli/cache/node.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
// Copyright 2018-2024 the Deno authors. All rights reserved. MIT license.

use deno_ast::CjsAnalysis;
use deno_core::error::AnyError;
use deno_core::serde_json;
use deno_runtime::deno_webstorage::rusqlite::params;

use crate::node::CliCjsAnalysis;

use super::cache_db::CacheDB;
use super::cache_db::CacheDBConfiguration;
use super::cache_db::CacheFailure;
Expand Down Expand Up @@ -59,7 +60,7 @@ impl NodeAnalysisCache {
&self,
specifier: &str,
expected_source_hash: &str,
) -> Option<CjsAnalysis> {
) -> Option<CliCjsAnalysis> {
Self::ensure_ok(
self.inner.get_cjs_analysis(specifier, expected_source_hash),
)
Expand All @@ -69,7 +70,7 @@ impl NodeAnalysisCache {
&self,
specifier: &str,
source_hash: &str,
cjs_analysis: &CjsAnalysis,
cjs_analysis: &CliCjsAnalysis,
) {
Self::ensure_ok(self.inner.set_cjs_analysis(
specifier,
Expand All @@ -93,7 +94,7 @@ impl NodeAnalysisCacheInner {
&self,
specifier: &str,
expected_source_hash: &str,
) -> Result<Option<CjsAnalysis>, AnyError> {
) -> Result<Option<CliCjsAnalysis>, AnyError> {
let query = "
SELECT
data
Expand All @@ -118,7 +119,7 @@ impl NodeAnalysisCacheInner {
&self,
specifier: &str,
source_hash: &str,
cjs_analysis: &CjsAnalysis,
cjs_analysis: &CliCjsAnalysis,
) -> Result<(), AnyError> {
let sql = "
INSERT OR REPLACE INTO
Expand Down Expand Up @@ -147,7 +148,7 @@ mod test {
let cache = NodeAnalysisCacheInner::new(conn);

assert!(cache.get_cjs_analysis("file.js", "2").unwrap().is_none());
let cjs_analysis = CjsAnalysis {
let cjs_analysis = CliCjsAnalysis::Cjs {
exports: vec!["export1".to_string()],
reexports: vec!["re-export1".to_string()],
};
Expand All @@ -157,8 +158,7 @@ mod test {
assert!(cache.get_cjs_analysis("file.js", "3").unwrap().is_none()); // different hash
let actual_cjs_analysis =
cache.get_cjs_analysis("file.js", "2").unwrap().unwrap();
assert_eq!(actual_cjs_analysis.exports, cjs_analysis.exports);
assert_eq!(actual_cjs_analysis.reexports, cjs_analysis.reexports);
assert_eq!(actual_cjs_analysis, cjs_analysis);

// adding when already exists should not cause issue
cache
Expand All @@ -170,8 +170,7 @@ mod test {
let cache = NodeAnalysisCacheInner::new(conn);
let actual_analysis =
cache.get_cjs_analysis("file.js", "2").unwrap().unwrap();
assert_eq!(actual_analysis.exports, cjs_analysis.exports);
assert_eq!(actual_analysis.reexports, cjs_analysis.reexports);
assert_eq!(actual_analysis, cjs_analysis);

// now changing the cli version should clear it
let conn = cache.conn.recreate_with_version("2.0.0");
Expand Down
58 changes: 40 additions & 18 deletions cli/node.rs
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
// Copyright 2018-2024 the Deno authors. All rights reserved. MIT license.

use std::borrow::Cow;

use deno_ast::CjsAnalysis;
use deno_ast::MediaType;
use deno_ast::ModuleSpecifier;
use deno_core::error::AnyError;
use deno_runtime::deno_fs;
use deno_runtime::deno_node::analyze::CjsAnalysis as ExtNodeCjsAnalysis;
use deno_runtime::deno_node::analyze::CjsAnalysisExports;
use deno_runtime::deno_node::analyze::CjsCodeAnalyzer;
use deno_runtime::deno_node::analyze::NodeCodeTranslator;
use serde::Deserialize;
use serde::Serialize;

use crate::cache::NodeAnalysisCache;
use crate::util::fs::canonicalize_path_maybe_not_exists;
Expand All @@ -35,6 +35,17 @@ pub fn resolve_specifier_into_node_modules(
.unwrap_or_else(|| specifier.clone())
}

#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
pub enum CliCjsAnalysis {
/// The module was found to be an ES module.
Esm,
/// The module was CJS.
Cjs {
exports: Vec<String>,
reexports: Vec<String>,
},
}

pub struct CliCjsCodeAnalyzer {
cache: NodeAnalysisCache,
fs: deno_fs::FileSystemRc,
Expand All @@ -49,7 +60,7 @@ impl CliCjsCodeAnalyzer {
&self,
specifier: &ModuleSpecifier,
source: &str,
) -> Result<CjsAnalysis, AnyError> {
) -> Result<CliCjsAnalysis, AnyError> {
let source_hash = NodeAnalysisCache::compute_source_hash(source);
if let Some(analysis) = self
.cache
Expand All @@ -60,21 +71,29 @@ impl CliCjsCodeAnalyzer {

let media_type = MediaType::from_specifier(specifier);
if media_type == MediaType::Json {
return Ok(CjsAnalysis {
return Ok(CliCjsAnalysis::Cjs {
exports: vec![],
reexports: vec![],
});
}

let parsed_source = deno_ast::parse_script(deno_ast::ParseParams {
let parsed_source = deno_ast::parse_program(deno_ast::ParseParams {
specifier: specifier.clone(),
text_info: deno_ast::SourceTextInfo::new(source.into()),
media_type,
capture_tokens: true,
scope_analysis: false,
maybe_syntax: None,
})?;
let analysis = parsed_source.analyze_cjs();
let analysis = if parsed_source.is_script() {
let analysis = parsed_source.analyze_cjs();
CliCjsAnalysis::Cjs {
exports: analysis.exports,
reexports: analysis.reexports,
}
} else {
CliCjsAnalysis::Esm
};
self
.cache
.set_cjs_analysis(specifier.as_str(), &source_hash, &analysis);
Expand All @@ -87,20 +106,23 @@ impl CjsCodeAnalyzer for CliCjsCodeAnalyzer {
fn analyze_cjs(
&self,
specifier: &ModuleSpecifier,
source: Option<&str>,
source: Option<String>,
) -> Result<ExtNodeCjsAnalysis, AnyError> {
let source = match source {
Some(source) => Cow::Borrowed(source),
None => Cow::Owned(
self
.fs
.read_text_file_sync(&specifier.to_file_path().unwrap())?,
),
Some(source) => source,
None => self
.fs
.read_text_file_sync(&specifier.to_file_path().unwrap())?,
};
let analysis = self.inner_cjs_analysis(specifier, &source)?;
Ok(ExtNodeCjsAnalysis {
exports: analysis.exports,
reexports: analysis.reexports,
})
match analysis {
CliCjsAnalysis::Esm => Ok(ExtNodeCjsAnalysis::Esm(source)),
CliCjsAnalysis::Cjs { exports, reexports } => {
Ok(ExtNodeCjsAnalysis::Cjs(CjsAnalysisExports {
exports,
reexports,
}))
}
}
}
}
2 changes: 1 addition & 1 deletion cli/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,7 @@ impl NpmModuleLoader {
// translate cjs to esm if it's cjs and inject node globals
self.node_code_translator.translate_cjs_to_esm(
specifier,
Some(code.as_str()),
Some(code),
permissions,
)?
} else {
Expand Down
31 changes: 28 additions & 3 deletions ext/node/analyze.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use std::collections::VecDeque;
use std::path::Path;
use std::path::PathBuf;

use deno_core::anyhow;
use deno_core::anyhow::Context;
use deno_core::ModuleSpecifier;
use once_cell::sync::Lazy;
Expand All @@ -21,7 +22,15 @@ use crate::PackageJson;
use crate::PathClean;

#[derive(Debug, Clone)]
pub struct CjsAnalysis {
pub enum CjsAnalysis {
/// File was found to be an ES module and the translator should
/// load the code as ESM.
Esm(String),
Cjs(CjsAnalysisExports),
}

#[derive(Debug, Clone)]
pub struct CjsAnalysisExports {
pub exports: Vec<String>,
pub reexports: Vec<String>,
}
Expand All @@ -38,7 +47,7 @@ pub trait CjsCodeAnalyzer {
fn analyze_cjs(
&self,
specifier: &ModuleSpecifier,
maybe_source: Option<&str>,
maybe_source: Option<String>,
) -> Result<CjsAnalysis, AnyError>;
}

Expand Down Expand Up @@ -73,14 +82,19 @@ impl<TCjsCodeAnalyzer: CjsCodeAnalyzer> NodeCodeTranslator<TCjsCodeAnalyzer> {
pub fn translate_cjs_to_esm(
&self,
specifier: &ModuleSpecifier,
source: Option<&str>,
source: Option<String>,
permissions: &dyn NodePermissions,
) -> Result<String, AnyError> {
let mut temp_var_count = 0;
let mut handled_reexports: HashSet<String> = HashSet::default();

let analysis = self.cjs_code_analyzer.analyze_cjs(specifier, source)?;

let analysis = match analysis {
CjsAnalysis::Esm(source) => return Ok(source),
CjsAnalysis::Cjs(analysis) => analysis,
};

let mut source = vec![
r#"import {createRequire as __internalCreateRequire} from "node:module";
const require = __internalCreateRequire(import.meta.url);"#
Expand Down Expand Up @@ -127,6 +141,17 @@ impl<TCjsCodeAnalyzer: CjsCodeAnalyzer> NodeCodeTranslator<TCjsCodeAnalyzer> {
reexport, reexport_specifier, referrer
)
})?;
let analysis = match analysis {
CjsAnalysis::Esm(_) => {
// todo(dsherret): support this once supporting requiring ES modules
return Err(anyhow::anyhow!(
"Cannot require ES module '{}' from '{}'",
reexport_specifier,
specifier
));
}
CjsAnalysis::Cjs(analysis) => analysis,
};

for reexport in analysis.reexports {
reexports_to_handle.push_back((reexport, reexport_specifier.clone()));
Expand Down
8 changes: 4 additions & 4 deletions ext/node/package_json.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,10 +93,6 @@ impl PackageJson {
),
};

if source.trim().is_empty() {
return Ok(PackageJson::empty(path));
}

let package_json = Self::load_from_string(path, source)?;
CACHE.with(|cache| {
cache
Expand All @@ -110,6 +106,10 @@ impl PackageJson {
path: PathBuf,
source: String,
) -> Result<PackageJson, AnyError> {
if source.trim().is_empty() {
return Ok(PackageJson::empty(path));
}

let package_json: Value = serde_json::from_str(&source).map_err(|err| {
anyhow::anyhow!(
"malformed package.json: {}\n at {}",
Expand Down
10 changes: 10 additions & 0 deletions tests/specs/node/detect_es_module_defined_as_cjs/__test__.jsonc
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"steps": [{
"args": "run main.ts",
"output": "main.out"
}, {
// ensure it works with cache
"args": "run main.ts",
"output": "main.out"
}]
}
5 changes: 5 additions & 0 deletions tests/specs/node/detect_es_module_defined_as_cjs/deno.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"unstable": [
"byonm"
]
}
1 change: 1 addition & 0 deletions tests/specs/node/detect_es_module_defined_as_cjs/main.out
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
3
3 changes: 3 additions & 0 deletions tests/specs/node/detect_es_module_defined_as_cjs/main.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import { add } from "package";

console.log(add(1, 2));

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Empty file.