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] Hyperlinking support for "complex" types (YAML generator) #867

Closed
AlexJerabek opened this issue Oct 8, 2018 · 21 comments
Closed

Comments

@AlexJerabek
Copy link
Contributor

Currently, the tool uses the uid of a type to help create link to the type's page when API Documenter's yml is run through the OPS build system (for docs.microsoft.com). This works for simple types (like excel.Excel.NamedItemCollection is this example):

  - uid: excel.Excel.Worksheet.names
    summary: |-
      Collection of names scoped to the current worksheet. Read-only.

      \[ [API set: ExcelApi 1.4](/javascript/office/requirement-sets/excel-api-requirement-sets) \]
    name: names
    fullName: excel.Excel.Worksheet.names
    langs:
      - typeScript
    type: property
    syntax:
      content: 'readonly names: Excel.NamedItemCollection;'
      return:
        type:
          - excel.Excel.NamedItemCollection

However, if the type is in a generic or unioned with another type, the uid is not inserted (like Excel.WorksheetCalculatedEventArgs in this example):

  - uid: excel.Excel.Worksheet.onCalculated
    summary: |-
      Occurs when the worksheet is calculated.

      \[ [API set: ExcelApi 1.8](/javascript/office/requirement-sets/excel-api-requirement-sets) \]
    name: onCalculated
    fullName: excel.Excel.Worksheet.onCalculated
    langs:
      - typeScript
    type: event
    syntax:
      content: 'readonly onCalculated: OfficeExtension.EventHandlers<Excel.WorksheetCalculatedEventArgs>;'
      return:
        type:
          - OfficeExtension.EventHandlers<Excel.WorksheetCalculatedEventArgs>

We want Excel.WorksheetCalculatedEventArgs to link to it's page, despite being syntactically part of Office.Extension.EventHandlers.

@octogonz
Copy link
Collaborator

octogonz commented Oct 8, 2018

@AlexJerabek IIRC this syntax will not work today, because the YAML schema does not support it:

      return:
        type:
          - OfficeExtension.EventHandlers<Excel.WorksheetCalculatedEventArgs>

Our feature request was for the docs team to improve the schema to allow arbitrary hyperlinks in that field, like this:

      return:
        type:
          - [OfficeExtension.EventHandlers](xref:excel.OfficeExtension.EventHandlers)<[Excel.WorksheetCalculatedEventArgs](xref:excel.Excel.WorksheetCalculatedEventArgs)>

Last I checked, this was not supported, in which case support by api-documenter is blocked until that's resolved. (Let me know if that's not the case.)

@dend FYI

@AlexJerabek
Copy link
Contributor Author

@pgonzal I received some updates for APEX on the subject. I'll copy their answers based on the example I provided earlier in the thread here:

(Den): For return types, we need to make sure that’s not free form – we should not be putting Markdown there. Instead, we should have a way to separate types properly to make sure that we generate links between angle brackets. Chan – does TypeDoc or JSDoc give us any information on individual types, when complex types are involved for the return field?

(Chan): Currently we have parsed this complex return type and resolved the reference of return type but might not work for all possible cases.

@octogonz
Copy link
Collaborator

@AlexJerabek let's take this discussion offline. I don't believe today's YAML file has the capability to model what he's describing.

@AlexJerabek AlexJerabek changed the title [API-Documenter] - Crosslink support for "complex" types [API-Documenter] (BLOCKING) - Crosslink support for "complex" types Jan 29, 2019
@iclanton iclanton added the blocked We can't proceed because we're waiting for something label Jan 29, 2019
@iclanton iclanton changed the title [API-Documenter] (BLOCKING) - Crosslink support for "complex" types [API-Documenter] - Crosslink support for "complex" types Jan 29, 2019
@iclanton iclanton added blocking and removed blocked We can't proceed because we're waiting for something labels Jan 29, 2019
@octogonz octogonz changed the title [API-Documenter] - Crosslink support for "complex" types [api-documenter] Hyperlinking support for "complex" types Feb 6, 2019
@octogonz octogonz changed the title [api-documenter] Hyperlinking support for "complex" types [api-documenter] Hyperlinking support for "complex" types (YAML generator) Feb 6, 2019
@sumurthy
Copy link

sumurthy commented Apr 8, 2019

Is there any update to this issue? The current experience is broken and it is impossible to find the Event API interface needed to carry out the calls for JavaScript developers. People can stumble their way through on TypeScript based on d.ts; but JavaScript only developers are blocked from getting required level of navigation across various types.

Could this please be prioritized?

@octogonz
Copy link
Collaborator

octogonz commented Apr 8, 2019

Someone from the community started PR #1005 which would provide the linking information on API Extractor's side.

However if the DocFX the YAML file format cannot represent these links, that would need to be fixed by the DocFX owners as a prerequisite. I seem to remember them saying that they had added a way to do this -- I'll dig through my old email and see if I can find it.

@sumurthy since I no longer have direct access to DocFX, one thing that would help a lot is if the GitHub issues included example YAML snippets showing the expected output (which have been tested and confirmed to work). If Office could provide that, it would help get these issues fixed faster.

FYI these requests are also basically blocked because it's unclear what the YAML should look like:

@AlexJerabek
Copy link
Contributor Author

@octogonz FYI, I've reached out on a couple internal docs.microsoft.com channels to get the schema you need.

@octogonz
Copy link
Collaborator

@AlexJerabek By the way AE 7.0.41 is a release candidate that hopefully will finally get shipped in the next few days. If you have time, now's a good point to try it out and let us know if you encounter any issues.

@AlexJerabek
Copy link
Contributor Author

@octogonz Just ran it an received some errors:

api-extractor 7.0.41 - https://api-extractor.com/

Using configuration from C:\GitHub\office-js-docs-reference\generate-docs\api-extractor-inputs-office\api-extractor.json

Error: JSON validation failed:
C:\GitHub\office-js-docs-reference\generate-docs\api-extractor-inputs-office\api-extractor.json
Error: #/ (Describes how the API Extractor tool ...)
       Additional properties not allowed: apiJsonFile,apiReviewFile,project,validationRules,policies
Error: #/ (Describes how the API Extractor tool ...)
       Missing required property: mainEntryPointFilePath
Error: #/dtsRollup (Configures how the .d.ts rollup file ...)
       Missing required property: enabled
Error: #/docModel (Configures how the doc model file (*....)
       Missing required property: enabled
Error: #/apiReport (Configures how the API report file (*...)
       Missing required property: enabled
Error: #/compiler (Determines how the TypeScript compile...)
       Additional properties not allowed: rootFolder,configType

Here's our api-extractor.json file. Is there documentation on how to update it?

{
  "$schema": "https://raw.githubusercontent.com/OfficeDev/office-js-docs-reference/master/generate-docs/schemas/api-extractor.schema.json",
  "compiler" : {
    "configType": "tsconfig",
    "rootFolder": "."
  },
  "policies": {
    "namespaceSupport": "permissive"
  },
  "validationRules": {
    "missingReleaseTags": "allow"
  },
  "project": {
    "entryPointSourceFile": "office.d.ts"
  },
  "apiReviewFile": {
    "enabled": false
  },
  "apiJsonFile": {
    "enabled": true,
    "outputFolder": "../json"
  }
}

@octogonz
Copy link
Collaborator

Yes, the config file schema has changed. Although some field names have changed, and the file paths are resolved slightly differently, the overall semantics are pretty much the same as before.

If you run api-extractor init in an empty folder, it will dump out this template file which contains complete documentation for all the fields.

The new $schema is here:

https://developer.microsoft.com/json-schemas/api-extractor/v7/api-extractor.schema.json

@octogonz
Copy link
Collaborator

@AlexJerabek Here's a rough sketch of what your upgraded file should look like (didn't have time to test it):

{
  "$schema": "https://developer.microsoft.com/json-schemas/api-extractor/v7/api-extractor.schema.json",
  "mainEntryPointFilePath": "office.d.ts",
  "apiReviewFile": {
    "enabled": false
  },
  "docModel": {
    "enabled": true,
    "apiJsonFilePath": "../json/<unscopedPackageName>.api.json"
  },
  "dtsRollup": {
    "enabled": false
  },
  "messages": {
    "extractorMessageReporting": {
      "ae-missing-release-tag": {
        "logLevel": "none"
      }
    },
  }
}

The "policies" section isn't needed any more.

@AlexJerabek
Copy link
Contributor Author

Thanks @octogonz. The updated json file worked (with the minor tweak of switching apiReviewFile to apiReport).

I reran our doc generation tooling chain with API Extractor 7.0.41. There do not appear to be any regressions.

