From 9217617583beaeb43038828043e502c69d90fc70 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Thu, 12 Sep 2024 13:31:01 +0100 Subject: [PATCH] fix(check): move is cjs check from resolving to loading --- cli/tsc/mod.rs | 93 ++++++++++++++++++++++++++++++++------------------ 1 file changed, 60 insertions(+), 33 deletions(-) diff --git a/cli/tsc/mod.rs b/cli/tsc/mod.rs index 1b443cafd3a63c..e3dd88a7b75e33 100644 --- a/cli/tsc/mod.rs +++ b/cli/tsc/mod.rs @@ -468,6 +468,7 @@ struct LoadResponse { data: String, version: Option, script_kind: i32, + is_cjs: bool, } #[op2] @@ -483,6 +484,29 @@ fn op_load_inner( state: &mut OpState, load_specifier: &str, ) -> Result, AnyError> { + fn load_from_node_modules( + specifier: &ModuleSpecifier, + node_resolver: Option<&NodeResolver>, + 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 state = state.borrow_mut::(); let specifier = normalize_specifier(load_specifier, &state.current_dir) @@ -491,6 +515,7 @@ fn op_load_inner( let mut hash: Option = None; let mut media_type = MediaType::Unknown; let graph = &state.graph; + let mut is_cjs = false; let data = if load_specifier == "internal:///.tsbuildinfo" { state.maybe_tsbuildinfo.as_deref().map(Cow::Borrowed) @@ -522,6 +547,7 @@ fn op_load_inner( data: "".to_string(), version: Some("1".to_string()), script_kind: as_ts_script_kind(*media_type), + is_cjs: false, })) } _ => None, @@ -546,26 +572,25 @@ fn op_load_inner( // means it's Deno code importing an npm module let specifier = node::resolve_specifier_into_node_modules(&module.specifier); - media_type = MediaType::from_specifier(&specifier); - 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()) - })?; - Some(Cow::Owned(code)) + Some(Cow::Owned(load_from_node_modules( + &specifier, + state.maybe_npm.as_ref().map(|n| n.node_resolver.as_ref()), + &mut media_type, + &mut is_cjs, + )?)) } } - } else if state + } else if let Some(npm) = state .maybe_npm .as_ref() - .map(|npm| npm.node_resolver.in_npm_package(specifier)) - .unwrap_or(false) + .filter(|npm| npm.node_resolver.in_npm_package(specifier)) { - media_type = MediaType::from_specifier(specifier); - 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()))?; - Some(Cow::Owned(code)) + Some(Cow::Owned(load_from_node_modules( + &specifier, + Some(npm.node_resolver.as_ref()), + &mut media_type, + &mut is_cjs, + )?)) } else { None }; @@ -579,6 +604,7 @@ fn op_load_inner( data: data.into_owned(), version: hash, script_kind: as_ts_script_kind(media_type), + is_cjs, })) } @@ -601,7 +627,7 @@ fn op_resolve( #[string] base: String, is_base_cjs: bool, #[serde] specifiers: Vec, -) -> Result, AnyError> { +) -> Result, AnyError> { op_resolve_inner( state, ResolveArgs { @@ -616,9 +642,9 @@ fn op_resolve( fn op_resolve_inner( state: &mut OpState, args: ResolveArgs, -) -> Result, AnyError> { +) -> Result, AnyError> { let state = state.borrow_mut::(); - let mut resolved: Vec<(String, String)> = + let mut resolved: Vec<(String, &'static str)> = Vec::with_capacity(args.specifiers.len()); let referrer_kind = if args.is_base_cjs { NodeModuleKind::Cjs @@ -640,16 +666,14 @@ fn op_resolve_inner( if specifier.starts_with("node:") { resolved.push(( MISSING_DEPENDENCY_SPECIFIER.to_string(), - MediaType::Dts.to_string(), + MediaType::Dts.as_ts_extension(), )); continue; } if specifier.starts_with("asset:///") { - let media_type = MediaType::from_str(&specifier) - .as_ts_extension() - .to_string(); - resolved.push((specifier, media_type)); + let ext = MediaType::from_str(&specifier).as_ts_extension(); + resolved.push((specifier, ext)); continue; } @@ -694,11 +718,11 @@ fn op_resolve_inner( } } }; - (specifier_str, media_type.as_ts_extension().into()) + (specifier_str, media_type.as_ts_extension()) } None => ( MISSING_DEPENDENCY_SPECIFIER.to_string(), - ".d.ts".to_string(), + MediaType::Dts.as_ts_extension(), ), }; log::debug!("Resolved {} to {:?}", specifier, result); @@ -853,14 +877,15 @@ fn resolve_non_graph_specifier_types( #[op2(fast)] fn op_is_node_file(state: &mut OpState, #[string] path: &str) -> bool { let state = state.borrow::(); - match ModuleSpecifier::parse(path) { - Ok(specifier) => state - .maybe_npm - .as_ref() - .map(|n| n.node_resolver.in_npm_package(&specifier)) - .unwrap_or(false), - Err(_) => false, - } + ModuleSpecifier::parse(path) + .ok() + .and_then(|specifier| { + state + .maybe_npm + .as_ref() + .map(|n| n.node_resolver.in_npm_package(&specifier)) + }) + .unwrap_or(false) } #[derive(Debug, Deserialize, Eq, PartialEq)] @@ -1168,6 +1193,7 @@ mod tests { "data": "console.log(\"hello deno\");\n", "version": "7821807483407828376", "scriptKind": 3, + "isCjs": false, }) ); } @@ -1206,6 +1232,7 @@ mod tests { "data": "some content", "version": null, "scriptKind": 0, + "isCjs": false, }) ); }