-
Notifications
You must be signed in to change notification settings - Fork 12.6k
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
Enable Go to Implementation in Typescript Editors #6209
Comments
So you want this functionality to jump to the corresponding |
In addition, another scenario would be enabling a jump to the original typescript .ts file definition in the library being used by the application. I would imagine this information is available pre-compile to d.ts and .js files. We now have entire sets of libraries being written in .ts files as source outputting ES5 syntax js. It would be nice to achieve the ability to jump to the original typescript implementation definition in the .ts files. I'm aware this is asking even more to jump to the .ts file implementation in the required library as it might need source map usage or distribution of source/link to original ts source. It would seem to me that the typescript compiler that generates the initial library of functions and classes would know the lines where the implementation is at and could preserve the line numbers when the library is distributed somehow so that the typscript compiler of the application using the class could know what line to jump to in the library. Is this information on line number of implementing function/class available in source maps? Are there other sources of this information? |
Given that we are going to be using the I don't know how |
Reopening, because going to js files isn't supported quite yet |
Any updates on this one? The current state of typescript is that you cannot jump to the implementation of code in many prominent libraries. This issue has been moved/closed/duped/deduped multiple times and fixing this bug could be a huge productivity boost. https://github.com/ubershmekel/vscode-ts-goto-source has a project that reproduces the issue. As explained in microsoft/vscode#26325 and referenced in microsoft/vscode#10806 and microsoft/vscode#18321 Note I had to use |
@DanielRosenwasser Any updates on this? - Single most wanted feature for me. |
@sabetAI I think the reasonable explanation is that go to implementation has nothing to do with typescript, Actually if you delete all the type declaration files go to implementation works. second edit @justingrant this problem is a purely technical issue, remove my wild guess |
I'm happy to see continued enthusiasm for solving this problem. Keep the upvotes and (especially) constructive suggestions and PRs coming! Like many hard things, eventually I'm sure we'll help to get a solution built for this annoying gap in the TS developer experience.
As far as I can tell, this impression is not correct. Almost all* packages that are written in TS, transpiled to JS, and published to npm don't work with "Go To Implementation" any more than packages originally written in JS. I said "almost all" above because there is a way to make it work: using the declarationMap tsconfig compiler option. But:
Anyone who wants to see this problem get better could:
The community has agency! If we want things to get better, any of the items above can make things better. There's also IMHO another confusion with "Go to Definition" vs. "Go To Type Definition" are implemented. My preference would be for "Go to Type Definition" to navigate to the types and "Go to Definition" to navigate to the implementation. Not sure this is worth changing community expectations, though, given how long the current behavior has been in place. For more details, see: #6209 (comment) |
@weswigham - sorry for delayed reply, I just opened #46627 for this. |
Thanks @justingrant! I want to make a few observations. If I understand correctly, the thing in highest demand here is to be able to jump (whether as an explicit action or automatically under the hood as part of an existing UI action) from a location in a On top of this, I’ll add some assertions that I think most people would agree with:
Putting all this together, I can draw a few intermediate conclusions:
Because of these problems, I’m actually a little confused at the focus of this issue on declaration maps. It seems like they would help only in edge case scenarios, and I think I can categorically reject the idea that we could use them to map from DefinitelyTyped to another source. The only solution I can think of that could solve the problem both for DefinitelyTyped and for bundled type definitions is, at the time of the Go-To-Implementation request, for us to make an effort to find, parse, and bind the corresponding JS file and make an effort to find the symbol in question. This would surely fail in the face of highly dynamic or unusual JS patterns, or where |
Hi @andrewbranch - thanks for jumping in on this important issue. After following it for a few years, I'll add context that may help.
For all the reasons above, I suspect that the right solution is two-tracked:
// mode_modules/some-package/src/Foo.js
import doSomething from 'do-something';
class Foo {
static bar(x) {
doSomething(x);
}
}
export default Foo;
// mayapp.ts
import Foo from 'some-package';
const x = Foo.bar(42); I assume that Go To Implementation for
The bullet-points above are pretty hand-wavey because I don't know enough about TS and Node's module-resolution behavior. But I assume TS and/or Node-resolution experts will know what's possible! @andrewbranch what do you think? You likely understand this stuff better than I do, so please correct me where I'm wrong above. Why navigating to transpiled source is bad
Are you sure about this? Have you heard this sentiment from median-skill developers? I'm skeptical, because transpiled code can be really hard to understand for non-advanced developers. For example, here's an example of the code I saw in VSCode when I first debugged into a method of AWS Amplify, Amazon's JS front-end library that wraps AWS services: onSuccess: function (session) { return __awaiter(_this, void 0, void 0, function () {
var cred, e_1, currentUser, e_2;
return __generator(this, function (_a) {
switch (_a.label) {
case 0:
logger.debug(session);
delete user['challengeName'];
delete user['challengeParam'];
_a.label = 1;
case 1:
_a.trys.push([1, 4, 5, 9]);
return [4 /*yield*/, this.Credentials.clear()];
case 2:
_a.sent();
return [4 /*yield*/, this.Credentials.set(session, 'session')];
case 3:
cred = _a.sent();
logger.debug('succeed to get cognito credentials', cred);
return [3 /*break*/, 9];
case 4:
e_1 = _a.sent();
logger.debug('cannot get cognito credentials', e_1);
return [3 /*break*/, 9];
case 5:
_a.trys.push([5, 7, , 8]);
return [4 /*yield*/, this.currentUserPoolUser()];
case 6:
currentUser = _a.sent();
that.user = currentUser;
dispatchAuthEvent('signIn', currentUser, "A user " + user.getUsername() + " has been signed in");
resolve(currentUser);
return [3 /*break*/, 8];
case 7:
e_2 = _a.sent();
logger.error('Failed to get the signed in user', e_2);
reject(e_2);
return [3 /*break*/, 8];
case 8: return [7 /*endfinally*/];
case 9: return [2 /*return*/];
}
});
}); }, Eek! I assume that most JS developers won't easily understand happening in the state machine above. But after aws-amplify/amplify-js#2680 was merged, debugging into the same method showed the original source, which is much easier to understand: onSuccess: async session => {
logger.debug(session);
delete user['challengeName'];
delete user['challengeParam'];
try {
await this.Credentials.clear();
const cred = await this.Credentials.set(session, 'session');
logger.debug('succeed to get cognito credentials', cred);
} catch (e) {
logger.debug('cannot get cognito credentials', e);
} finally {
try {
// In order to get user attributes and MFA methods
// We need to trigger currentUserPoolUser again
const currentUser = await this.currentUserPoolUser();
that.user = currentUser;
dispatchAuthEvent(
'signIn',
currentUser,
`A user ${user.getUsername()} has been signed in`
);
resolve(currentUser);
} catch (e) {
logger.error('Failed to get the signed in user', e);
reject(e);
}
}
}, I agree that there are some cases where sophisticated users may want to see the transpiled JS. But those are unusual cases. The normal case (and the only case that many unsophisticated developers can handle) is to show the original source, for the same reasons that we show original source in the debugger.
I think you are correct here. It doesn't have to be perfect to add a lot of value. |
Thanks for the detailed explanation, and all good points. I don’t doubt that publishing TS sources to npm for debugging purposes is becoming more common. Likewise, as you also noted, we’re seeing a slow but steady trend of packages moving away from DefinitelyTyped to ship their own types (presumably most of these are autogenerated from TS sources, but some are hand-authored or generated from a tool other than tsc). However, I feel confident that the majority of people’s node_modules today lacks TS implementation files, so any solution that does not include an attempt to resolve symbols to JS, at least when no other option is present, I think will be highly unsatisfying and confusing.
Nope! This was a total guess based on past experience with JS-only VS Code users who get confused (and sometimes angry 😅) when they get dropped into something that looks TypeScripty. I think there are users who would rather get dropped into the JS, but at this point I don’t have evidence to say they make up a particular ratio of all users. But, I don’t think it really matters—if we have all the pieces in place to jump to the exact right place in a I guess my thought process was, if I need to look at implementation files in node_modules, something has already gone horribly, horribly wrong, and I’m bracing for a bad time. I hopefully don’t need to fully understand generator downleveling in order to find a line to put a breakpoint on. But I’m sure my standards here are too low for long-term thinking 😄. It seems like you’re doing a very good job of thinking about how to improve things for the long game, which I very much appreciate. I was admittedly more focused on how to get acceptable results now. Point well taken that we should be thinking about both. It’s probably worth at least doing some writing about the benefits of publishing declaration maps and sources with your packages.
I actually don’t think we know what’s possible without some experiments. My suspicion is that exports will usually be easy, and drilling down into them further will be hit-or-miss, but I don’t know. |
Cool, sounds good. A few responses inline. So glad you're looking into this!
Yep. Those users have a difficult tradeoff: hard-to-read transpiled code or also-hard-to-read (for them) TS code. But this is exactly the same problem they have in the debugger today, and debugging into node_modules is much more common than using Go To Implementation for node_modules. For this reason, I'd recommend thinking of these problems separately.
I think keeping the problems separate will make it easier to make progress on both in parallel without getting stuck in arguments about whether to show TS or JS for Go To Implementation. Question: do declaration maps that TSC emits point to transpiled code or to original source? If the former, then they'll work just like the debugger. 👍 But if the latter, then it seems like it'd be hard to find the transpiled JS. I guess TS could load the entire sourcemap of the package and see if the file/row/col that was resolved by the declaration map also shows up in a sourcemap file, and trace it back from there? Anyway, it'd be good to know whether declaration maps point to transpiled or original source.
👍 Yep! I come back to aligning with the (more commonly used for node_modules navigation) debugger behavior: if there's original source, show that. If not, then try to fall back to something reasonable.
Many years ago, I was working with the VS team back when the original "Just My Code" feature was being built. There was a big debate between the C# and (especially) the C++ team whose users had a long history of viewing, debugging, and copying libraries like MFC vs. the VB team whose users were confused by library code, esp. because libraries were written in C# and C++! What's nice is that both preferences have been improved. For library-code-haters, Just My Code continues to exist and be enhanced. For library-code-lovers, it's gotten easier to view, debug into, and modify library code. But the fact that 15 years later this debate hasn't gone away is a signal that this is a persistent difference between developers. For some devs, viewing library code is an awful, exceptional case. But for many other users, debugging, viewing, and even editing/PR-ing library code is a normal course of business, because libraries are often buggy, poorly documented, or need to be extended in some way. You see this parallel in other walks of life too. For example, if a light switch isn't working, some folks will grab some tools while others will grab a phone and call an electrician. So I think it's helpful to assume that for many developers (but not all!) they will read, debug into, fix, and borrow library code frequently, so we should try to make it work as well as possible. For Just My Code fans, they won't be clicking Go To Implementation anyways (because their preference is to avoid library code!) so I think this feature is really targeted only at the anti-JMC crowd.
Yep, I think the same.
What are next steps to get more attention on this? In #16792 (comment) I wrote a possible outline for this content. I'm happy to help contribute but honestly I feel unqualified to write it because I don't understand enough about the problem space. But I'd love to pair with someone to help make this content happen. LMK how I could help. A good place to start might be to simply create a "Writing libraries in TypeScript" section in the docs and put some minimal content there. Then that section in the docs could be a honeypot to attract tips and tricks from various TS library authors via PRs. In addition to sourcemaps/decl-maps content, there could be sections about how to configure various bundlers, how to migrate from DT to bundling types, and other topics that apply to library authors. |
So one thing I haven't seen mentioned, though I may have missed it in this long thread, I miss seeing the actual code comments, function & parameter descriptions. Sure types are great, but knowing the package authors intention for each function is almost more valuable.
What VSCode gives me: |
@danieliser totally agree—however, if we get a solution to this at all, it’s likely to be a “best guess” effort to find the corresponding JS from the typings. I’m ok with providing a go-to function for that because it’s better than nothing, but I don’t think we’ll achieve high enough confidence (or satisfactory performance) in that guess in order to pull in the JSDoc. This seems like a case where the answer is just to get the package authors to copy the JSDoc comments to the typings (or copy it in a DT PR). If the typings are generated from TS source, the comments will be copied to the typings automatically. If they’re missing, it means someone’s manual process or third-party tooling is insufficient and the problem should be addressed there. |
Please note the milestone change means I plan to investigate solutions to this during the 4.7 timeframe. I don’t think we have a clear path yet, so it doesn’t necessarily mean that something will ship in 4.7. |
I'd like to add a use case: some of our files are authored as The problem is that with such files, it's impossible to "go to definition" / "go to implementation" as VS Code and other editors stop at the |
I have a prototype up at #48264 that I’m welcoming feedback on. I’m 100% sure it doesn’t cover everything but if folks are willing to give it a try, it would be helpful to gather some test cases that don’t work yet so I can see if there’s a good way forward on them. To try it: There’s a build from @typescript-bot on the PR (copied below) that you can npm install, then ensure your VS Code window is using that build of TypeScript by opening the command palette (in a TS or JS file) and running One specific thing to keep an eye out for in testing: There are some cases where we are quite confident in the results we return, other cases where the best we can do is jumping to the top of a file, and then other cases where we’re confident about the file but we make a number of guesses within the file. Under the hood, I am indicating whether or not the result is a guess, but this information isn’t currently exposed in VS Code. If we go forward with presenting guesses like this, it will almost certainly go along with some kind of visual indicator noting that it’s a guess. Here’s an example on Two of these guesses are right, while the top one is wrong. If you notice more places containing wrong guesses like this, please note that and tell me how to repro them on the PR. I need a bit of crowdsourcing to determine whether the guesses are worth doing at all and if there’s an easy way to improve them. Thanks all!
|
The plan is to add an experimental new command in VS Code to trigger the behavior added in #48264. I’ll keep this thread updated to let you know when you can try it out in VS Code Insiders + typescript@next. |
Great progress here. Thanks @andrewbranch for your perseverance in making this happen! Can't wait to try it out. |
I forgot 🤦♂️ You can try this in VS Code Insiders and typescript@next! And pretty soon it will be in the stable versions. I started an issue to gather feedback: #49003. Thanks! |
Already in the newest release: Visual Studio Code April 2022 |
@andrewbranch Thank you thank you thank you for making this happen! You just made my life so much easier |
Could you provide a way for Typescript Editors to list the implementations of a type?
This would definitely help developer productivity to be able to find implementations of types faster.
For example, in the Atom Editor atom-typescript plugin you can press F12 to 'Go to Declaration' which as currently implemented you often arrive at the typescript .d.ts file which has the function signature but not the actual implementation.
Since the Typescript parser often generates the d.ts files from the .ts implementation files. Could that association between the d.ts and .ts files be preserved (perhaps output with a typescript compiler option) so that Typescript editors could take developers to a list of the implementing classes or functions?
I opened a similar request for the Atom-typescript editor, but they indicate that they cannot implement this without Typescript parser support:
TypeStrong/atom-typescript#790
The text was updated successfully, but these errors were encountered: