-
Notifications
You must be signed in to change notification settings - Fork 596
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
[api-extractor] Optionally include source code coordinates in generated documentation #895
Comments
Do you have a sense of how hard this would be to implement? In a new project I'm working on, we'd like to have links to source files (don't care about specific line numbers) in the documentation. So if it's not too difficult I'd be willing to help add this feature. |
@ecraig12345 That would be really great! This is definitely an important feature. Lemme think about the design and follow up... |
There are some interesting design considerations for this feature. Consider this declaration: node-core-library/src/JsonFile.ts /**
* Options for JsonFile.saveJsonFile()
*
* @public
*/
export interface IJsonFileSaveOptions extends IJsonFileStringifyOptions {
/**
* Creates the folder recursively using FileSystem.ensureFolder()
* Defaults to false.
*/
ensureFolderExists?: boolean;
. . .
} 1. Location unitsIn the model's tree, the two declaration nodes are
Note that these coordinates are interchangeable ONLY if you have access to the original source file. Otherwise we don't know what line number corresponds to 2. What to linkThere's also a question of what the range should correspond to. Do we link to just the word (BTW I'm not proposing to implement all this right away. But it can sometimes be helpful to speculate about an "ideal" future design.) 3. Loose linkingNote that API Extractor normally documents exported APIs, which sometimes are different from the original declaration. For example if you do this: src/index.ts export { IJsonFileSaveOptions as _IJsonFileSaveOptions } from './src/JsonFile'; ...then the API will be called Also recall that API Extractor processes .d.ts files, not .ts files. We rely on .d.ts.map files to locate the original source coordinates. These maps are not super accurate. In some cases we may not be able to produce a link at all. 4. Multiple source filesToday each API item is declared in a single source file. But this is likely to change in the future. Certain kinds of TypeScript declaration merging may be treated as a single .api.json declaration. Also microsoft/tsdoc#137 proposes a TSDoc tag 5. JSON VerbositySource file paths tend to be big, however. If we repeatedly write But we also want stable diffs when APIs are updated. For example, if replace the strings with indexes into a central array of source file paths, the indexes might get shuffled whenever a new API is added. 6. Relative pathsSource files will be relative to a project folder. The api-extractor.json config file defines a Also, although it's a strongly discouraged practice, TypeScript allows people to write 7. Repo URLsPresumably the main usefulness of these links would be to generate GitHub URLs to the original file. If we're storing relative paths, maybe the |
Here's the JSON for the node-core-library.api.json {
"kind": "Interface",
"canonicalReference": "@microsoft/node-core-library!IJsonFileSaveOptions:interface",
"docComment": "/**\n * Options for JsonFile.saveJsonFile()\n *\n * @public\n */\n",
"excerptTokens": [
{
"kind": "Content",
"text": "export interface IJsonFileSaveOptions extends "
},
{
"kind": "Reference",
"text": "IJsonFileStringifyOptions",
"canonicalReference": "@microsoft/node-core-library!IJsonFileStringifyOptions:interface"
},
{
"kind": "Content",
"text": " "
}
],
"releaseTag": "Public",
"name": "IJsonFileSaveOptions",
"members": [
{
"kind": "PropertySignature",
"canonicalReference": "@microsoft/node-core-library!IJsonFileSaveOptions#ensureFolderExists:member",
"docComment": "/**\n * Creates the folder recursively using FileSystem.ensureFolder() Defaults to false.\n */\n",
"excerptTokens": [
{
"kind": "Content",
"text": "ensureFolderExists?: "
},
{
"kind": "Content",
"text": "boolean"
},
{
"kind": "Content",
"text": ";"
}
],
"releaseTag": "Public",
"name": "ensureFolderExists",
"propertyTypeTokenRange": {
"startIndex": 1,
"endIndex": 2
}
},
. . .
],
"extendsTokenRanges": [
{
"startIndex": 1,
"endIndex": 3
}
]
}, What if we changed the first "excerptTokens": [
{
"kind": "Content",
"text": "export interface "
},
{ // <== split into a separate token
"kind": "Content",
"text": "IJsonFileSaveOptions",
"sourceFile": "src/JsonFile.ts", // <== new
"sourceStart": [ 46, 18 ], // <== new
"sourceEnd": [ 46, 46 ] // <== new
},
{
"kind": "Content",
"text": " extends "
},
{
"kind": "Reference",
"text": "IJsonFileStringifyOptions",
"canonicalReference": "@microsoft/node-core-library!IJsonFileStringifyOptions:interface"
},
{
"kind": "Content",
"text": " "
}
], The full-blown design would be like this:
Then to address the humble scenario of "Give me a GItHub URL for this thing", the For named things like For unnamed things like an indexer ( |
@ecraig12345 Maybe we can approach it like this: I can start a PR that introduces the new APIs (a minimal subset of the "ideal" proposal above). My PR can also illustrate how to calculate coordinates for a few kinds like (BTW I believe DocFX also supports source-linking. If you're ambitious, maybe later you could come back with a separate PR that updates API Documenter to write these coordinates into the DocFX YAML files. A lot of people would be happy about that.) |
@rbuckton (who might have some thoughts about these requirements) |
Thanks for thinking this through! Some thoughts in response: 1. Location unitsThe location/range within the file actually isn't important for my purposes and seems like it would complicate the implementation (especially if you try to go through sourcemaps and link to the original source file rather than the .d.ts), so I wonder if it would be possible to support only paths in the initial implementation and leave within-file ranges for later? 2. What range to linkIf there's a way through api-extractor to get the doc comment for an item once you have its name, I think linking to just the name itself might make sense. (I don't have a strong opinion on this though since it's not something we're as likely to use.) 3. Loose linkingThis is a good point. To clarify, do you mean we should link to the export line, or the original interface definition (even if the name is different)? A related tricky case is whole-package re-exports, 6. Relative pathsI think using paths relative to either 7. Repo linksI would be okay with leaving repo link generation to the consumer, at least at first, due to various complicating factors like converting from generated file paths to original source paths (if declaration maps aren't configured), getting the repo URL, and determining the correct branch. Where to store paths in the JSONI can see how the proposal of storing the location in I'd suggest something more like this instead (or possibly bundling the info together in an object under a
|
@octogonz Have you gotten a chance to think about this any more? It's pretty important for the project I'm currently working on. I can probably help with the implementation, but I'll need some examples first (still don't have a good enough mental model of how the TS compiler API works to start this one by myself). |
I haven't been able to look at this yet. I'm out of office right now but will be back after Thanksgiving. |
I thought about it some more. Seems like I was over-analyzing this problem. You're right that it's probably not super useful to maintain source code coordinates for each excerpt token. It should be enough to track the original source file/line for each API item. @ecraig12345 If you want to get started on this before I'm back, you can obtain the coordinates for an AstDeclaration like this: const astDeclaration: AstDeclaration;
const sourceFile: ts.SourceFile = astDeclaration.declaration.getSourceFile();
const sourceFilePath: string = sourceFile.fileName;
const lineAndCharacter: ts.LineAndCharacter = sourceFile.getLineAndCharacterOfPosition(
astDeclaration.declaration.pos); The above code will return the location in the .d.ts file. To translate that to a .ts file path, you'll need to use the source map logic found in SourceMapper.ts. That's not a whole lot to go on, but maybe you can figure it out. Otherwise I'll put together soon as I get some time. Thanks for your patience! |
Bumping this for attention (sorry for the noise). Getting the file/line combination would be super useful. E.g. I'm currently making a Github action that warns about changes to the API on a PR. If I had access to the path/line, those warnings could show up directly in the diff view! |
FYI: So at the very least, API Extractor might want to keep track of that minimal set of information to be output using |
Regarding the DocFX coordinates: In the interests of conciseness, we could split this information into two parts: Probably the same for all API items within a given NPM package
👆 These can probably be stored centrally with the Potentially different for each API item
👆 Whereas these could be stored separately for each item. However: the CC @zelliott |
For open source projects, the documentation web site may need to point people to the source code where an API is declared. API Extractor should track the original source file location in the .api.json files, and optionally render this as part of the API reference.
The text was updated successfully, but these errors were encountered: