Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(lsp): resolve jsx import source with types mode #25064

Merged
merged 4 commits into from
Aug 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ deno_config = { version = "=0.30.1", features = ["workspace", "sync"] }
deno_core = { workspace = true, features = ["include_js_files_for_snapshotting"] }
deno_doc = { version = "0.146.0", features = ["html", "syntect"] }
deno_emit = "=0.44.0"
deno_graph = { version = "=0.81.2" }
deno_graph = { version = "=0.81.3" }
deno_lint = { version = "=0.63.1", features = ["docs"] }
deno_lockfile.workspace = true
deno_npm = "=0.23.1"
Expand Down
53 changes: 26 additions & 27 deletions cli/lsp/language_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -956,31 +956,27 @@ impl Inner {
.refresh(&self.config.settings, &self.workspace_files, &file_fetcher)
.await;
for config_file in self.config.tree.config_files() {
if let Ok((compiler_options, _)) = config_file.to_compiler_options() {
if let Some(compiler_options_obj) = compiler_options.as_object() {
if let Some(jsx_import_source) =
compiler_options_obj.get("jsxImportSource")
{
if let Some(jsx_import_source) = jsx_import_source.as_str() {
let specifiers = vec![Url::parse(&format!(
"data:application/typescript;base64,{}",
base64::engine::general_purpose::STANDARD
.encode(format!("import '{jsx_import_source}/jsx-runtime';"))
))
.unwrap()];
let referrer = config_file.specifier.clone();
self.task_queue.queue_task(Box::new(|ls: LanguageServer| {
spawn(async move {
if let Err(err) = ls.cache(specifiers, referrer, false).await
{
lsp_warn!("{:#}", err);
}
});
}));
(|| {
let compiler_options = config_file.to_compiler_options().ok()?.0;
let compiler_options_obj = compiler_options.as_object()?;
let jsx_import_source = compiler_options_obj.get("jsxImportSource")?;
let jsx_import_source = jsx_import_source.as_str()?;
let referrer = config_file.specifier.clone();
let specifier = Url::parse(&format!(
"data:application/typescript;base64,{}",
base64::engine::general_purpose::STANDARD
.encode(format!("import '{jsx_import_source}/jsx-runtime';"))
))
.unwrap();
self.task_queue.queue_task(Box::new(|ls: LanguageServer| {
spawn(async move {
if let Err(err) = ls.cache(vec![specifier], referrer, false).await {
lsp_warn!("{:#}", err);
}
}
}
}
});
}));
Some(())
})();
}
}

Expand Down Expand Up @@ -3533,19 +3529,22 @@ impl Inner {
force_global_cache: bool,
) -> Result<PrepareCacheResult, AnyError> {
let config_data = self.config.tree.data_for_specifier(&referrer);
let byonm = config_data.map(|d| d.byonm).unwrap_or(false);
let mut roots = if !specifiers.is_empty() {
specifiers
} else {
vec![referrer.clone()]
};

// always include the npm packages since resolution of one npm package
// might affect the resolution of other npm packages
if let Some(npm_reqs) = self
if byonm {
roots.retain(|s| s.scheme() != "npm");
} else if let Some(npm_reqs) = self
.documents
.npm_reqs_by_scope()
.get(&config_data.map(|d| d.scope.as_ref().clone()))
{
// always include the npm packages since resolution of one npm package
// might affect the resolution of other npm packages
roots.extend(
npm_reqs
.iter()
Expand Down
60 changes: 60 additions & 0 deletions tests/integration/lsp_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11723,6 +11723,66 @@ fn lsp_jsx_import_source_config_file_automatic_cache() {
client.shutdown();
}

#[test]
fn lsp_jsx_import_source_byonm_preact() {
let context = TestContextBuilder::new()
.use_http_server()
.use_temp_cwd()
.add_npm_env_vars()
.build();
let temp_dir = context.temp_dir();
temp_dir.write(
"deno.json",
json!({
"compilerOptions": {
"jsx": "react-jsx",
"jsxImportSource": "npm:preact@^10.19.6"
},
"unstable": ["byonm"],
})
.to_string(),
);
temp_dir.write(
"package.json",
json!({
"dependencies": {
"preact": "^10.19.6",
},
})
.to_string(),
);
let file = source_file(temp_dir.path().join("file.tsx"), r#"<div></div>;"#);
context.run_npm("install");
let mut client = context.new_lsp_command().build();
client.initialize_default();
let diagnostics = client.did_open_file(&file);
assert_eq!(json!(diagnostics.all()), json!([]));
let res = client.write_request(
"textDocument/hover",
json!({
"textDocument": { "uri": file.uri() },
"position": { "line": 0, "character": 1 },
}),
);
assert_eq!(
res,
json!({
"contents": [
{
"language": "typescript",
"value": "(property) JSXInternal.IntrinsicElements.div: JSXInternal.HTMLAttributes<HTMLDivElement>",
},
"",
],
"range": {
"start": { "line": 0, "character": 1 },
"end": { "line": 0, "character": 4 },
},
}),
);
client.shutdown();
}

#[test]
fn lsp_jsx_import_source_types_pragma() {
let context = TestContextBuilder::new()
Expand Down
2 changes: 2 additions & 0 deletions tests/util/server/src/lsp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1301,6 +1301,8 @@ impl SourceFile {
let lang = match path.as_path().extension().unwrap().to_str().unwrap() {
"js" => "javascript",
"ts" | "d.ts" => "typescript",
"jsx" => "javascriptreact",
"tsx" => "typescriptreact",
"json" => "json",
"md" => "markdown",
other => panic!("unsupported file extension: {other}"),
Expand Down