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

Go to definition sometimes jumps to non JavaScript file #82054

Closed
tschaub opened this issue Oct 7, 2019 · 8 comments
Closed

Go to definition sometimes jumps to non JavaScript file #82054

tschaub opened this issue Oct 7, 2019 · 8 comments
Assignees
Labels
*as-designed Described behavior is as designed javascript JavaScript support issues

Comments

@tschaub
Copy link

tschaub commented Oct 7, 2019

  • VSCode Version: 1.38.1
  • OS Version: macOS 10.14.6

Steps to Reproduce:

  1. Open a JavaScript project with JavaScript dependencies.
  2. Find a function in the project that is defined in one of the dependencies.
  3. Use "Go to Definition" to drill down and find out how that function is implemented.
  4. Sometimes you find what you are looking for: the source of the function. However, sometimes you find a file named something like index.d.ts that is definitely not what you were looking for.

I made two movies of the steps above. In both movies, I'm drilling down three levels: from useMachine to createUseMachine to interpret. In the first movie, I succeed in finding the implementation of interpret. In the second movie, I fail.

Here is the first movie (success):

success

Here is the second movie (failure):
failure

Does this issue occur when all extensions are disabled?: Yes

The behavior I describe above feels like a bug to me. I imagine people who author their projects in TypeScript might call it a feature. But maybe they would admit that the inconsistency (sometimes jumping to a *.js file and sometimes jumping to a *.d.ts file) is unsettling at least. Whether it is a feature or a bug, I'm left with these two questions (which I know are meant for StackOverflow instead of this issue tracker):

  1. As a JavaScript application developer, is it possible to configure VSCode to prefer the JavaScript sources over *.d.ts files in "Go to Definition?"

  2. As a JavaScript library developer, is there anything I can do when publishing my library to make it so users can more easily get to the JavaScript sources and don't accidentally end up jumping to a *.d.ts file?

This "Go to Definition" functionality, IntelliSense more broadly, and all of VSCode are incredible. Thank you so much for your work on them.

@Blasz
Copy link

Blasz commented Oct 8, 2019

Other issues like these have been marked as a duplicate of #68782 so I'm curious if this one will as well.

For the record I agree that this is definitely an issue and something we should be having a conversation about and do not think the justification in #68782 adequately addresses it.

After further digging I found microsoft/TypeScript#6209 which seems to be the root cause of the problem.

@mjbvz could you please update #68782 with a link to microsoft/TypeScript#6209 since a ton of issues have been closed as a duplicate of it and its missing that piece of context.

@strokirk
Copy link

According to @weswigham, "the tools to do it are now in place":
microsoft/TypeScript#6209 (comment)

@mjbvz
Copy link
Collaborator

mjbvz commented Oct 13, 2019

This behavior is by-design. The reason why we cannot jump to 3rd party JS is that JS's dynamic nature makes it impossible to extract type information from in a completely reliable and performant manner. To get around this, VS Code uses d.ts tye definition files to provide intellisense for 3rd party libraries instead.

With microsoft/TypeScript#6209, library authors can generate definition map files using TypeScript (these also work on js files using TS 3.7). These definition maps should be shipped with the library code. They map from the d.ts back to the original source code, so that go to definition in VS Code opens the original source code instead of the d.ts

@mjbvz mjbvz closed this as completed Oct 13, 2019
@mjbvz mjbvz added *as-designed Described behavior is as designed javascript JavaScript support issues labels Oct 13, 2019
@weswigham
Copy link
Member

@mjbvz what we don't do that we could do is combine the declaration maps with normal source maps to map the declaration file back to the js, for projects that ship both declaration maps and source maps, but not the original source.

@tschaub
Copy link
Author

tschaub commented Oct 13, 2019

Thanks for the comments @mjbvz and @weswigham.

I wonder if you might consider it a VSCode bug that sometimes you end up jumping to a *.d.ts file and sometimes you end up jumping to a *.js file for the same function (as described in the example above).

From the perspective of a JavaScript developer, it feels like a risk to generate and publish *.d.ts files if it means that users might end up stuck looking at those files instead of the *.js source files when using "Go to definition" in their editor.

And I know that VSCode does some work to look up *.d.ts files from DefinitelyTyped. This also feels undesirable if it keeps users from seeing the actual source files, but I'm uncertain how I might be able to stop this from happening when publishing a JavaScript library.

VSCode seems to do pretty well extracting documentation and types from JSDoc comments in JavaScript. This feels like a pretty nice experience as both a consumer and publisher of JavaScript. On the other hand, I'm scared to add a *.d.ts file to a project (or to have one show up in DefinitelyTyped) if it means that users will land there instead of in the source file.

@tschaub
Copy link
Author

tschaub commented Oct 13, 2019

Regarding "scared to add a *.d.ts", here is some more detail.

If I am the author of a JavaScript package named dependency, it feels pretty nice to publish a source like this:

/**
 * Indicate whether things work.
 * @param {boolean} maybe It works.
 * @return {string} A message indicating whether it works.
 */
export default function works(maybe) {
  return maybe ? 'yes' : 'no';
}

By "feels nice," I mean that I think it is neat that I can publish a source file that runs on a broad range of engines and communicates both type information to the machine and usage information to the user.

As a user, when I consume the above dependency, I think it is awesome that VSCode provides stuff like this:

image

Right there, all the usage information that the user needs, and presumably enough type information for the machine to warn the user if they get the usage wrong.

As the publisher of the dependency package above, when people ask me to publish *.d.ts files, I get scared because I know that it means that instead of seeing my thoughtfully authored usage documentation and getting nice "Go to definition" functionality, users will land at a *.d.ts file instead of the source.

@Blasz
Copy link

Blasz commented Oct 13, 2019

This behavior is by-design. The reason why we cannot jump to 3rd party JS is that JS's dynamic nature makes it impossible to extract type information from in a completely reliable and performant manner.

I don't understand this, because it is possible to jump to 3rd party JS in a javascript file - at least in some cases, provided that there are no typescript definitions for it. The OP describes one such case.

Are you saying that this is fallback behaviour that is not relied upon because of reliability/performance concerns? And that it's also not possible to provide both behaviours for the user via 'Go to definition' and 'Go to type definition'?

@weswigham
Copy link
Member

I don't understand this, because it is possible to jump to 3rd party JS in a javascript file - at least in some cases, provided that there are no typescript definitions for it. The OP describes one such case.

It's probably worth noting that TS 3.7 will support generating .d.ts files for js files that we understand, and, in theory, declaration maps for those .d.ts files (bug reports on places where the mapping is lacking would be welcome!). Which would then mean that only handwritten .d.ts files included with a package would be problematic (and, ofc, those could be fixed either by generating them or by also hand-generating declaration maps, somehow! A heristic tool to generate best effort maps for existing declaration files and JS files might be neat!)

@vscodebot vscodebot bot locked and limited conversation to collaborators Nov 27, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
*as-designed Described behavior is as designed javascript JavaScript support issues
Projects
None yet
Development

No branches or pull requests

5 participants