RFC: Syntax for "declaration references" (hyperlinks) #9
Comments
|
Me and @MartynasZilinskas have a solution to this. We'll post more info a bit later today / tomorrow. We also have it working in ts-extractor's rewritten version according to what we talked about privately with you (ids, aforementioned ambiguity for same name, different types and not requiring JSON output). We also seem to have solved "declarations vs symbols" thingy and already extract both and also deal with them ids-wise. P.S. You called hyperlinks what we call ids, but I'd rather we call them ids or identifiers, because we can use them in various situations, not only hyperlinks and they have to be unique and point to something specific, which is identifier. |
|
How should internal references be looked up? TypeDoc isn't limited to a single API export so you could have multiple |
octogonz
changed the title
|
@DovydasNavickas I've retitled this from "hyperlinks" to "API item references." This clarifies that we're not just talking about clickable hyperlinks. The name "ids" seemed a little too vague.
Option 1: Require the names to be exportedToday API Extractor only allows references to exported items, using the official exported name. Today it doesn't support multiple entry points, but it would be fairly straightforward. For example, if an API consumer has to write this: import { Button } from '@scope/my-library/ui/controls';Then the /**
* {@link @scope/my-library/ui/controls:Button.render | the render() method}
*/So for a local reference inside of @scope/my-library, we might expect it to disambiguate the entry point like this: /**
* {@link /ui/controls:Button.render | the render() method}
*/Note that the TSDoc layer would not be directly involved in this. The @microsoft/tsdoc library would simply provide a standard notation and parser, which might return an AST node like this: // {@link @scope/my-library/ui/controls:Button.render | the render() method}
{
"nodeKind": "linkNode",
"apiItemReference": {
"scope": "@scope",
"library": "my-library",
"modulePath": "/ui/controls",
"itemPath": "Button.render"
},
"linkText": {
"nodeKind": "textNode",
"content": "the render() method"
}
}Option 2: Rely on the compiler symbolsAnother option would be to rely on the compiler symbols. This is fairly straightforward with today's compiler API, but only if the symbol is actually imported. Example: import { Button as RenamedForSomeReason } from '../../ui/controls';
/**
* {@link RenamedForSomeReason.render | the render() method}
*/ |
// index.ts file in `example-package`
// example-package/index:Foo#class
class Foo {
// example-package/index:Foo#class.a#property
public a: string;
}
// example-package/index:Foo#namespace
namespace Foo {
// example-package/index:Foo#class.a#variable
const a: number;
}We ugpraded the format for specifying api item. I am thinking of adding an additional ApiItem symbol. It will respresent all declarations behind that symbol. "example-package/index:Foo": {
"members": [
"example-package/index:Foo#class",
"example-package/index:Foo#namespace"
]
}When resolving // `example-package` at index.ts
namespace Foo {
const a: number;
}
class Foo {
/**
* Property `a`.
*/
public a: string;
}
// `other-package`
class Bar extends Bar{
/**
* {@inheritdoc example-package/index:Foo#class.a}
*/
public a: string;
}Specifying kind for The only problem I see is overloads, because they have same name and kinds. P.S. Working delimiters |
|
I like this idea of using But I don't think you would ever need the Property getters/setters are another example of multiple declarations for the same symbol, but I feel fairly strongly they should be treated as a single API item. Overloads are an interesting case. C# /// comments would use a notation like However, TypeScript requires all of the overloaded signatures to be right next to each other. They cannot be merged from different files. Also the ordering matters (since it determines matching precedence). So for this case, perhaps we could simply number them? class Widget {
// This is {@link Widget.doSomething#1}
doSomething(name: string, age: number): void;
// This is {@link Widget.doSomething#2}
doSomething(options: { name: string, age: number }): void;
// This is {@link Widget.doSomething#3}
doSomething(): void;
// This is the private implementation
doSomething(whatever?: any, age?: number): void {
}
}It's not super elegant... but then neither are overloads. We don't use overloads all that much since the reflection check has a perf overhead, and the behavior gets weird with subclassing and such. It is a half-baked feature compared to languages such as Java or C++. API Extractor doesn't even give them separate manual pages; we treat overloads as a single function with a complicated type signature. For people who really care about this, a more elaborate solution might be to provide a way to create named monikers. Something like this: class Widget {
/** @overload nameAndAge */
doSomething(name: string, age: number): void;
/** @overload options */
doSomething(options: { name: string, age: number }): void;
/** @overload none */
doSomething(): void;
doSomething(junk?: any, age?: number): void {
}
}
/** The {@link Widget.doSomething#nameAndAge} is the first overload.Any better ideas? |
|
I upgraded {package-name}/{path-to-file}:{selector}#{kind}&{itemCounter} Code sample: export function pickCard(x: { suit: string; card: number }[]): number;
export function pickCard(x: number): { suit: string; card: number };
export function pickCard(x: any): number | { suit: string; card: number } | undefined {
const suits = ["hearts", "spades", "clubs", "diamonds"];
// Check to see if we're working with an object/array
// if so, they gave us the deck and we'll pick the card
if (typeof x == "object") {
let pickedCard = Math.floor(Math.random() * x.length);
return pickedCard;
} else if (typeof x == "number") {
// Otherwise just let them pick the card
let pickedSuit = Math.floor(x / 13);
return { suit: suits[pickedSuit], card: x % 13 };
}
}This how ast item ids look like under the hood. We are using a flat Map for items registry, so items ids need to be unique.
If Simple example /**
* Says Hello world!
*/
export const a: string = "Hello World!";
/**
* {@inheritdoc @simplrjs/package-name/index.ts:a}
*/
export const b: string = "-";Example export namespace A.B.C.D.E {
/**
* Says Hello world!
*/
export const a: string = "Hello World!";
}
/**
* {@inheritdoc @simplrjs/package-name/index.ts:A.B.C.D.E.a}
*/
export const b: string = "-";
If you need to inherit documentation from a specific overload, numbering is the way to go. |
|
@pgonzal said:
@MartynasZilinskas said:
I think we have situation, where we have two different goals:
But:
What I'm a bit more careful about is having those optional parts omitted and reference becoming ambiguous later on in the development, when say another declaration is added to the symbol (e.g. inherit doc or a link gets ambiguous). On the other hand, that might even become a perk. Thoughts? |
|
For the overload part, I strongly disagree using magic numbers to label them. If the performance is a concern, the doc generator may produce extra metadata to help distinguish them (by index, for example) for other tools, but the developer must have the ability to reference overloads by their signatures. The |
|
@yume-chan The concern is not with performance, but how verbose can it become, like @pgonzal pointed out. The order of overload signatures matters, so numbering them makes sense. Consider this example: class Widget {
doSomething(name: string, age: number): void; // Widget.doSomething(string, number)
doSomething(options: { name: string; age: number }): void; // Widget.doSomething({ name: string; age: number })
doSomething(): void; // Widget.doSomething()
}How reference will look like for this overload? doSomething(options: { name: string; age: number }): void;I see the real problem when you have whole bunch of overloads (example). In this case, labeling would be convenient. Usage:
Example: class Widget {
/**
* Do something with name and age.
* @label nameAndAge
*/
doSomething(name: string, age: number): void; // Widget.doSomething#nameAndAge or Widget.doSomething&1
doSomething(options: { name: string; age: number }): void; // Widget.doSomething&2
/**
* No arguments needed to do something.
*/
doSomething(): void; // Widget.doSomething&3
}
class CustomWidget {
/**
* {@inheritdoc Widget.doSomething#nameAndAge}
*/
doSomething(name: string, age: number): void;
/**
* {@inheritdoc Widget.doSomething&3}
*/
doSomething(): void {}
}All possible naming variants so far:
What do you think about
The |
I now understand the problem is the complex type system in TypeScript. I came from C# and I have used to using signature to reference overloads.
I think label is a good idea.
I know, I just used the word "type". But |
Well, in TypeScript's realm they are declarations, not types. And their common symbol is what binds them.
I think this might be a confusing analogy. Also, if we write But then it looks weird for me as it resembles a function call. And if you define reference for property Which goes as And this notation is more than familiar for anyone having done programming. Also, if we adopt the class Foo {
public myProp: string;
/**
* @label magical-overload
*/
public myMethod(a: string): void;
public myMethod(): void;
}This would be the references: |
|
@DovydasNavickas how would your proposed notation handle these two class Vector {
public x: number;
public y: number;
public toString(): string {
return `${this.x}, ${this.y}`;
}
public static toString(x: string, y: string): string {
return `${x}, ${y}`;
}
} |
|
We discussed with @DovydasNavickas offline about handling
// Package: simple-package
class Vector {
public x: number;
public y: number;
// Full id: simple-package:Vector#Class.toString#ClassMethod
public toString(): string {
return `${this.x}, ${this.y}`;
}
// Full id: simple-package:Vector#Class.toString#Static#ClassMethod
public static toString(x: string, y: string): string {
return `${x}, ${y}`;
}
}
class Vector2 {
/**
* Used relative id.
* @inheritDoc Vector.toString#Static#ClassMethod
*/
public static toString(x: string, y: string): string {
return `${x}, ${y}`;
}
}Static method
In this case, we need to specify declaration's syntax kind, because otherwise, we won't know if developer might've made a mistake. class Vector {
public x: number;
public y: number;
// Full id: simple-package:Vector#Class.toString#ClassMethod
public toString(): string {
return `${this.x}, ${this.y}`;
}
// [2] Full id: simple-package:Vector#Class.toString#Static#ClassMethod
public static toString(x: string, y: string): string {
return `${x}, ${y}`;
}
}
class Vector2 {
/**
* [1] Used relative id.
* @inheritDoc Vector.toString
*/
public toString(): string {
return `${this.x}, ${this.y}`;
}
}Should we throw an error about ambiguous identifier on [1] when the static method with the same name is later added at [2]? This is the only shortcoming that we see at the moment. |
|
In the newest prototype of The format that will be used in linking declarations doesn't change. There will be 3 types of separators:
[
|
|
I find the notation here to be a little counterintuitive: Since we generally use InitialCaps for class names, the Here's a possible alternative since everyone expects
For the labels, if the developer is already doing the work to define labels to facilitate documentation generation, then they should be able to ensure that their labels are unique. Thus perhaps we would never need multiple hashtags? For example: |
|
I agree with using |
But what if the class does include a field called "class" or something not a valid JavaScript identifier? |
For that case, I suppose quotation marks would be required and could disambiguate it: class Example {
public 'class': number;
/**
* Assigns the {@link Example['class']} member.
*/
public assignClass() {
this['class'] = 123;
}
}But is this a realistic documentation scenario? Personally I have a clear boundary between "strange constructs that we sometimes need to get the job done" versus "typical constructs that I'd bother documenting". I'm open to other perspectives/counterexamples, though. |
|
@pgonzal I think symbols will be a real use case, like And in my opinion, |
|
BTW I noticed that JSDoc calls this concept "namepaths": http://usejsdoc.org/about-namepaths.html They use |
|
In DocFX, we leverage the concept of UIDs - it would be interesting to see if that is something that we can plug in here as well, to make sure that writers/devs can easily leverage existing conventions. |
octogonz
added
the
tool scenario
|
@MartynasZilinskas I am at the point where I need to implement a parser for the syntax discussed above. I'm going to make it part of the TSDoc library and spec. It sounds like we landed on essentially this: Did you have anything other corrections/enhancements that should be incorporated? |
BTW is this code that you could share? I looked around for it in the master branch but didn't see it. |
|
After chatting with @RyanCavanaugh, we've decided to call this syntax a "declaration reference". He expressed some concern about indexing based on "order of appearance in the source file", and favored the idea of defining explicit labels, such as the |
|
Update: microsoft/rushstack#881 (comment) led to an interesting discussion about how to name overloaded methods in the web site docs. This problem is closely related to the declaration reference selectors. Let us know your opinion -- compare these 3 alternatives: A. require a unique list of parameter names
OR: B. label selectors
OR: C. index selectors
Here's some screenshots @AlexJerabek shared that show a little more context:
|
|
I just had the intent of linking to a package, like so: But it seems not allowed to link to an entire package, I must link to an element INSIDE the package? |
|
The syntax should be: I agree this feels a little counterintuitive. But imagine your package was called |
|
I see.Thanks for the explanation! |
|
Question - does the full name for interface members include The first example in the malformed names section includes it, other names do not: It seems like it might be a typo: tsdoc/spec/code-snippets/DeclarationReferences.ts Lines 284 to 286 in 100e0f0 |
|
That is a typo. Interface members cannot be static. Thanks for pointing this out! Maybe you could open a PR to fix it? I am traveling this week. |
|
According to the AEDoc documentation:
This behavior makes very little sense, is non-DRY, and is highly astonishing because it's completely unlike how TypeScript works. It will cause lots of developer confusion. The obvious expectation is that Please reconsider. |
|
I tend to disagree, TypeDoc currently uses the proposed behavior as it doesn't yet use TSDoc, and this makes it impossible to reference a global Linking based on package entry also helps make sure that my documentation doesn't change because I've imported some new function in a file (which might not even be public!) |
@argv-minus-one I agree that this a somewhat obvious expectation. However, I don't know how we could realistically implement it without a lot of work. For example, suppose I wrote something like this: import Widget from 'widget-lib';
/**
* Generates a new id for a {@link Widget}.
*/
export function generateWidgetId(): string;This code produces an output .d.ts file that's missing the To address this, we could improve TypeScript and TSLint to understand the Suppose we did all that stuff. There's some further consequences: API Extractor doesn't generate docs directly. Instead, it writes an .api.json file for each NPM package, so that API Documenter can come along and produce an integrated website with proper hyperlinking between all the packages. The .api.json stage also enables people to develop their packages in different Git repos. These JSON files contain the TSDoc and declaration references but don't have Okay but if we implemented all that stuff, file-relative would be doable, and it's probably a more intuitive for developers. By the time we got this working, there'd be a lot of legacy code using the old notation, so we maybe should define a special operator (let's say |
|
What about defining a syntax for declaration references in doc comments that's independent of TSDoc tags? You might decide that anything surrounded by TypeScript then knows that There would need to be some other syntax to get different link text. The obvious choice is I don't understand how the rollup issue applies only to declaration references, and not to rolling up declaration files in general. Suppose you have to roll this up: // module1.d.ts
import { Widget } from "one-widget-package";
export declare class MyWidget extends Widget { … }// module2.d.ts
import { Widget } from "another-widget-package";
export declare class MyOtherWidget extends Widget { … }You can't just naïvely concatenate them: // rollup.d.ts
import { Widget } from "one-widget-package";
export declare class MyWidget extends Widget { … }
import { Widget /* Name collision! */ } from "another-widget-package";
export declare class MyOtherWidget extends Widget /* Which Widget? */ { … }Doesn't there already have to be some sort of disambiguation done to solve this problem? If so, why isn't that solution also applicable to declaration references? For example, if you disambiguate by renaming imports: // module1.d.ts
import { Widget } from "one-widget-package";
/** It's a kind of [[Widget]]. */
export declare class MyWidget extends Widget { … }// module2.d.ts
import { Widget } from "another-widget-package";
/** It's a different kind of [[Widget]]. */
export declare class MyOtherWidget extends Widget { … }// rollup.d.ts
import { Widget } from "one-widget-package";
/** It's a kind of [[Widget]]. */
export declare class MyWidget extends Widget { … }
import { Widget as Widget2 } from "another-widget-package";
/** It's a different kind of [[Widget2]]. */
export declare class MyOtherWidget extends Widget2 { … } |
|
@rbuckton has been working on a simplification of the declaration syntax that also makes it more consistent with JSDoc namepaths. The initial work is committed here:
And a bunch of examples can be found by searching for |
Just link to disambiguation page and the problem is solved. |
export class A {
public b: string = 'hello';
}
export namespace A {
export let b: number = 123;
}
let a = new A();
// a.b is a string
// A.b is a number
FYI @rbuckton's updated syntax uses different operators to disambiguate them:
But where it is NOT ambiguous (for example because there is no static member), then we won't consider it an error to write (Ron has made a lot of progress with overhauling the parser to integrate this new grammar, but it isn't a simple update. It will be a breaking change for the |
|
Will the upcoming private fields readily follow such syntax?
Per sec. 4 of the proposal:
|
Do you really think it is necessary to reference hard private Members in documentation? I think code comments as developer documentation should suffice in this case, especially since the referenced target must be declared in the same class in the same source code file and thus should be easy to find. |
|
Is the tsdoc project still being actively worked on? Seems like many items from the tsdoc website/spec reference things like this one that are not complete but have been open for several years now... for this one, five years! Does anyone know the status of the tsdoc project overall?
|
|
@GitMurf The TSDoc project is still actively supported, but for some time there has not been much progress on new features. TSDoc is maintained by Rush Stack, along with API Extractor which relies very heavily on the TSDoc system. Unfortunately we have not be able to prioritize development of TSDoc because the current implementation already meets the needs of API Extractor very well. Every time TSDoc comes up, I want so badly to come work on it, but unfortunately some other project always seems to have higher priority. If someone from the community could get involved and help push TSDoc along, that would be really great. 🙏 TSDoc is a relatively complex project however, so what we need most isn't small PRs, but rather someone with time to learn about the parser and requirements, and maybe help get some of the already existing PRs merged. (If anyone's interested, feel free to contact me.) |
Certain tags such as
{@inheritdoc}contain references to other API items. AEDoc supports two notations for API item references:For internal references, we use:
exportName.memberNameExample:
{@inheritdoc BaseWidget.draw}The "
.memberName" is optional and included e.g. if the API item is a member of a class or interface.For external references, we use:
@scopeName/packageName:exportName.memberNameExample:
{@inheritdoc @microsoft/widget-lib:BaseWidget.draw}The "
@scopeName/" and ".memberName" are optional. The "@scopeName/" is used for scoped NPM packages.This might be a reasonable starting point. However, it doesn't consider nesting members such as namespaces (which has turned out to be surprisingly common for legacy SDKs, despite all the downsides and attempts to deprecate them).
Also, AEDoc today overlooks an interesting edge case:
In this example, an expression such as
{@link A.b}would be ambiguous. Which declaration is it referring to? How to handle this?The text was updated successfully, but these errors were encountered: