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

Editor support for @see and {@link} in JSDoc comments tags #35524

Closed
5 tasks done
Elarcis opened this issue Dec 5, 2019 · 28 comments · Fixed by #41877
Closed
5 tasks done

Editor support for @see and {@link} in JSDoc comments tags #35524

Elarcis opened this issue Dec 5, 2019 · 28 comments · Fixed by #41877
Labels
Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Domain: JSDoc Relates to JSDoc parsing and type generation Suggestion An idea for TypeScript

Comments

@Elarcis
Copy link

Elarcis commented Dec 5, 2019

Search Terms

  • jsdoc @link
  • jsdoc @see
  • jsdoc @see @link

Suggestion

Reminder of what {@link} does

Whenever a {@link} tag is encountered in JSDoc, it’d be nice to have it formatted as an actual anchor. It works with URL and symbols relative to the documented one: a function, a property of the current class, or a function in another class?

The @see tag can also reference a symbol without any {@link}, provided there is only a path to a symbol and no free-form text next to it. https://jsdoc.app/tags-see.html

Use Cases

I often speak of other functions/classes in my doc comments, and as {@link} is described on JSDoc’s website, it’d be nice to have it parsed by the langage server and have it shown as a clickable link in any compatible doc widget.

Examples

import { Blah } from "./blah";

/** 
 * This won’t work due to scope: {@link baz}
 * This will: {@link Foo#baz}
 *
 * 
 * The @see annotation can reference symbols as well, as stated on 
 * {@link https://jsdoc.app/tags-see.html JSDoc’s website}
 * @see Document
 */
class Foo<T> {
  /** I want a link to {@link bar}. */
  private baz: T;
  
  /** But I can also reference {@link Foo | the parent class}. */
  public bar(): T {
    return this.baz;
  }

  /** @deprecated please use {@link Blah#grog} instead.  */
  public grog(): void {}

  /** What to do with this link? {@link mysterySymbol} */
  public grug(): void {}
}

Non-imported symbols

A question that subsists is “what to do when {@link} refers to a symbol that exists in the project but isn’t imported in the current file?”

  • Do nothing? {@link mysterySymbol} is converted into a basic non-interactive mysterySymbol? Con is that to get a working link, you’d have to import symbols that are only used for doc.
  • Allow an import("the-module").mysterySymbol syntax like what was done for types declarations?
  • Do a “best guess” and instead of directly going to a file:line:column, find all references of the given symbol in the project and somehow make the editor open a “peek” widget?
  • What if the symbol is located in a dependency, i.e. node_module?

Checklist

My suggestion meets these guidelines:

  • This wouldn't be a breaking change in existing TypeScript/JavaScript code
  • This wouldn't change the runtime behavior of existing JavaScript code
  • This could be implemented without emitting different JS based on the types of the expressions
  • This isn't a runtime feature (e.g. library functionality, non-ECMAScript syntax with JavaScript output, etc.)
  • This feature would agree with the rest of TypeScript's Design Goals.
@RyanCavanaugh RyanCavanaugh added Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Suggestion An idea for TypeScript labels Jan 15, 2020
@yGuy
Copy link

yGuy commented Feb 3, 2020

The JetBrains IDEs have had support for this forever (the see and link syntax is part of the JavaDoc syntax for almost 25 years, now). For me, it's the single most annoying difference in documentation lookup behavior when I switch from WebStorm to VS Code. We even provide a script to our customers so that they can remove all the internal links from the documentation in the d.ts file to make it look less ugly and make it more usable.

WebStorm, too, has the problem of unresolved documentation links when there is no import for the Symbol.

JSDoc actually in a way proposes a solution to this issue with a Syntax to "import" a type from another "module":

/** An event. Its name is module:foo/bar.event:MyEvent.
 * @see module:foo/bar.SomeNonImportedSymbol
 */

Adapted from https://jsdoc.app/about-namepaths.html

@mjbvz mjbvz added the Domain: JSDoc Relates to JSDoc parsing and type generation label Feb 4, 2020
@sandersn
Copy link
Member

