Skip to content

Commit

Permalink
fix(lsp/check): don't resolve unknown media types to a .js extension (
Browse files Browse the repository at this point in the history
denoland#27631)

Fixes denoland#25762. Note that some of
the things in that issue are not resolved (vite/client types not working
properly which has other root causes), but the wildcard module
augmentation specifically is fixed by this.

We were telling TSC that files with unknown media types had an extension
of `.js`, so the ambient module declarations weren't applying. Instead,
just don't resolve them, so the ambient declaration applies.
  • Loading branch information
nathanwhit authored Jan 11, 2025
1 parent f6dcc13 commit 70c822b
Show file tree
Hide file tree
Showing 4 changed files with 66 additions and 18 deletions.
14 changes: 10 additions & 4 deletions cli/lsp/tsc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4509,11 +4509,12 @@ fn op_release(

#[op2]
#[serde]
#[allow(clippy::type_complexity)]
fn op_resolve(
state: &mut OpState,
#[string] base: String,
#[serde] specifiers: Vec<(bool, String)>,
) -> Result<Vec<Option<(String, String)>>, deno_core::url::ParseError> {
) -> Result<Vec<Option<(String, Option<String>)>>, deno_core::url::ParseError> {
op_resolve_inner(state, ResolveArgs { base, specifiers })
}

Expand Down Expand Up @@ -4595,10 +4596,11 @@ async fn op_poll_requests(
}

#[inline]
#[allow(clippy::type_complexity)]
fn op_resolve_inner(
state: &mut OpState,
args: ResolveArgs,
) -> Result<Vec<Option<(String, String)>>, deno_core::url::ParseError> {
) -> Result<Vec<Option<(String, Option<String>)>>, deno_core::url::ParseError> {
let state = state.borrow_mut::<State>();
let mark = state.performance.mark_with_args("tsc.op.op_resolve", &args);
let referrer = state.specifier_map.normalize(&args.base)?;
Expand All @@ -4611,7 +4613,11 @@ fn op_resolve_inner(
o.map(|(s, mt)| {
(
state.specifier_map.denormalize(&s),
mt.as_ts_extension().to_string(),
if matches!(mt, MediaType::Unknown) {
None
} else {
Some(mt.as_ts_extension().to_string())
},
)
})
})
Expand Down Expand Up @@ -6461,7 +6467,7 @@ mod tests {
resolved,
vec![Some((
temp_dir.url().join("b.ts").unwrap().to_string(),
MediaType::TypeScript.as_ts_extension().to_string()
Some(MediaType::TypeScript.as_ts_extension().to_string())
))]
);
}
Expand Down
8 changes: 4 additions & 4 deletions cli/tsc/99_main_compiler.js
Original file line number Diff line number Diff line change
Expand Up @@ -723,7 +723,7 @@ delete Object.prototype.__proto__;
}
: arg;
if (fileReference.fileName.startsWith("npm:")) {
/** @type {[string, ts.Extension] | undefined} */
/** @type {[string, ts.Extension | null] | undefined} */
const resolved = ops.op_resolve(
containingFilePath,
[
Expand All @@ -735,7 +735,7 @@ delete Object.prototype.__proto__;
],
],
)?.[0];
if (resolved) {
if (resolved && resolved[1]) {
return {
resolvedTypeReferenceDirective: {
primary: true,
Expand Down Expand Up @@ -785,15 +785,15 @@ delete Object.prototype.__proto__;
debug(` base: ${base}`);
debug(` specifiers: ${specifiers.map((s) => s[1]).join(", ")}`);
}
/** @type {Array<[string, ts.Extension] | undefined>} */
/** @type {Array<[string, ts.Extension | null] | undefined>} */
const resolved = ops.op_resolve(
base,
specifiers,
);
if (resolved) {
/** @type {Array<ts.ResolvedModuleWithFailedLookupLocations>} */
const result = resolved.map((item) => {
if (item) {
if (item && item[1]) {
const [resolvedFileName, extension] = item;
return {
resolvedModule: {
Expand Down
27 changes: 17 additions & 10 deletions cli/tsc/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -746,17 +746,17 @@ fn op_resolve(
state: &mut OpState,
#[string] base: String,
#[serde] specifiers: Vec<(bool, String)>,
) -> Result<Vec<(String, &'static str)>, ResolveError> {
) -> Result<Vec<(String, Option<&'static str>)>, ResolveError> {
op_resolve_inner(state, ResolveArgs { base, specifiers })
}

#[inline]
fn op_resolve_inner(
state: &mut OpState,
args: ResolveArgs,
) -> Result<Vec<(String, &'static str)>, ResolveError> {
) -> Result<Vec<(String, Option<&'static str>)>, ResolveError> {
let state = state.borrow_mut::<State>();
let mut resolved: Vec<(String, &'static str)> =
let mut resolved: Vec<(String, Option<&'static str>)> =
Vec::with_capacity(args.specifiers.len());
let referrer = if let Some(remapped_specifier) =
state.maybe_remapped_specifier(&args.base)
Expand All @@ -770,14 +770,14 @@ fn op_resolve_inner(
if specifier.starts_with("node:") {
resolved.push((
MISSING_DEPENDENCY_SPECIFIER.to_string(),
MediaType::Dts.as_ts_extension(),
Some(MediaType::Dts.as_ts_extension()),
));
continue;
}

if specifier.starts_with("asset:///") {
let ext = MediaType::from_str(&specifier).as_ts_extension();
resolved.push((specifier, ext));
resolved.push((specifier, Some(ext)));
continue;
}

Expand Down Expand Up @@ -857,14 +857,15 @@ fn op_resolve_inner(
(
specifier_str,
match media_type {
MediaType::Css => ".js", // surface these as .js for typescript
media_type => media_type.as_ts_extension(),
MediaType::Css => Some(".js"), // surface these as .js for typescript
MediaType::Unknown => None,
media_type => Some(media_type.as_ts_extension()),
},
)
}
None => (
MISSING_DEPENDENCY_SPECIFIER.to_string(),
MediaType::Dts.as_ts_extension(),
Some(MediaType::Dts.as_ts_extension()),
),
};
log::debug!("Resolved {} from {} to {:?}", specifier, referrer, result);
Expand Down Expand Up @@ -1441,7 +1442,10 @@ mod tests {
},
)
.expect("should have invoked op");
assert_eq!(actual, vec![("https://deno.land/x/b.ts".into(), ".ts")]);
assert_eq!(
actual,
vec![("https://deno.land/x/b.ts".into(), Some(".ts"))]
);
}

#[tokio::test]
Expand All @@ -1460,7 +1464,10 @@ mod tests {
},
)
.expect("should have not errored");
assert_eq!(actual, vec![(MISSING_DEPENDENCY_SPECIFIER.into(), ".d.ts")]);
assert_eq!(
actual,
vec![(MISSING_DEPENDENCY_SPECIFIER.into(), Some(".d.ts"))]
);
}

#[tokio::test]
Expand Down
35 changes: 35 additions & 0 deletions tests/integration/lsp_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17221,3 +17221,38 @@ fn lsp_wasm_module() {
);
client.shutdown();
}

#[test]
fn wildcard_augment() {
let context = TestContextBuilder::new().use_temp_cwd().build();
let mut client = context.new_lsp_command().build();
let temp_dir = context.temp_dir().path();
let source = source_file(
temp_dir.join("index.ts"),
r#"
import styles from "./hello_world.scss";
function bar(v: string): string {
return v;
}
bar(styles);
"#,
);
temp_dir.join("index.d.ts").write(
r#"
declare module '*.scss' {
const content: string;
export default content;
}
"#,
);
temp_dir
.join("hello_world.scss")
.write("body { color: red; }");

client.initialize_default();

let diagnostics = client.did_open_file(&source);
assert_eq!(diagnostics.all().len(), 0);
}

0 comments on commit 70c822b

Please sign in to comment.