1
0
Fork 0
mirror of https://github.com/denoland/deno.git synced 2024-12-21 23:04:45 -05:00

fix(check): move is cjs check from resolving to loading (#25597)

This is required to do when loading because TypeScript handles and
resolves `/// <reference path="..." />` internally.
This commit is contained in:
David Sherret 2024-09-13 14:36:26 +01:00 committed by GitHub
parent f2b53d42ac
commit 1270d9bc93
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 70 additions and 78 deletions

View file

@ -117,27 +117,6 @@ delete Object.prototype.__proto__;
}
}
class SpecifierIsCjsCache {
/** @type {Set<string>} */
#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<string, ts.SourceFile>} */
const sourceFileCache = new Map();
@ -154,7 +133,8 @@ delete Object.prototype.__proto__;
/** @type {Map<string, boolean>} */
const isNodeSourceFileCache = new Map();
const isCjsCache = new SpecifierIsCjsCache();
/** @type {Map<string, boolean>} */
const isCjsCache = new Map();
// Maps asset specifiers to the first scope that the asset was loaded into.
/** @type {Map<string, string | null>} */
@ -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;

View file

@ -468,6 +468,7 @@ struct LoadResponse {
data: String,
version: Option<String>,
script_kind: i32,
is_cjs: bool,
}
#[op2]
@ -483,6 +484,29 @@ fn op_load_inner(
state: &mut OpState,
load_specifier: &str,
) -> Result<Option<LoadResponse>, AnyError> {
fn load_from_node_modules(
specifier: &ModuleSpecifier,
node_resolver: Option<&NodeResolver>,
media_type: &mut MediaType,
is_cjs: &mut bool,
) -> Result<String, AnyError> {
*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::<State>();
let specifier = normalize_specifier(load_specifier, &state.current_dir)
@ -491,6 +515,7 @@ fn op_load_inner(
let mut hash: Option<String> = 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<String>,
) -> Result<Vec<(String, String)>, AnyError> {
) -> Result<Vec<(String, &'static str)>, AnyError> {
op_resolve_inner(
state,
ResolveArgs {
@ -616,9 +642,9 @@ fn op_resolve(
fn op_resolve_inner(
state: &mut OpState,
args: ResolveArgs,
) -> Result<Vec<(String, String)>, AnyError> {
) -> Result<Vec<(String, &'static str)>, AnyError> {
let state = state.borrow_mut::<State>();
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::<State>();
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]