sandersn commented Feb 4, 2020

More issues to consider in the proposal:

  1. what kinds of references should be supported? Typescript doesn't support many jsdoc-style namepaths.
  2. what kinds of urls should be supported? I think parsing urls can get hairy fast.
  3. Should invalid @link tags ever have an error?
  4. how should the parser choose between URL and Symbol references? Should it be a best-guess kind of thing?

Some nice-to-haves that we'll need before any implementation happens:

  1. Data on frequency of @link tags and the frequency of URL-vs-symbol as well as various kinds of namepaths.
  2. Thoughts about effects on incremental parsing. (we're probably OK disabling it here, but we should think about it first.)
  3. Thoughts about efficiency and complexity of jsdoc parsing: @link is the only inline tag and the parser currently doesn't have any infrastructure to support that.

Edit: Playing the role of obstinate implementer, I'd say that we could get 80% or more of the benefit of @see and @link just parsing parsing them as special non-tags of the form @see EntityName, which then add a list of references to whatever tag they're on:

/** @param x - a thing; @see foo.bar for details */

Would produce

{ 
  kind: JSDocParameterTag, 
  name: "x", 
  comment: "a thing; @see foo.bar for details",
  references: ["foo.bar"] // or an actual symbol, after binding is done
}

@sandersn
Copy link
Member

sandersn commented Feb 4, 2020

@DanielRosenwasser the previous issue #16498 had 60 upvotes. Let's spend some time thinking again about whether we want this.

@mjbvz
Copy link
Contributor

mjbvz commented Feb 4, 2020

@sandersn Checkout vscode.d.ts for examples of how VS Code handle documentation links today (For example, see: [CodeActionKinds](#CodeActionKind))

It'd be great if we could move away from our homegrown solution that only works inside vscode.d.ts to a more standard one powered by TS

@gangsthub
Copy link

Would it be possible that relative paths were also considered (JSDocs' namepaths)? I was thinking that it could be useful for referencing docs or other assets:

// (might not be the exact syntax)
/**
 * Without the `@link`
 * @see ../README.md#Troubleshooting
 */

// or

/**
 * See {@link ./my_asset.svg}...
 */

@JasonHK
Copy link

JasonHK commented Apr 12, 2020

4. how should the parser choose between URL and Symbol references? Should it be a best-guess kind of thing?

@sandersn I think if the string starts with a protocol and not starts with either module:, external: or event:, then it's a URL.

And also, if the string starts with ./ or ../, then it's a relative path. Omitting ./ was not permissible since it will easily be mixed with namepaths.

By the way, should the namepath syntax follow JSDoc, using # for instance members, . for static members and ~ for inner members?

@DanielRosenwasser DanielRosenwasser changed the title TSServer – support for @see and {@link} in JSDoc comments TSServer – support for @see and {@link} in JSDoc comments tags Apr 22, 2020
@DanielRosenwasser DanielRosenwasser changed the title TSServer – support for @see and {@link} in JSDoc comments tags Editor support for @see and {@link} in JSDoc comments tags Apr 22, 2020
@ackvf
Copy link

ackvf commented Apr 30, 2020

Our code comments sometimes mention symbols in other files. I want to be able to copy such a path LayoutEditor.tsx@acceptsChildren and paste it into goToSymbol field like this:

vscode-go-to-symbol

However, it doesn't work currently and needs to be done in two steps.

taken from microsoft/vscode#96651

@bradennapier
Copy link

bradennapier commented May 21, 2020

Ahh interesting, I was actually playing with implementing some of these types of things in a PR but I ended up keeping it simple to make sure there was wide support for it. I also wasn't sure what was possible since I haven't played with the tsServer in the past.

I have to see this is definitely huge IMO. The rich realtime documentation as you code is probably the most underrated and powerful features to come with modern programming / IDE's - and making these hovers/popups more rich and useful will be a huge plus!

My PR aims to be more general purpose by allowing absolute project & workspace linking, but it does reach into the tsServer to grab the definition of the type being documented. I believe in the same light there are other commands here that could grab symbols locations and provide those links... I just wasn't sure how to link to file & line numbers but prob can add it (assuming tsServer provides necessary pieces) if the PR gains support!

Note: The current implementation is broken for relative paths. This is due to the fact they are utilizing the open file to resolve the relative link. This is problematic since we often are imported values from other files. The PR here fixes that issue for the hovers.

At the very least it may provide insight into the vscode implementation requirements!

microsoft/vscode#98238

@bradennapier
Copy link

bradennapier commented May 21, 2020

A question that subsists is “what to do when {@link} refers to a symbol that exists in the project but isn’t imported in the current file?”

imo it should not do anything and any links should be required in the file linking them. It simply makes things less magical and makes sense. Auto resolution simply has too many potential caveats and problems that can also be specific to the users environment at some times.

import() is interesting option and should prob be supported as long as the preview text is then transformed to the symbol name automatically which is trivial.


While the jsdoc mentions resolution via something like MyType#my I would say it makes the most sense to just support standard typescript semantics, although the question becomes how to handle the required generics. Foo<any>['baz'] -- perhaps allowing them to be omitted in links and/or just provide the signature Foo<T>['baz'] since this wouldn't affect resolving the location of the definition.

{@link Foo<T>['baz']}
// or just
{@link Foo['baz']}

@ghost
Copy link

ghost commented Jun 7, 2020

I just read through all of the previous issues and everything and I have to say I'm rather confused. But I would like to contribute here, based on how big of a task that actually is, but I would definitely like to look into it.

Does anybody have a recommendation for where to start and can maybe outline the current status of this whole issue, what has already been done and what still needs to be done?

I agree with @bradennapier, live documentation via popups in VSCode would be a huge plus for developer experience and I personally would love to use it myself.

Also, what role does TSDoc currently play here? I think it shouldn't only support plain JSDoc fully but also TSDoc.

If there is any kind of work group currently looking into this issue specifically I would love to join.

@JasonHK
Copy link

JasonHK commented Jun 10, 2020

I just read through all of the previous issues and everything and I have to say I'm rather confused. But I would like to contribute here, based on how big of a task that actually is, but I would definitely like to look into it.

Does anybody have a recommendation for where to start and can maybe outline the current status of this whole issue, what has already been done and what still needs to be done?

I agree with @bradennapier, live documentation via popups in VSCode would be a huge plus for developer experience and I personally would love to use it myself.

Also, what role does TSDoc currently play here? I think it shouldn't only support plain JSDoc fully but also TSDoc.

If there is any kind of work group currently looking into this issue specifically I would love to join.

@leontepe I think microsoft/tsdoc is just a reference implementation.

@ghost
Copy link

ghost commented Jun 10, 2020

I think microsoft/tsdoc is just a reference implementation.

As far as I understand it, TSDoc is a documentation standard that's similar to / based on JSDoc but without unnecessary type documentations that are already included in the TypeScript language itself. That's why I was saying it should take that into account as well, IMO.

@JasonHK
Copy link

JasonHK commented Jun 10, 2020

I think microsoft/tsdoc is just a reference implementation.

As far as I understand it, TSDoc is a documentation standard that's similar to / based on JSDoc but without unnecessary type documentations that are already included in the TypeScript language itself. That's why I was saying it should take that into account as well, IMO.

Yeah, I agree.

There are some tags that were supported in TSDoc but not in TypeScript language server, like @link and @remarks tag.

Also, I prefer the way TSDoc handle the @example tag. It always treat it's content as Markdown, even contents that weren't start with ```.

Plus, TSDoc handles @ escape correctly. @tag inside Markdown code block won't treat as TSDoc tag.

@ghost
Copy link

ghost commented Jun 10, 2020

@JasonHK Yeah. And since basically every piece of code Microsoft writes is moving towards TypeScript and away from plain JavaScript, I think they should also have good native support for TSDoc in VSCode.

@qJake
Copy link

qJake commented Sep 16, 2020

I second @yGuy - if I'm a new developer and I want to write JS/TS documentation, I'm going to search for how to do that and I'm going to get results about (long-standing, widely accepted) JSDoc documentation. So, my opinion would be that we stick as close to the JSDoc spec as possible, for backwards compatibility and continuity reasons.

@KilianKilmister
Copy link

@yGuy I don't mind not having this option, and you do make some good points.
I was just curious.

@NoelAbrahams
Copy link

Duplicate of #5802. Glad to see it's been fixed — it's only been five years.

@kajmagnus
Copy link

kajmagnus commented Sep 26, 2020

Would it be a weird idea to optionally log warnings, and optionally even breaking the build, if a @see link is broken?

E.g. the linked-to thing got renamed / moved / removed, but the linking text wasn't updated.

@sandersn wrote:

Should invalid @link tags ever have an error?

Personally I'd want that yes — optionally breaking the build (a compiler flag?). So annoying if trying to understand old code, and it says "blah blah, see: ..." and then that other related code is ... nowhere. (At the same time, if urgently releasing a security patch, then it'd be stressful to have to spend time cleaning up documentation links? So, optionally?)

@qJake
Copy link

qJake commented Oct 1, 2020

Is there currently an option in TSC that behaves similar to Visual Studio's "Treat warnings as errors" option? If so, we could just log them as warnings and use that pre-existing flag to change them to errors if the project called for it?

nweldev pushed a commit to fullwebdev/fullwebdev that referenced this issue Apr 23, 2021
nweldev pushed a commit to fullwebdev/fullwebdev that referenced this issue Apr 24, 2021
@airtonix
Copy link

airtonix commented Mar 7, 2023

Would it be a weird idea to optionally log warnings, and optionally even breaking the build, if a @see link is broken?

E.g. the linked-to thing got renamed / moved / removed, but the linking text wasn't updated.

@sandersn wrote:

Should invalid @link tags ever have an error?

Personally I'd want that yes — optionally breaking the build (a compiler flag?). So annoying if trying to understand old code, and it says "blah blah, see: ..." and then that other related code is ... nowhere. (At the same time, if urgently releasing a security patch, then it'd be stressful to have to spend time cleaning up documentation links? So, optionally?)

This should be an eslint rule (or a whatever-your-lint-tool-is-rule).

I'm not about to install vscode in a github workflow just to lint comments 🤣

@ssalka
Copy link

ssalka commented Apr 18, 2023

Wanted to chime in on this because I thought it already worked this way, but turns out it doesn't:

If I want to do a @see {@link ...} referencing a variable in another file, I shouldn't have to add a literal import statement of the variable to be able to do this with JSDoc. Rather, I'd expect to be able to use TypeScript's existing import() type syntax, which essentially abstracts the literal import away into a comment, which I can feel comfortable knowing it definitely won't have any runtime effect. I could use a literal import type statement, but if I'm only trying to do a @see @link, it feels better to accomplish the import also through JSDoc (keeping the import close to where it is used rather than with actual imports that are needed for runtime behavior).

The import("the-module").mysterySymbol suggestion in the original section on Non-imported symbols seems like the most TypeScript-y way to accomplish this, and follows the heuristic line of thinking "if I can write const x: Type = ..., I should be able to write /** @see {@link Type} */ for any valid Type, including such mysterySymbol import types"

@500-internal-server-error

Could support for differentiating MyClass.staticMethod and MyClass#instanceMethod be added? It seems like a minor yet useful change. Currently I need to do {@link MyClass.instanceMethod | MyClass#instanceMethod} to do this, and even then it's only a visual thing, not enforced to be static/instance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting More Feedback This means we'd like to hear from more people who would be helped by this feature Domain: JSDoc Relates to JSDoc parsing and type generation Suggestion An idea for TypeScript
Projects
None yet
Development

Successfully merging a pull request may close this issue.