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

[api-documenter] Add logic to disambiguate duplicate UID, file, or toc entries #1331

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
400 changes: 361 additions & 39 deletions apps/api-documenter/src/documenters/YamlDocumenter.ts

Large diffs are not rendered by default.

107 changes: 107 additions & 0 deletions build-tests/api-documenter-test/etc/api-documenter-test.api.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,113 @@
"canonicalReference": "",
"name": "",
"members": [
{
"kind": "Class",
"canonicalReference": "(Collision:class)",
"docComment": "/**\n * @public\n */\n",
"excerptTokens": [
{
"kind": "Content",
"text": "export declare class "
},
{
"kind": "Reference",
"text": "Collision"
},
{
"kind": "Content",
"text": " "
}
],
"releaseTag": "Public",
"name": "Collision",
"members": [
{
"kind": "Property",
"canonicalReference": "(a:instance)",
"docComment": "",
"excerptTokens": [
{
"kind": "Reference",
"text": "a"
},
{
"kind": "Content",
"text": ": "
},
{
"kind": "Content",
"text": "number"
},
{
"kind": "Content",
"text": ";"
}
],
"releaseTag": "Public",
"name": "a",
"propertyTypeTokenRange": {
"startIndex": 2,
"endIndex": 3
},
"isStatic": false
}
],
"implementsTokenRanges": []
},
{
"kind": "Interface",
"canonicalReference": "(Collision:interface)",
"docComment": "/**\n * @public\n */\n",
"excerptTokens": [
{
"kind": "Content",
"text": "export interface "
},
{
"kind": "Reference",
"text": "Collision"
},
{
"kind": "Content",
"text": " "
}
],
"releaseTag": "Public",
"name": "Collision",
"members": [
{
"kind": "PropertySignature",
"canonicalReference": "b",
"docComment": "",
"excerptTokens": [
{
"kind": "Reference",
"text": "b"
},
{
"kind": "Content",
"text": ": "
},
{
"kind": "Content",
"text": "number"
},
{
"kind": "Content",
"text": ";"
}
],
"releaseTag": "Public",
"name": "b",
"propertyTypeTokenRange": {
"startIndex": 2,
"endIndex": 3
}
}
],
"extendsTokenRanges": []
},
{
"kind": "Class",
"canonicalReference": "(DocBaseClass:class)",
Expand Down
12 changes: 12 additions & 0 deletions build-tests/api-documenter-test/etc/api-documenter-test.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,18 @@

```ts

// @public (undocumented)
export class Collision {
// (undocumented)
a: number;
}

// @public (undocumented)
export interface Collision {
// (undocumented)
b: number;
}

// @public
export const constVariable: number;

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<!-- Do not edit this file. It is automatically generated by API Documenter. -->

[Home](./index.md) &gt; [api-documenter-test](./api-documenter-test.md) &gt; [Collision](./api-documenter-test.collision.md) &gt; [a](./api-documenter-test.collision.a.md)

## Collision.a property

<b>Signature:</b>

```typescript
a: number;
```
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<!-- Do not edit this file. It is automatically generated by API Documenter. -->

[Home](./index.md) &gt; [api-documenter-test](./api-documenter-test.md) &gt; [Collision](./api-documenter-test.collision.md) &gt; [b](./api-documenter-test.collision.b.md)

## Collision.b property

<b>Signature:</b>

```typescript
b: number;
```
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
<!-- Do not edit this file. It is automatically generated by API Documenter. -->

[Home](./index.md) &gt; [api-documenter-test](./api-documenter-test.md) &gt; [Collision](./api-documenter-test.collision.md)

## Collision interface


<b>Signature:</b>

```typescript
export interface Collision
```

## Properties

| Property | Type | Description |
| --- | --- | --- |
| [b](./api-documenter-test.collision.b.md) | <code>number</code> | |

Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ This project tests various documentation generation scenarios and doc comment sy

| Class | Description |
| --- | --- |
| [Collision](./api-documenter-test.collision.md) | |
| [DocBaseClass](./api-documenter-test.docbaseclass.md) | Example base class |
| [DocClass1](./api-documenter-test.docclass1.md) | This is an example class. |
| [Generic](./api-documenter-test.generic.md) | Generic class. |
Expand All @@ -33,6 +34,7 @@ This project tests various documentation generation scenarios and doc comment sy

| Interface | Description |
| --- | --- |
| [Collision](./api-documenter-test.collision.md) | |
| [IDocInterface1](./api-documenter-test.idocinterface1.md) | |
| [IDocInterface2](./api-documenter-test.idocinterface2.md) | |
| [IDocInterface3](./api-documenter-test.idocinterface3.md) | Some less common TypeScript declaration kinds. |
Expand Down
18 changes: 12 additions & 6 deletions build-tests/api-documenter-test/etc/yaml/api-documenter-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,20 +11,22 @@ items:
- typeScript
type: package
children:
- api-documenter-test.Collision
- 'api-documenter-test.Collision:interface'
- api-documenter-test.DocBaseClass
- api-documenter-test.DocClass1
- api-documenter-test.DocEnum
- api-documenter-test.Generic
- api-documenter-test.globalFunction
- globalFunction(number)
- api-documenter-test.IDocInterface1
- api-documenter-test.IDocInterface2
- api-documenter-test.IDocInterface3
- api-documenter-test.IDocInterface4
- api-documenter-test.IDocInterface5
- api-documenter-test.IDocInterface6
- api-documenter-test.OuterNamespace.InnerNamespace.nestedFunction
- api-documenter-test.OuterNamespace.InnerNamespace.nestedFunction(number)
- api-documenter-test.SystemEvent
- uid: api-documenter-test.globalFunction
- uid: globalFunction(number)
summary: An exported function
name: globalFunction(x)
fullName: globalFunction(x)
Expand All @@ -42,10 +44,10 @@ items:
description: ''
type:
- number
- uid: api-documenter-test.OuterNamespace.InnerNamespace.nestedFunction
- uid: api-documenter-test.OuterNamespace.InnerNamespace.nestedFunction(number)
summary: A function inside a namespace
name: OuterNamespace.InnerNamespace.nestedFunction
fullName: OuterNamespace.InnerNamespace.nestedFunction
name: OuterNamespace.InnerNamespace.nestedFunction(number)
fullName: OuterNamespace.InnerNamespace.nestedFunction(number)
langs:
- typeScript
type: function
Expand All @@ -61,6 +63,10 @@ items:
type:
- number
references:
- uid: api-documenter-test.Collision
name: Collision
- uid: 'api-documenter-test.Collision:interface'
name: Collision
- uid: api-documenter-test.DocBaseClass
name: DocBaseClass
- uid: api-documenter-test.DocClass1
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
### YamlMime:UniversalReference
items:
- uid: api-documenter-test.Collision
name: Collision
fullName: Collision
langs:
- typeScript
type: class
package: api-documenter-test
children:
- api-documenter-test.Collision.a
- uid: api-documenter-test.Collision.a
name: a
fullName: a
langs:
- typeScript
type: property
syntax:
content: 'a: number;'
return:
type:
- number
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
### YamlMime:UniversalReference
items:
- uid: 'api-documenter-test.Collision:interface'
name: Collision
fullName: Collision
langs:
- typeScript
type: interface
package: api-documenter-test
children:
- 'api-documenter-test.Collision:interface.b'
- uid: 'api-documenter-test.Collision:interface.b'
name: b
fullName: b
langs:
- typeScript
type: property
syntax:
content: 'b: number;'
return:
type:
- number
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@ items:
type: class
package: api-documenter-test
children:
- api-documenter-test.DocBaseClass.(constructor)
- api-documenter-test.DocBaseClass.(constructor)_1
- uid: api-documenter-test.DocBaseClass.(constructor)
- api-documenter-test.DocBaseClass.constructor()
- api-documenter-test.DocBaseClass.constructor(number)
- uid: api-documenter-test.DocBaseClass.constructor()
summary: The simple constructor for `DocBaseClass`
name: (constructor)()
fullName: (constructor)()
Expand All @@ -20,7 +20,7 @@ items:
type: constructor
syntax:
content: constructor();
- uid: api-documenter-test.DocBaseClass.(constructor)_1
- uid: api-documenter-test.DocBaseClass.constructor(number)
summary: The overloaded constructor for `DocBaseClass`
name: (constructor)(x)
fullName: (constructor)(x)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,16 +20,16 @@ items:
- api-documenter-test.IDocInterface2
package: api-documenter-test
children:
- api-documenter-test.DocClass1.deprecatedExample
- api-documenter-test.DocClass1.exampleFunction
- api-documenter-test.DocClass1.exampleFunction_1
- api-documenter-test.DocClass1.interestingEdgeCases
- api-documenter-test.DocClass1.deprecatedExample()
- 'api-documenter-test.DocClass1.exampleFunction(string,string)'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rbuckton What would you do with this input? 🙃

export declare function pathologicalExample(options: {
  args: {
    [name: string]: string | boolean;
  };
  buildErrorIconPath?: string;
  buildSuccessIconPath?: string;
  distFolder: string;
  gulp: GulpProxy | Gulp;
  isRedundantBuild?: boolean;
  jestEnabled?: boolean;
  libAMDFolder?: string;
  libES6Folder?: string;
  libESNextFolder?: string;
  libFolder: string;
  onTaskEnd?: (taskName: string, duration: number[], error?: any) => void;
  onTaskStart?: (taskName: string) => void;
  packageFolder: string;
  production: boolean;
  properties?: {
    [key: string]: any;
  };
  relogIssues?: boolean;
  rootPath: string;
  shouldWarningsFailBuild: boolean;
  showToast?: boolean;
  srcFolder: string;
  tempFolder: string;
  uniqueTasks?: IExecutable[];
  verbose: boolean;
} & Partial<ICopyConfig>): void;

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

items:
- uid: package.pathologicalExample
  name: package.pathologicalExample
  type: function
  syntax:
    parameters:
    - id: options
      type:
      - anonymous0:local
    return:
      type:
      - void
references:
- uid: anonymous0:local
  name: |> <content of options type>
  spec.typeScript:
  - name: '{\n  args: {\n    [name: string]: string | boolean;\n  }; <snip> gulp: '
  - name: GulpProxy
    uid: otherpackage.GulpProxy
  - name: ' | '
  - name: Gulp
    uid: gulp.Gulp
  - name: ';\n  isRedundantBuild?: <snip> uniqueTasks?: '
  - name: IExecutable
    uid: otherpackage.IExecutable
  - name: '[];\n  verbose: boolean;\n} & '
  - name: Partial
    uid: Partial
  - name: '<'
  - name: ICopyConfig
    uid: otherpackage.ICopyConfig
  - name: '>'
- uid: otherpackage.GulpProxy
  name: GulpProxy
- uid: gulp.Gulp
  name: Gulp
- uid: otherpackage.IExecutable
  name: IExecutable
- uid: otherpackage.ICopyConfig
  name: ICopyConfig

You also could have a uid of {{args:{{[name:string]~string|boolean}},buildErrorIconPath?:string,buildSuccessIconPath?:string,...}&Partial{ICopyConfig} (with the ... replaced) since it doesn't need to be human readable, just machine readable.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The entry in references is effectively the same as an Excerpt, with some additional information (uid, nameWithType, fullName).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it doesn't need to be human readable, just machine readable.

@rbuckton From a technical perspective, this may be true. But I'm pretty uncomfortable about designing a system that emits huge unwieldy expressions in a file that people may have to understand/debug.

Also, what about the other related ID problems called out in #1308? We wouldn't want these huge expressions to appear in:

  • Markdown URLs
  • DocFX URLs
  • Titles that appear in member tables in the docs

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Complex types should probably have aliased UIDs (possibly based on a hash of the stable UID). They would point to an entry in the yaml references list which would have the expanded text/excerpt that breaks down the type into hyperlinkable tokens.

- api-documenter-test.DocClass1.exampleFunction(number)
- api-documenter-test.DocClass1.interestingEdgeCases()
- api-documenter-test.DocClass1.malformedEvent
- api-documenter-test.DocClass1.modifiedEvent
- api-documenter-test.DocClass1.regularProperty
- api-documenter-test.DocClass1.sumWithExample
- api-documenter-test.DocClass1.tableExample
- uid: api-documenter-test.DocClass1.deprecatedExample
- 'api-documenter-test.DocClass1.sumWithExample(number,number)'
- api-documenter-test.DocClass1.tableExample()
- uid: api-documenter-test.DocClass1.deprecatedExample()
deprecated:
content: Use `otherThing()` instead.
name: deprecatedExample()
Expand All @@ -43,7 +43,7 @@ items:
type:
- void
description: ''
- uid: api-documenter-test.DocClass1.exampleFunction
- uid: 'api-documenter-test.DocClass1.exampleFunction(string,string)'
summary: This is an overloaded function.
name: 'exampleFunction(a, b)'
fullName: 'exampleFunction(a, b)'
Expand All @@ -65,7 +65,7 @@ items:
description: the second string
type:
- string
- uid: api-documenter-test.DocClass1.exampleFunction_1
- uid: api-documenter-test.DocClass1.exampleFunction(number)
summary: This is also an overloaded function.
name: exampleFunction(x)
fullName: exampleFunction(x)
Expand All @@ -83,7 +83,7 @@ items:
description: the number
type:
- number
- uid: api-documenter-test.DocClass1.interestingEdgeCases
- uid: api-documenter-test.DocClass1.interestingEdgeCases()
summary: |-
Example: "<!-- -->{ \\<!-- -->"maxItemsToShow<!-- -->\\<!-- -->": 123 }<!-- -->"

Expand Down Expand Up @@ -135,7 +135,7 @@ items:
return:
type:
- api-documenter-test.SystemEvent
- uid: api-documenter-test.DocClass1.sumWithExample
- uid: 'api-documenter-test.DocClass1.sumWithExample(number,number)'
summary: Returns the sum of two numbers.
remarks: This illustrates usage of the `@example` block tag.
name: 'sumWithExample(x, y)'
Expand All @@ -158,7 +158,7 @@ items:
description: the second number to add
type:
- number
- uid: api-documenter-test.DocClass1.tableExample
- uid: api-documenter-test.DocClass1.tableExample()
summary: 'An example with tables:'
remarks: <table> <tr> <td>John</td> <td>Doe</td> </tr> </table>
name: tableExample()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ items:
- api-documenter-test.IDocInterface1
package: api-documenter-test
children:
- api-documenter-test.IDocInterface2.deprecatedExample
- uid: api-documenter-test.IDocInterface2.deprecatedExample
- api-documenter-test.IDocInterface2.deprecatedExample()
- uid: api-documenter-test.IDocInterface2.deprecatedExample()
deprecated:
content: Use `otherThing()` instead.
name: deprecatedExample()
Expand Down
Loading