diff --git a/cli/tsc/99_main_compiler.js b/cli/tsc/99_main_compiler.js index 42bf1055f2..24dda4f172 100644 --- a/cli/tsc/99_main_compiler.js +++ b/cli/tsc/99_main_compiler.js @@ -117,27 +117,6 @@ delete Object.prototype.__proto__; } } - class SpecifierIsCjsCache { - /** @type {Set} */ - #cache = new Set(); - - /** @param {[string, ts.Extension]} param */ - maybeAdd([specifier, ext]) { - if (ext === ".cjs" || ext === ".d.cts" || ext === ".cts") { - this.#cache.add(specifier); - } - } - - add(specifier) { - this.#cache.add(specifier); - } - - /** @param specifier {string} */ - has(specifier) { - return this.#cache.has(specifier); - } - } - // In the case of the LSP, this will only ever contain the assets. /** @type {Map} */ const sourceFileCache = new Map(); @@ -154,7 +133,8 @@ delete Object.prototype.__proto__; /** @type {Map} */ const isNodeSourceFileCache = new Map(); - const isCjsCache = new SpecifierIsCjsCache(); + /** @type {Map} */ + const isCjsCache = new Map(); // Maps asset specifiers to the first scope that the asset was loaded into. /** @type {Map} */ @@ -235,7 +215,7 @@ delete Object.prototype.__proto__; scriptSnapshot, { ...getCreateSourceFileOptions(sourceFileOptions), - impliedNodeFormat: isCjsCache.has(fileName) + impliedNodeFormat: (isCjsCache.get(fileName) ?? false) ? ts.ModuleKind.CommonJS : ts.ModuleKind.ESNext, // in the lsp we want to be able to show documentation @@ -636,18 +616,13 @@ delete Object.prototype.__proto__; if (!fileInfo) { return undefined; } - let { data, scriptKind, version, isCjs } = fileInfo; + const { data, scriptKind, version, isCjs } = fileInfo; assert( data != null, `"data" is unexpectedly null for "${specifier}".`, ); - // use the cache for non-lsp - if (isCjs == null) { - isCjs = isCjsCache.has(specifier); - } else if (isCjs) { - isCjsCache.add(specifier); - } + isCjsCache.set(specifier, isCjs); sourceFile = ts.createSourceFile( specifier, @@ -722,11 +697,10 @@ delete Object.prototype.__proto__; /** @type {[string, ts.Extension] | undefined} */ const resolved = ops.op_resolve( containingFilePath, - isCjsCache.has(containingFilePath), + isCjsCache.get(containingFilePath) ?? false, [fileReference.fileName], )?.[0]; if (resolved) { - isCjsCache.maybeAdd(resolved); return { primary: true, resolvedFileName: resolved[0], @@ -756,13 +730,12 @@ delete Object.prototype.__proto__; /** @type {Array<[string, ts.Extension] | undefined>} */ const resolved = ops.op_resolve( base, - isCjsCache.has(base), + isCjsCache.get(base) ?? false, specifiers, ); if (resolved) { const result = resolved.map((item) => { if (item) { - isCjsCache.maybeAdd(item); const [resolvedFileName, extension] = item; return { resolvedFileName, @@ -841,9 +814,7 @@ delete Object.prototype.__proto__; if (!fileInfo) { return undefined; } - if (fileInfo.isCjs) { - isCjsCache.add(specifier); - } + isCjsCache.set(specifier, fileInfo.isCjs); sourceTextCache.set(specifier, fileInfo.data); scriptVersionCache.set(specifier, fileInfo.version); sourceText = fileInfo.data; diff --git a/cli/tsc/mod.rs b/cli/tsc/mod.rs index 1b443cafd3..6db282f2dc 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, }) ); } @@ -1235,10 +1262,7 @@ mod tests { }, ) .expect("should have invoked op"); - assert_eq!( - actual, - vec![("https://deno.land/x/b.ts".into(), ".ts".into())] - ); + assert_eq!(actual, vec![("https://deno.land/x/b.ts".into(), ".ts")]); } #[tokio::test] @@ -1258,10 +1282,7 @@ mod tests { }, ) .expect("should have not errored"); - assert_eq!( - actual, - vec![(MISSING_DEPENDENCY_SPECIFIER.into(), ".d.ts".into())] - ); + assert_eq!(actual, vec![(MISSING_DEPENDENCY_SPECIFIER.into(), ".d.ts")]); } #[tokio::test]