Hopefully, we can get the OPS yaml schema soon, so this issue can be fixed :-)

@octogonz
Copy link
Collaborator

Awesome, thanks!

@AlexJerabek
Copy link
Contributor Author

AlexJerabek commented Apr 16, 2019

@octogonz Here's the response I got from Docs Support:

Since JavaScript/TypeScript is using UniversalReference format, there is no JSON file defining the YAML schema. The schema is probably defined in a C# file https://github.com/dotnet/docfx/blob/master/src/Microsoft.DocAsCode.Metadata.ManagedReference.Common/Models/ReferenceItem.cs 

Not sure if that .cs file unblocks you in any way.

@AlexJerabek
Copy link
Contributor Author

And here's their page on cross referencing:
image

@AlexJerabek
Copy link
Contributor Author

@octogonz,
OPS has enabled some cross-link scenarios. The following screenshot shows a UID in the yml properly rendering in the docs, despite being surrounded by angle brackets.

GenericType

However, it appears links within unioned types are still not supported,

UnionedType

That said, I think it would be beneficial to go forward as if it all works as intended and use UIDs in all the "type" yml sections. Those links will aid significant portions of our documentation. Unlinked UIDs don't look terrible and will eventually be supported.

Thoughts?

@rbuckton
Copy link
Member

rbuckton commented Jun 5, 2019

It looks like what is needed is for the YamlDocumenter to generate synthetic references using the following policies:

I've tested and verified that this works for UniversalReference documents. Basically what is needed is that when a complex type (such as a generic type, function type, union type, etc.) is to be written to the YAML output it should be converted into a Spec UID. If the Spec UID is not a normal UID reference, then a synthetic reference should be added to the document that maps the Spec UID to the individual UIDs.

For example, given the following TypeScript in package baz:

import { MyClass } from "foo";
import { A, B } from "bar";
export declare function f(): MyClass<A, B>;

We would generate the YAML below:

### YamlMime:UniversalReference
items:
  ...
- uid: baz.f
  name: f()
  fullName: f()
  langs:
  - typeScript
  type: function
  syntax:
    content: 'export declare function f(): MyClass<A, B>;'
    return:
      type:
        - foo.MyClass{bar.A,bar.B}
references:
- uid: foo.MyClass{bar.A,bar.B}
  name.typeScript: MyClass<A, B>
  fullName.typeScript: MyClass<A, B>
  spec.typeScript:
  - uid: foo.MyClass
    name: MyClass
    fullName: MyClass
  - name: <
    fullName: <
  - uid: bar.A
    name: A
    fullName: A
  - name: ', '
    fullName: ', '
  - uid: bar.B
    name: B
    fullName: B
  - name: '>'
    fullName: '>'    

In essence, the synthetic reference for the type aligns with the excerpt tokens that make up the type.

This results in docfx generating the following output as part of its viewmodel:

<xref uid="foo.MyClass" displayProperty="name" altProperty="fullName" />&lt;↵
<xref uid="bar.A" displayProperty="name" altProperty="fullName" />, ↵
<xref uid="bar.B" displayProperty="name" altProperty="fullName" />&gt;

(note: line continuation arrows (↵) added for clarity)

I tested this out on a project of mine. You can see an example of how I'm generating the names here: https://github.com/esfx/esfx/blob/master/scripts/docs/yamlDocumenter.js#L650.

You can see the outcome of this here: https://esfx.js.org/esfx/api/collections-linkedlist/linkedlist.html#collections_linkedlist_LinkedList_deleteNode

@rbuckton
Copy link
Member

rbuckton commented Jun 5, 2019

Note, this also required making changes to typescript.schema.json as well to allow for the reference spec entries.

@AlexJerabek
Copy link
Contributor Author

@rbuckton,
Any updates on getting your proposed changes into the YamlDocumenter? I'm happy to test any tooling changes on our doc set.

@octogonz
Copy link
Collaborator

@AlexJerabek the current progress is in PR #1337. I will review it this weekend.

BTW we've also had a bunch of conversations over the past few weeks about the issues called out in #1308. Ron is proposing some significant improvements to the TSDoc ref notation (and corresponding DocFX UID representations).

@octogonz
Copy link
Collaborator

octogonz commented Aug 8, 2019

Status update:

@AlexJerabek
Copy link
Contributor Author

PR #1337 fixed this issue, so I'll close it. Thank you for everything that went into the fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants