From e70341e65e9f4e44811210c9b24e67a29b2c497a Mon Sep 17 00:00:00 2001 From: David Sherret Date: Fri, 25 Oct 2024 13:56:40 -0400 Subject: fix(check): ignore resolving `jsxImportSource` when jsx is not used in graph (#26548) --- cli/tsc/mod.rs | 46 +++++++++++++++++++++++++++++++++++++++------- 1 file changed, 39 insertions(+), 7 deletions(-) (limited to 'cli/tsc/mod.rs') diff --git a/cli/tsc/mod.rs b/cli/tsc/mod.rs index 0e3387494..de91889b6 100644 --- a/cli/tsc/mod.rs +++ b/cli/tsc/mod.rs @@ -5,6 +5,7 @@ use crate::args::TypeCheckMode; use crate::cache::FastInsecureHasher; use crate::node; use crate::npm::CliNpmResolver; +use crate::npm::ResolvePkgFolderFromDenoReqError; use crate::util::checksum; use crate::util::path::mapped_specifier_for_tsc; @@ -35,6 +36,7 @@ use deno_runtime::deno_node::NodeResolver; use deno_semver::npm::NpmPackageReqReference; use node_resolver::errors::NodeJsErrorCode; use node_resolver::errors::NodeJsErrorCoded; +use node_resolver::errors::ResolvePkgSubpathFromDenoModuleError; use node_resolver::NodeModuleKind; use node_resolver::NodeResolution; use node_resolver::NodeResolutionMode; @@ -45,6 +47,7 @@ use std::fmt; use std::path::Path; use std::path::PathBuf; use std::sync::Arc; +use thiserror::Error; mod diagnostics; @@ -688,12 +691,30 @@ fn op_resolve_inner( Some(ResolutionResolved { specifier, .. }) => { resolve_graph_specifier_types(specifier, &referrer, state)? } - _ => resolve_non_graph_specifier_types( - &specifier, - &referrer, - referrer_kind, - state, - )?, + _ => { + match resolve_non_graph_specifier_types( + &specifier, + &referrer, + referrer_kind, + state, + ) { + Ok(maybe_result) => maybe_result, + Err( + err @ ResolveNonGraphSpecifierTypesError::ResolvePkgFolderFromDenoReq( + ResolvePkgFolderFromDenoReqError::Managed(_), + ), + ) => { + // it's most likely requesting the jsxImportSource, which isn't loaded + // into the graph when not using jsx, so just ignore this error + if specifier.ends_with("/jsx-runtime") { + None + } else { + return Err(err.into()); + } + } + Err(err) => return Err(err.into()), + } + } }; let result = match maybe_result { Some((specifier, media_type)) => { @@ -818,12 +839,23 @@ fn resolve_graph_specifier_types( } } +#[derive(Debug, Error)] +enum ResolveNonGraphSpecifierTypesError { + #[error(transparent)] + ResolvePkgFolderFromDenoReq(#[from] ResolvePkgFolderFromDenoReqError), + #[error(transparent)] + ResolvePkgSubpathFromDenoModule(#[from] ResolvePkgSubpathFromDenoModuleError), +} + fn resolve_non_graph_specifier_types( raw_specifier: &str, referrer: &ModuleSpecifier, referrer_kind: NodeModuleKind, state: &State, -) -> Result, AnyError> { +) -> Result< + Option<(ModuleSpecifier, MediaType)>, + ResolveNonGraphSpecifierTypesError, +> { let npm = match state.maybe_npm.as_ref() { Some(npm) => npm, None => return Ok(None), // we only support non-graph types for npm packages -- cgit v1.2.3 From 826e42a5b5880c974ae019a7a21aade6a718062c Mon Sep 17 00:00:00 2001 From: David Sherret Date: Fri, 1 Nov 2024 12:27:00 -0400 Subject: fix: improved support for cjs and cts modules (#26558) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * cts support * better cjs/cts type checking * deno compile cjs/cts support * More efficient detect cjs (going towards stabilization) * Determination of whether .js, .ts, .jsx, or .tsx is cjs or esm is only done after loading * Support `import x = require(...);` Co-authored-by: Bartek Iwańczuk --- cli/tsc/mod.rs | 153 ++++++++++++++++++++++++++++++++++++++++++--------------- 1 file changed, 114 insertions(+), 39 deletions(-) (limited to 'cli/tsc/mod.rs') diff --git a/cli/tsc/mod.rs b/cli/tsc/mod.rs index de91889b6..dc7fc38f7 100644 --- a/cli/tsc/mod.rs +++ b/cli/tsc/mod.rs @@ -3,9 +3,11 @@ use crate::args::TsConfig; use crate::args::TypeCheckMode; use crate::cache::FastInsecureHasher; +use crate::cache::ModuleInfoCache; use crate::node; use crate::npm::CliNpmResolver; use crate::npm::ResolvePkgFolderFromDenoReqError; +use crate::resolver::CjsTracker; use crate::util::checksum; use crate::util::path::mapped_specifier_for_tsc; @@ -32,13 +34,13 @@ use deno_graph::GraphKind; use deno_graph::Module; use deno_graph::ModuleGraph; use deno_graph::ResolutionResolved; +use deno_runtime::deno_fs; use deno_runtime::deno_node::NodeResolver; use deno_semver::npm::NpmPackageReqReference; use node_resolver::errors::NodeJsErrorCode; use node_resolver::errors::NodeJsErrorCoded; -use node_resolver::errors::ResolvePkgSubpathFromDenoModuleError; +use node_resolver::errors::PackageSubpathResolveError; use node_resolver::NodeModuleKind; -use node_resolver::NodeResolution; use node_resolver::NodeResolutionMode; use once_cell::sync::Lazy; use std::borrow::Cow; @@ -302,8 +304,76 @@ pub struct EmittedFile { pub media_type: MediaType, } +pub fn into_specifier_and_media_type( + specifier: Option, +) -> (ModuleSpecifier, MediaType) { + match specifier { + Some(specifier) => { + let media_type = MediaType::from_specifier(&specifier); + + (specifier, media_type) + } + None => ( + Url::parse("internal:///missing_dependency.d.ts").unwrap(), + MediaType::Dts, + ), + } +} + +#[derive(Debug)] +pub struct TypeCheckingCjsTracker { + cjs_tracker: Arc, + module_info_cache: Arc, +} + +impl TypeCheckingCjsTracker { + pub fn new( + cjs_tracker: Arc, + module_info_cache: Arc, + ) -> Self { + Self { + cjs_tracker, + module_info_cache, + } + } + + pub fn is_cjs( + &self, + specifier: &ModuleSpecifier, + media_type: MediaType, + code: &Arc, + ) -> bool { + if let Some(module_kind) = + self.cjs_tracker.get_known_kind(specifier, media_type) + { + module_kind.is_cjs() + } else { + let maybe_is_script = self + .module_info_cache + .as_module_analyzer() + .analyze_sync(specifier, media_type, code) + .ok() + .map(|info| info.is_script); + maybe_is_script + .and_then(|is_script| { + self + .cjs_tracker + .is_cjs_with_known_is_script(specifier, media_type, is_script) + .ok() + }) + .unwrap_or_else(|| { + self + .cjs_tracker + .is_maybe_cjs(specifier, media_type) + .unwrap_or(false) + }) + } + } +} + #[derive(Debug)] pub struct RequestNpmState { + pub cjs_tracker: Arc, pub node_resolver: Arc, pub npm_resolver: Arc, } @@ -456,7 +526,7 @@ pub fn as_ts_script_kind(media_type: MediaType) -> i32 { MediaType::Tsx => 4, MediaType::Json => 6, MediaType::SourceMap - | MediaType::TsBuildInfo + | MediaType::Css | MediaType::Wasm | MediaType::Unknown => 0, } @@ -489,25 +559,22 @@ fn op_load_inner( ) -> Result, AnyError> { fn load_from_node_modules( specifier: &ModuleSpecifier, - node_resolver: Option<&NodeResolver>, + npm_state: Option<&RequestNpmState>, media_type: &mut MediaType, is_cjs: &mut bool, ) -> Result { *media_type = MediaType::from_specifier(specifier); - *is_cjs = node_resolver - .map(|node_resolver| { - match node_resolver.url_to_node_resolution(specifier.clone()) { - Ok(NodeResolution::CommonJs(_)) => true, - Ok(NodeResolution::Esm(_)) - | Ok(NodeResolution::BuiltIn(_)) - | Err(_) => false, - } - }) - .unwrap_or(false); let file_path = specifier.to_file_path().unwrap(); let code = std::fs::read_to_string(&file_path) .with_context(|| format!("Unable to load {}", file_path.display()))?; - Ok(code) + let code: Arc = code.into(); + *is_cjs = npm_state + .map(|npm_state| { + npm_state.cjs_tracker.is_cjs(specifier, *media_type, &code) + }) + .unwrap_or(false); + // todo(dsherret): how to avoid cloning here? + Ok(code.to_string()) } let state = state.borrow_mut::(); @@ -560,6 +627,9 @@ fn op_load_inner( match module { Module::Js(module) => { media_type = module.media_type; + if matches!(media_type, MediaType::Cjs | MediaType::Cts) { + is_cjs = true; + } let source = module .fast_check_module() .map(|m| &*m.source) @@ -573,11 +643,13 @@ fn op_load_inner( Module::Npm(_) | Module::Node(_) => None, Module::External(module) => { // means it's Deno code importing an npm module - let specifier = - node::resolve_specifier_into_node_modules(&module.specifier); + let specifier = node::resolve_specifier_into_node_modules( + &module.specifier, + &deno_fs::RealFs, + ); Some(Cow::Owned(load_from_node_modules( &specifier, - state.maybe_npm.as_ref().map(|n| n.node_resolver.as_ref()), + state.maybe_npm.as_ref(), &mut media_type, &mut is_cjs, )?)) @@ -590,7 +662,7 @@ fn op_load_inner( { Some(Cow::Owned(load_from_node_modules( specifier, - Some(npm.node_resolver.as_ref()), + Some(npm), &mut media_type, &mut is_cjs, )?)) @@ -739,7 +811,13 @@ fn op_resolve_inner( } } }; - (specifier_str, media_type.as_ts_extension()) + ( + specifier_str, + match media_type { + MediaType::Css => ".js", // surface these as .js for typescript + media_type => media_type.as_ts_extension(), + }, + ) } None => ( MISSING_DEPENDENCY_SPECIFIER.to_string(), @@ -810,29 +888,27 @@ fn resolve_graph_specifier_types( Some(referrer), NodeResolutionMode::Types, ); - let maybe_resolution = match res_result { - Ok(res) => Some(res), + let maybe_url = match res_result { + Ok(url) => Some(url), Err(err) => match err.code() { NodeJsErrorCode::ERR_TYPES_NOT_FOUND | NodeJsErrorCode::ERR_MODULE_NOT_FOUND => None, _ => return Err(err.into()), }, }; - Ok(Some(NodeResolution::into_specifier_and_media_type( - maybe_resolution, - ))) + Ok(Some(into_specifier_and_media_type(maybe_url))) } else { Ok(None) } } Some(Module::External(module)) => { // we currently only use "External" for when the module is in an npm package - Ok(state.maybe_npm.as_ref().map(|npm| { - let specifier = - node::resolve_specifier_into_node_modules(&module.specifier); - NodeResolution::into_specifier_and_media_type( - npm.node_resolver.url_to_node_resolution(specifier).ok(), - ) + Ok(state.maybe_npm.as_ref().map(|_| { + let specifier = node::resolve_specifier_into_node_modules( + &module.specifier, + &deno_fs::RealFs, + ); + into_specifier_and_media_type(Some(specifier)) })) } Some(Module::Node(_)) | None => Ok(None), @@ -844,7 +920,7 @@ enum ResolveNonGraphSpecifierTypesError { #[error(transparent)] ResolvePkgFolderFromDenoReq(#[from] ResolvePkgFolderFromDenoReqError), #[error(transparent)] - ResolvePkgSubpathFromDenoModule(#[from] ResolvePkgSubpathFromDenoModuleError), + PackageSubpathResolve(#[from] PackageSubpathResolveError), } fn resolve_non_graph_specifier_types( @@ -863,7 +939,7 @@ fn resolve_non_graph_specifier_types( let node_resolver = &npm.node_resolver; if node_resolver.in_npm_package(referrer) { // we're in an npm package, so use node resolution - Ok(Some(NodeResolution::into_specifier_and_media_type( + Ok(Some(into_specifier_and_media_type( node_resolver .resolve( raw_specifier, @@ -871,7 +947,8 @@ fn resolve_non_graph_specifier_types( referrer_kind, NodeResolutionMode::Types, ) - .ok(), + .ok() + .map(|res| res.into_url()), ))) } else if let Ok(npm_req_ref) = NpmPackageReqReference::from_str(raw_specifier) @@ -890,17 +967,15 @@ fn resolve_non_graph_specifier_types( Some(referrer), NodeResolutionMode::Types, ); - let maybe_resolution = match res_result { - Ok(res) => Some(res), + let maybe_url = match res_result { + Ok(url) => Some(url), Err(err) => match err.code() { NodeJsErrorCode::ERR_TYPES_NOT_FOUND | NodeJsErrorCode::ERR_MODULE_NOT_FOUND => None, _ => return Err(err.into()), }, }; - Ok(Some(NodeResolution::into_specifier_and_media_type( - maybe_resolution, - ))) + Ok(Some(into_specifier_and_media_type(maybe_url))) } else { Ok(None) } -- cgit v1.2.3 From f091d1ad69b4e5217ae3272b641171781a372c4f Mon Sep 17 00:00:00 2001 From: David Sherret Date: Wed, 13 Nov 2024 10:10:09 -0500 Subject: feat(node): stabilize detecting if CJS via `"type": "commonjs"` in a package.json (#26439) This will respect `"type": "commonjs"` in a package.json to determine if `.js`/`.jsx`/`.ts`/.tsx` files are CJS or ESM. If the file is found to be ESM it will be loaded as ESM though. --- cli/tsc/mod.rs | 78 +++++++++++++++++++++++++++++++++++----------------------- 1 file changed, 47 insertions(+), 31 deletions(-) (limited to 'cli/tsc/mod.rs') diff --git a/cli/tsc/mod.rs b/cli/tsc/mod.rs index dc7fc38f7..a56906162 100644 --- a/cli/tsc/mod.rs +++ b/cli/tsc/mod.rs @@ -343,31 +343,36 @@ impl TypeCheckingCjsTracker { media_type: MediaType, code: &Arc, ) -> bool { - if let Some(module_kind) = - self.cjs_tracker.get_known_kind(specifier, media_type) - { - module_kind.is_cjs() - } else { - let maybe_is_script = self - .module_info_cache - .as_module_analyzer() - .analyze_sync(specifier, media_type, code) - .ok() - .map(|info| info.is_script); - maybe_is_script - .and_then(|is_script| { - self - .cjs_tracker - .is_cjs_with_known_is_script(specifier, media_type, is_script) - .ok() - }) - .unwrap_or_else(|| { - self - .cjs_tracker - .is_maybe_cjs(specifier, media_type) - .unwrap_or(false) - }) - } + let maybe_is_script = self + .module_info_cache + .as_module_analyzer() + .analyze_sync(specifier, media_type, code) + .ok() + .map(|info| info.is_script); + maybe_is_script + .and_then(|is_script| { + self + .cjs_tracker + .is_cjs_with_known_is_script(specifier, media_type, is_script) + .ok() + }) + .unwrap_or_else(|| { + self + .cjs_tracker + .is_maybe_cjs(specifier, media_type) + .unwrap_or(false) + }) + } + + pub fn is_cjs_with_known_is_script( + &self, + specifier: &ModuleSpecifier, + media_type: MediaType, + is_script: bool, + ) -> Result { + self + .cjs_tracker + .is_cjs_with_known_is_script(specifier, media_type, is_script) } } @@ -627,8 +632,12 @@ fn op_load_inner( match module { Module::Js(module) => { media_type = module.media_type; - if matches!(media_type, MediaType::Cjs | MediaType::Cts) { - is_cjs = true; + if let Some(npm_state) = &state.maybe_npm { + is_cjs = npm_state.cjs_tracker.is_cjs_with_known_is_script( + specifier, + module.media_type, + module.is_script, + )?; } let source = module .fast_check_module() @@ -737,6 +746,7 @@ fn op_resolve_inner( "Error converting a string module specifier for \"op_resolve\".", )? }; + let referrer_module = state.graph.get(&referrer); for specifier in args.specifiers { if specifier.starts_with("node:") { resolved.push(( @@ -752,16 +762,19 @@ fn op_resolve_inner( continue; } - let graph = &state.graph; - let resolved_dep = graph - .get(&referrer) + let resolved_dep = referrer_module .and_then(|m| m.js()) .and_then(|m| m.dependencies_prefer_fast_check().get(&specifier)) .and_then(|d| d.maybe_type.ok().or_else(|| d.maybe_code.ok())); let maybe_result = match resolved_dep { Some(ResolutionResolved { specifier, .. }) => { - resolve_graph_specifier_types(specifier, &referrer, state)? + resolve_graph_specifier_types( + specifier, + &referrer, + referrer_kind, + state, + )? } _ => { match resolve_non_graph_specifier_types( @@ -834,6 +847,7 @@ fn op_resolve_inner( fn resolve_graph_specifier_types( specifier: &ModuleSpecifier, referrer: &ModuleSpecifier, + referrer_kind: NodeModuleKind, state: &State, ) -> Result, AnyError> { let graph = &state.graph; @@ -886,6 +900,7 @@ fn resolve_graph_specifier_types( &package_folder, module.nv_reference.sub_path(), Some(referrer), + referrer_kind, NodeResolutionMode::Types, ); let maybe_url = match res_result { @@ -965,6 +980,7 @@ fn resolve_non_graph_specifier_types( &package_folder, npm_req_ref.sub_path(), Some(referrer), + referrer_kind, NodeResolutionMode::Types, ); let maybe_url = match res_result { -- cgit v1.2.3 From 617350e79c58b6e01984e3d7c7436d243d0e5cff Mon Sep 17 00:00:00 2001 From: David Sherret Date: Thu, 14 Nov 2024 15:24:25 -0500 Subject: refactor(resolver): move more resolution code into deno_resolver (#26873) Follow-up to cjs refactor. This moves most of the resolution code into the deno_resolver crate. Still pending is the npm resolution code. --- cli/tsc/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'cli/tsc/mod.rs') diff --git a/cli/tsc/mod.rs b/cli/tsc/mod.rs index a56906162..452d5c165 100644 --- a/cli/tsc/mod.rs +++ b/cli/tsc/mod.rs @@ -6,7 +6,6 @@ use crate::cache::FastInsecureHasher; use crate::cache::ModuleInfoCache; use crate::node; use crate::npm::CliNpmResolver; -use crate::npm::ResolvePkgFolderFromDenoReqError; use crate::resolver::CjsTracker; use crate::util::checksum; use crate::util::path::mapped_specifier_for_tsc; @@ -34,6 +33,7 @@ use deno_graph::GraphKind; use deno_graph::Module; use deno_graph::ModuleGraph; use deno_graph::ResolutionResolved; +use deno_resolver::npm::ResolvePkgFolderFromDenoReqError; use deno_runtime::deno_fs; use deno_runtime::deno_node::NodeResolver; use deno_semver::npm::NpmPackageReqReference; -- cgit v1.2.3 From c9baf3849fdbe161a9251a712a71e2b91eeabf3e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Fri, 15 Nov 2024 09:33:03 +0000 Subject: perf: use available system memory for v8 isolate memory limit (#26868) Instead of using the default 1.4Gb limit (which was meant for browser tabs) configure V8 to set the heap limit to the amount of memory available in the system. Closes https://github.com/denoland/deno/issues/23424 Closes https://github.com/denoland/deno/issues/26435 Closes https://github.com/denoland/deno/issues/21226 --- cli/tsc/mod.rs | 2 ++ 1 file changed, 2 insertions(+) (limited to 'cli/tsc/mod.rs') diff --git a/cli/tsc/mod.rs b/cli/tsc/mod.rs index 452d5c165..8f8bed20a 100644 --- a/cli/tsc/mod.rs +++ b/cli/tsc/mod.rs @@ -9,6 +9,7 @@ use crate::npm::CliNpmResolver; use crate::resolver::CjsTracker; use crate::util::checksum; use crate::util::path::mapped_specifier_for_tsc; +use crate::worker::create_isolate_create_params; use deno_ast::MediaType; use deno_core::anyhow::anyhow; @@ -1104,6 +1105,7 @@ pub fn exec(request: Request) -> Result { root_map, remapped_specifiers, )], + create_params: create_isolate_create_params(), ..Default::default() }); -- cgit v1.2.3