TypeScript cross-repository go to definition broken with LSIF

[felixfbecker]

Actual

• Go to https://sourcegraph.com/github.com/sourcegraph/about/-/blob/website/src/components/Header.tsx
• Hover over Link in the first line
• Get no hover or go to definition

Expected

As an example of what would be expected, this is how the language server behaves:

• Add to your user settings:

  "codeIntel.lsif": false,
  "typescript.serverUrl": "wss://typescript.sourcegraph.com"

• Navigate to https://sourcegraph.com/github.com/sourcegraph/sourcegraph-typescript/-/blob/src/tracing.ts
• Hover over Span in the first line
• Wait for dependency installation to finish (this would not be expected for LSIF of course 🙂 )
  
• Hover again, click go to definition
• Get taken to https://sourcegraph.com/github.com/opentracing/opentracing-javascript@cb74683fefe5bd4d711eaa0c273f3fc17a6c3781/-/blob/src/span.ts#L11:1
  

There are some special cases here that are considered too, like JS-only libraries that jump to the definition of the typings in https://github.com/DefinitelyTyped/DefinitelyTyped, or references to the standard library, that jump to the lib.d.ts in https://github.com/Microsoft/TypeScript (at the right TypeScript version)

[efritz]

LSIF can currently only jump to locations that are indexed, so if opentracing had an LSIF index cross-references should work.

This isn't unexpected behavior for the state we're at, but if you have behavioral or technical suggestions to bridge the gap in the meantime we'd be happy to hear it.

[felixfbecker]

Could the LSIF indexer use the information from node_modules to determine the target repo, commit, file and position like the language server does it? I wrote an overview of the algorithm back then here: sourcegraph/sourcegraph-typescript#8 (comment)

The "resolve clone URL to repo URL using raw API" step would probably not be done by the indexer, but by the LSIF server or JIT by the code intel extension (not sure how it currently works for LSIF-to-LSIF xrepo j2d). The indexer would just do the steps of the algorithm necessary to produce the information necessary for LSIF's xrepo information encoding mechanism (package monikers etc).

[efritz]

I'm going to bring this up in team discussions and see what the effort/benefit ratio is. Currently I'd say the solution is to index more things! Pop a list of repos into the code-intel channel and we can add it to our auto-indexing effort. I think this is a particularly bad case on dot-com as it can clone these dependencies, where most private instances won't have anything to jump to regardless.

[uwedeportivo]

Dear all,

This is your release captain speaking. 🚂🚂🚂

Branch cut for the 3.16 release is scheduled for tomorrow.

Is this issue / PR going to make it in time? Please change the milestone accordingly.
When in doubt, reach out!

Thank you

[efritz]

Removing from 3.17 - I forgot the context of this issue and now realize that it's an open problem to improve the indexer, and not a simple bugfix.

[efritz]

Moving discussion of this request over to https://github.com/sourcegraph/sourcegraph/issues/13137.

[felixfbecker]

Is that really the root cause though? The repos I'd expect to jump to in the examples in the issue are all cloned on sourcegraph.com, it just seems the definition is not being mapped to those repos (see above for a proposal how that could work). Just showing the file content of a "virtual" file in node_modules, as opposed to jumping to the source repo, is probably not what the user wants, no?

[efritz]

Yes, the new issue covers any case where RepoA depends on RepoB and RepoB doesn't have full information (either because of a missing index or because it's not cloned).

Correct, I'm trying to look for solutions that don't have virtual files where it's not completely necessary.