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 Extractor: Add support for custom TSDoc tags #1628

Closed
wants to merge 1 commit into from

Conversation

ecraig12345
Copy link
Member

@ecraig12345 ecraig12345 commented Nov 13, 2019

Add support for custom TSDoc tags in the API Extractor config file and when deserializing via ApiModel. Fixes #519.

Implementing this involved a couple significant design decisions which I've called out below. I'm open to suggestions on whether these (and really any element of the change) should be handled differently.

Configuration placement and naming

Right now, the new section in the JSON is at the top level and would look like this:

{
  "tsdoc": {
    "tagDefinitions": [
      {
        "tagName": "@foo",
        "syntaxKind": "block",
        "allowMultiple": true
      },
      {
        "tagName": "@bar",
        "syntaxKind": "inline"
      }
    ]
  }
}

The placement (top-level or not) and naming are negotiable. The idea of putting tagDefinitions under a parent tsdoc tag was to facilitate possibly adding more TSDoc-related options later. The tag definition items are based on ITSDocTagDefinitionParameters.

How to update and share the TSDoc configuration

Another significant design decision is whether the new tags should be added to the global AedocDefinitions.tsdocConfiguration, or whether a new TSDoc configuration should be generated when adding tags. The advantage of updating the global using AedocDefinitions.tsdocConfiguration.addTagDefinitions(...) is simplicity. The disadvantage is that it's polluting a global object, which makes behavior harder to trace and could cause problems if anyone wants to use different configurations within the same program.

To avoid polluting a global, I went with creating a new TSDoc configuration which incorporates existing defaults, and passing it as a parameter to the various places that need it. Currently the configuration for API Extractor is created in ExtractorConfig, and the one for deserialization is created in ApiModel (though I wonder if that ought to be done in ApiPackage instead).

@octogonz
Copy link
Collaborator

I wonder if there's a way to make this configuration discoverable by other tools... For example @bafolts recently created a package eslint-plugin-tsdoc that enabels ESLint to validate TSDoc comments. Since it also invokes the TSDoc parser, it also needs to know about custom tags.

We could write this config into the tsdoc-metadata.json file, but that's an output, so it wouldn't be available until after you've built the project.

We could invent a special tsdoc-config.json input file.

We could also define the custom tags in the @packageDocumentation code comment.

I don't know the right answer. Your approach here is pretty good, but it might be good to brainstorm some other options.

@octogonz
Copy link
Collaborator

octogonz commented Nov 14, 2019

We could write this config into the tsdoc-metadata.json file, but that's an output, so it wouldn't be available until after you've built the project.

Thinking about this some more, I feel like we made a mistake by designing tsdoc-metadata.json as a machine-generated file instead of as a human-authored config file. And instead of storing tool versions, that file should store a TSDocConfiguration just like your proposal above.

@ecraig12345 if you don't mind waiting, I'd like to pursue moving your configuration down into the @microsoft/tsdoc library itself. This is a more significant change, but if we're going to invest the effort in this, the ESLint rules should be able to read it. I'm willing to make the PR for that, since this will fix an annoyance for the new ESLint rule.

Rather than breaking the tsdocMetadataFilePath feature of API Extractor, maybe we can use a different filename tsdoc-config.json for the new thing. Then we can deprecate tsdoc-metadata.json and remove it in the next major version. That way this can still be a minor change for API Extractor.

@ecraig12345
Copy link
Member Author

That sounds like the right thing to do long-term. From working on this, I actually figured out a (rather hackish) workaround for adding extra tags when invoking API Extractor programmatically: directly calling AedocDefinitions.tsdocConfiguration.addTagDefinitions(...) from my project's code before invoking API Extractor or creating an API model. While I know this is unsupported (since AedocDefinitions is an internal API) and therefore may break in the future, it will unblock the scenario I was trying to address until proper custom tag configuration is implemented.

@octogonz
Copy link
Collaborator

Cool. I posted my proposed design here and will start work on a PR as soon as I get some time.

octogonz added a commit to microsoft/tsdoc that referenced this pull request Nov 15, 2019
@octogonz
Copy link
Collaborator

This PR implements an initial sketch of the tsdoc-config.json feature: microsoft/tsdoc#196

@octogonz
Copy link
Collaborator

We published @microsoft/tsdoc-config, and there's a sample usage of the API in the TSDoc ESLint plugin project.

The basic idea is that the custom tag definitions go in a new config file tsdoc.json instead of in api-extractor.json. The JSON schema for the config file is based on your PR #1628.

With this design, I feel like we can also move the AEDoc-specific definitions out of [AedocDefinitions.ts](https://github.com/microsoft/rushstack/blob/master/apps/api-extractor-
model/src/aedoc/AedocDefinitions.ts) and into a shared config something like this:

my-project/tsdoc.json

{
  "$schema": "https://developer.microsoft.com/json-schemas/tsdoc/v0/tsdoc.schema.json",
  "extends": [ "@microsoft/api-extractor/includes/tsdoc-aedoc.json" ]
}

@ecraig12345 It sounds like you already found a workaround to unblock yourself, but if you want to keep going, let us know. Otherwise maybe I can find some time to push this feature along.

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

Successfully merging this pull request may close these issues.

[api-extractor] Support extensible AEDoc tags
2 participants