lsp: JSDoc imports not recognised

[jespertheend]

Describe the bug
When the deno extension is enabled, typed imports via /** @type {import("... stop working.

To Reproduce

1. Create Bar.js:

export class Bar {}

2. Create Foo.js:

/** @type {import("./Bar.js").Bar} */
const foo = null;

3. Hover over foo and note it's type is set to Bar.
4. Enable deno with Deno: Initialize Workspace Configuration
5. Hover over foo again.

Expected behavior
The type remains set to Bar, rather than any.

Screenshots

If applicable, add screenshots to help explain your problem.

Versions

vscode: 1.62.3 deno: 1.16.3 extension: 3.9.2

[jespertheend]

It seems like something is going wrong inside the tsc code. I've debugged the deno process and as far as I can tell, when hovering over a jsdoc import, deno is simply requesting getQuickInfo and tsc responds with any.
I'm currently debugging the tsc file, if I find anything I'll report here.

[jespertheend]

Alright I'm on to something.
In https://github.com/microsoft/TypeScript/blob/8a5d476ec98fed6d297c70062890dc20b074b4ea/src/compiler/moduleNameResolver.ts#L670 the value for underlying.get("./Bar.js") is undefined when using jsdoc. Whereas it is an object for using typescript imports.

I can't seem to set breakpoints though, so it's hard to figure out where it's being set to undefined.

[jespertheend]

I've added the following to tsc.rs:

let mut ts_runtime = load().expect("could not load tsc");
let inspector = ts_runtime.inspector();
inspector.create_local_session();
inspector.wait_for_session_and_break_on_next_statement();

let server = Arc::new(InspectorServer::new("127.0.0.1:9229".parse().unwrap(), version::get_user_agent()));
server.register_inspector(
    "deno:cli/tsc/99_main_compiler.js".to_string(),
    &mut ts_runtime,
    false,
);

so that I can inspect the typescript language server. But I'm unable to place breakpoints. This makes it next to impossible to figure out why the script isn't loaded.
I tried to attach LLDB to the deno process to figure out why breakpoints don't get placed, but while debugging rust code works, the debugger on v8 code seems to be misaligned and I can't inspect the values of variables. This in turn makes it next to impossible to figure out why breakpoints aren't getting placed.

If anyone has any idea of either:

• Why breakpoints can't be placed when inspecting using the code above
• or on how to setup a debugging environment that can debug v8 c++ code

that'd be really helpful. Because I'm out of ideas.

[bartlomieju]

@jespertheend are you able to connect to the inspector of compiler isolate correctly? Could you record a gif showing what's going on?

[kitsonk]

Trying to debug the isolate is complicated, and it is likely to get much better results by running a specific test with the debug flag set in setup() within the tsc.rs of the lsp and then sending in a fixture to test whatever...

If there isn't sufficient logging in the 99_main_compiler.js, additional debug() and error() statements can be added to provide insight.

[jespertheend]

I've managed to find a codepath in deno's source code that differs between .ts imports and jsdoc imports. So if that's the root cause getting the inspector to work likely isn't necessary.

But fwiw this is what's currently happening:

Screen.Recording.2021-12-20.at.22.36.50.mp4.mp4

The inspector websocket messages only get handled when the lsp is being used, which is to be expected as they're both on the same thread. But I don't get why breakpoints don't work. I've managed to pin it down to matches() in v8-debugger-agent-impl.cc. It iterates over all scripts and tries to find one with the correct url:

https://github.com/v8/v8/blob/17a99fec258bcc07ea9fc5e4fabcce259751db03/src/inspector/v8-debugger-agent-impl.cc#L577

But none of the files match. Is a module loader required in order for breakpoints to work? As far as I can tell that's the only difference between the inspector for deno run and the one I added in tsc.rs.

[jespertheend]

Hmm ok I think I got it. But the issue lies deeper than I had hoped.
The code takes a long journey from op_resolve all the way to the root issue. But the gist of it is that in analyze_dependencies import statements in jsdoc are simply not collected:
https://github.com/denoland/deno_graph/blob/90522def3533ed9f858c3d17562a096afd759bb6/src/ast.rs#L54

This calls into swc code so I'm afraid it's a missing feature from swc.

[jespertheend]

deno/cli/lsp/documents.rs

Lines 1025 to 1059 in 1eb7873

	pub fn resolve(
	&self,
	specifiers: Vec<String>,
	referrer: &ModuleSpecifier,
	) -> Option<Vec<Option<(ModuleSpecifier, MediaType)>>> {
	let dependencies = self.get(referrer)?.0.dependencies.clone();
	let mut results = Vec::new();
	for specifier in specifiers {
	if specifier.starts_with("asset:") {
	if let Ok(specifier) = ModuleSpecifier::parse(&specifier) {
	let media_type = MediaType::from(&specifier);
	results.push(Some((specifier, media_type)));
	} else {
	results.push(None);
	}
	} else if let Some(dep) = dependencies.get(&specifier) {
	if let Some(Ok((specifier, _))) = &dep.maybe_type {
	results.push(self.resolve_dependency(specifier));
	} else if let Some(Ok((specifier, _))) = &dep.maybe_code {
	results.push(self.resolve_dependency(specifier));
	} else {
	results.push(None);
	}
	} else if let Some(Some(Ok((specifier, _)))) =
	self.resolve_imports_dependency(&specifier)
	{
	// clone here to avoid double borrow of self
	let specifier = specifier.clone();
	results.push(self.resolve_dependency(&specifier));
	} else {
	results.push(None);
	}
	}
	Some(results)
	}

This is where it goes wrong. Because the dependencies list is empty, it fails to resolve the imported module.
I think think it might be possible to fix this without making changes to swc.
If you add a regular import at the top of a file, that fixes the jsdoc import:

import {Bar} from "./Bar.js";

/** @type {import("./Bar.js").Bar} */
const foo = null;

[kitsonk]

Ok, thanks for isolating it. It is a problem with deno_graph in that it parse_module does not include those as dependencies of the graph.

[kitsonk]

We need to enhance deno_graph with denoland/deno_graph#91 and when it gets folded back into the CLI, it should "work like magic".

[jespertheend]

I think I have a fix that might be easier. All tests seem to pass but I'm not sure if this causes any imports to resolve even though they shouldn't.

[kitsonk]

@jespertheend as I stated in the PR, it won't work for remote modules. We need to information in module graph.

[jespertheend]

I just found out this duplicates #11362

[jespertheend]

Thanks so much for this @kitsonk! I'm pretty much only using js files with jsdoc imports all over the place. So without this the Deno extension was basically unusable for me.