Skip to content

Commit

Permalink
fix(check): move is cjs check from resolving to loading
Browse files Browse the repository at this point in the history
  • Loading branch information
dsherret committed Sep 12, 2024
1 parent 25690c0 commit 9217617
Showing 1 changed file with 60 additions and 33 deletions.
93 changes: 60 additions & 33 deletions cli/tsc/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -468,6 +468,7 @@ struct LoadResponse {
data: String,
version: Option<String>,
script_kind: i32,
is_cjs: bool,
}

#[op2]
Expand All @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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,
Expand All @@ -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
};
Expand All @@ -579,6 +604,7 @@ fn op_load_inner(
data: data.into_owned(),
version: hash,
script_kind: as_ts_script_kind(media_type),
is_cjs,
}))
}

Expand All @@ -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 {
Expand All @@ -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
Expand All @@ -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;
}

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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)]
Expand Down Expand Up @@ -1168,6 +1193,7 @@ mod tests {
"data": "console.log(\"hello deno\");\n",
"version": "7821807483407828376",
"scriptKind": 3,
"isCjs": false,
})
);
}
Expand Down Expand Up @@ -1206,6 +1232,7 @@ mod tests {
"data": "some content",
"version": null,
"scriptKind": 0,
"isCjs": false,
})
);
}
Expand Down

0 comments on commit 9217617

Please sign in to comment.