RFC: Detecting whether a dependency implements TSDoc

[octogonz]

Consider this example from API Extractor:

/** @beta */
export interface IPrepareOptions {
  target: string;
}

/** @public */
export interface IWidget {
  /** Renders the widget */
  render(): void;
}

/** @public */
export class Widget implements IWidget {
  /** {@inheritdoc IWidget.render} */  // <-- inherited doc comment
  render(): void;

  prepare(options: IPrepareOptions): void;  // <-- inconsistent visibility error
}

This illustrates a couple kinds of cross references:

1. Documentation inheritance: The Widget.render docs are copied from IWidget.render using the {@inheritdoc} tag. This avoids having to write duplicate documentation.
2. Visibility checking: API Extractor allows API items to be marked with a "release tag" such as @beta. For a public release, only the @public definitions will be included in the generated *.d.ts rollup file; the other definitions are trimmed out. In the above example, the analyzer must report an error because if IPrepareOptions gets trimmed, then Widget.prepare won't be able to consumer that interface.

Now, suppose that IWidget and IPrepareOptions are imported from a separate NPM package called widget-lib. This is no problem -- API Extractor can simply parse the TSDoc comments from this file:

./node_modules/widget-lib/lib/index.d.ts

This seems fine on the surface, but the node_modules folder is full of *.d.ts files that have no awareness of API Extractor. Their doc comments might randomly contain a string such as @beta, or they might simply have malformed comments that would normally be reported as a TSDoc error.

We need a way to determine whether a given NPM package actually implements TSDoc or not.

Our proposal is to add a custom field tsdoc to the package.json file format like this:

{
  "name": "widget-lib",
  "version": "1.0.0",
  "main": "lib/index.d.ts",
  "typings": "lib/index.js",
  "tsdoc": {
    "tsdocFlavor": "AEDoc"
  }
}

(Note that the CommonJS spec specifically allows for custom fields.)

Initially, the tsdoc block would support a single field tsdocFlavor that helps tooling to recognize custom extensions to the core TSDoc tags. In the future it might include other fields such as a version number. See this file for a real world prototype of this convention.

Does this seem like a good design?

[octogonz]

If a given file (e.g. in a local ./typings folder) wants to opt-out of TSDoc or indicate a different flavor, perhaps it could do this using a // directive in the source code (similar to how tslint annotations work). The package.json field would merely set the default for the package folder.

[aciccarello]

I'd like to explore possibilities for resilient handling of type definitions from node_modules by default but I see the benefits to explicit flavor declarations. Maybe there could be a minimal, default flavor that minimizes possible parsing confusion.

Is there any reason (other than for tsdoc config) that the package.json would already be read? I'm wondering if it would be possible to simplify things by sticking to // declarations and possibly having them autogenerated in the declaration files.

[yume-chan]

I'd expect tsdoc have only versions and extensions, not "flavours". If it does, will parsers and code editors need to support multiple flavours? And what if I want to write docs supporting multiple flavours? Tsdoc is said to become a standard for doc comments, so parsers and extensions must not have basic incompatibilities, and extensions must find ways to ensure no ambiguities.

[DovydasNavickas]

I guess flavor would serve only informational purpose, like metadata. TSDoc would parse documentation comments and generate AST representing them and no matter what flavor would be defined in metadata, AST would not be affected.

Never the less, giving the tools some kind of metadata about your docs might be beneficial, although talking about flavors, I'd strongly advocate standardizing the documentation to not need flavors at all.

[octogonz]

Is there any reason (other than for tsdoc config) that the package.json would already be read?

API Extractor already relies on the package.json file to map files to their associated package. For example, suppose we follow a type alias that points to this file:

/home/pgonzal/my-project/node_modules/library1/_node_modules/library2/lib/Widget.d.ts:

The algorithm will walk upwards from this path until it finds the nearest package.json file here:

/home/pgonzal/my-project/node_modules/library1/_node_modules/library2/package.json

...and then it will understand that the Widget type is part of library2. However, it will not attempt to parse the doc comments to look for @public or @internal tags unless it sees a tsdoc marker in the package.json file. This is important because the *.d.ts rollup trims its output according to these tags. They are not just for documentation.

I'd expect tsdoc have only versions and extensions, not "flavours". If it does, will parsers and code editors need to support multiple flavours? And what if I want to write docs supporting multiple flavours? Tsdoc is said to become a standard for doc comments, so parsers and extensions must not have basic incompatibilities, and extensions must find ways to ensure no ambiguities.

This is a pretty good point actually. If two different tools introduce custom TSDoc tags, then should be possible for both tools to be used on the same source files. How about something like this?

  "tsdoc": {
    "version": "1.1",  // <-- we don't need this right away, but might need to introduce it later
    "extensions": [ "aedoc", "typedoc" ]  // <-- sets of custom tags that are in use in these files
  }

Never the less, giving the tools some kind of metadata about your docs might be beneficial, although talking about flavors, I'd strongly advocate standardizing the documentation to not need flavors at all.

One of the goals of TSDoc is that two different tools can add custom tags while sharing a common grammar. However, it's still possible that two different tools might introduce a nonstandard tag called @internal and disagree about its meaning. We could ask that all custom tags should be centrally registered with the TSDoc project and coordinated to avoid these conflicts, but that might be overly bureaucratic. Registering your extension name would be less cumbersome. Another option would be to simply say that the extension name is an NPM package name, which would ensure uniqueness.

[tenry92]

An alternative to the tsdoc field in the package.json would be a separate tsdoc.json file in the project root, similar like ESDoc allows it.

The configuration should indicate whether lax or strict mode is being used. In addition to this, additional fields can be used by the consumers (doc generator etc.), such as "excludePrivate", "excludeProtected", logo file etc.

[dend]

@pgonzal @tenry92 feels like adding yet another configuration file might be unnecessary, if the same thing can be achieved by extending package.json.

[tenry92]

@dend I would allow both variants. Some people surely prefer that kind of configuration being in the main package.json, others might want it to be in a dedicated file. Also, I can imagine for some project having 2+ tsconfig-files: one for public documentation (some doc generator shall ignore internal/private members) and one for internal documentation with other settings within it.

[dend]

@tenry92 what is the benefit of having two ways to configure the same thing? That seems like something that would add unnecessary confusion, and just extra paths to test and debug. Given that we are starting from scratch, this is an opportunity to define one standard way of doing things, instead of trying to solve problems that don't necessarily yet exist.

[tenry92]

@dend Like I said, what if someone needs one configuration for public documentation and one or internal documentation within the same project?

[dend]

@tenry92 you could probably do something like:

"tsdoc": {
    "profiles":{
        "internal":{
            "folder": "blah",
            "flavor": "foo"
        },
        "external":{
            "folder": "not-blah/some-folder",
            "flavor": "bar"
        }
    }
}

[octogonz]

API Extractor 7 eliminates the old analysis engine, which now requires every project to have the "tsdocFlavor" field in package.json. This means I would need to update 30 or 40 projects in our monorepo to add this field, which is a little tedious. Also, we realized that it might be error prone, since people might copy package.json and use it as a template for a new project, without understanding what "tsdocFlavor" is supposed to mean. Or maybe they experimented with API Exctractor and then decided not to use it, but forgot to remove the entry from package.json.

This led us to a different idea: What if, as part of the build, API Extractor wrote an output file dist/tsdoc-metadata.json that contains something like this:

{
  "tsdocVersion": "1.1",
  "toolPackages": [
    {
       "packageName": "@microsoft/tsdoc",
       "packageVersion": "0.12.4"
    },
    {
       "packageName": "@microsoft/api-extractor",
       "packageVersion": "7.0.0"
    }
  ]
}

This file would NOT be added to Git. But it would be published as part of the NPM package.

When API Extractor crawls into the package folder for a dependency, it will find the package.json file and then look for dist/tsdoc-metadata.json relative to that folder. If it finds this file, then it can assume the library has TSDoc tags compatible with API Extractor.

There are two advantages of this approach:

• No configuration effort is required to use this feature (other than making sure your .npmignore file doesn't exclude the file)
• No false positives -- if you stop using API Extractor, the file no longer get written

The loader for this file could be part of the @microsoft/tsdoc library.

@iclanton

[octogonz]

Also, I agree that the concept of "flavors" is a little vague. In this new proposal, we would simply list various tools that were invoked. Presumably these tools define some sort of extensions to the core TSDoc syntax. Knowing which tools were used should generally give enough information about how to parse the files.

If additional metadata is needed beyond that (e.g. how the tool was configured), we could allow custom properties for each "toolPackages" entry. Something like this:

{
  "tsdocVersion": "1.1",
  "toolPackages": [
    {
       "packageName": "@microsoft/tsdoc",
       "packageVersion": "0.12.4"
    },
    {
       "packageName": "@microsoft/api-extractor",
       "packageVersion": "7.0.0",
       "customData": {
         "dtsTrimming": "enabled"
       }
    }
  ]
}

[octogonz]

Updated to clarify that "tsdocVersion": "1.1" refers to the TSDoc standard version, whereas the @microsoft/tsdoc library version is tracked like any other package.

[octogonz]

@iclanton pointed out that the my-package/dist/tsdoc-metadata.json includes somewhat arbitrary assumptions about the folder structure.

Instead, he suggested the following lookup rules:

1. If package.json contains a field such as "tsdocMetadata": "./path1/path2/tsdoc-metadata.json", then that takes precedence. This convention will be rarely needed, since the rules below generally produce a good result. OTHERWISE
2. If package.json contains a field such as "typings": "./path1/path2/index.d.ts", then we would look for the file under "./path1/path2/tsdoc-metadata.json"; OTHERWISE
3. If package.json contains a field such as "main": "./path1/path2/index.js", then we would look for the file under "./path1/path2/tsdoc-metadata.json"; OTHERWISE
4. We would look for the file under "./tsdoc-metadata.json" since the default entry point is "./index.js"

[yacinehmito]

@pgonzal I like this proposition. I'm currently experimenting with api-extractor on a project that uses build instead of dist and it would be hard to justify a change only for api-extractor.

[octogonz]

We recently published an ESLint rule that validates TSDoc syntax. For example it can underline errors as you are authoring code in your editor. The ESLint plugin has similar requirements as the tsdoc-metadata.json scenario, except that:

1. ESLint needs to discover the TSDoc flavor before the project is compiled (whereas API Extractor writes tsdoc-metadata.json as a build output)
2. ESLint needs to know custom tag definitions instead of tool versions. Ideally we'd want to share custom tag configurations between multiple projects.

In PR #1628 @ecraig12345 proposed a way to define custom tags in API Extractor's proprietary config file. However, that wouldn't be easily readable by the ESLint rule. In hindsight, it seems that tsdoc-metadata.json would have been better defined as a human-authored file, and it should store the actual TSDoc configuration rather than tool versions.

New Proposal

• Let's introduce a new file tsdoc-config.json that is discovered using exactly the same lookup procedure as above, except that the package.json field will be called tsdocConfig instead of tsdocMetadata.

• The file format will store TSDocConfiguration settings like this:

  tsdoc-config.json

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

• The file format could eventually support an extends field, allowing custom tag definitions to be shared across projects by importing them using NPM resolution

• The loader for this tsdoc-config.json file format will be built into the @microsoft/tsdoc library

• Initially we can introduce this alongside tsdoc-metadata.json, but I'm thinking we should deprecate tsdoc-metadata.json and eliminate it eventually

Thoughts?

[MartynasZilinskas]

@octogonz About the extend feature, it would be nice to have a support for packages.
Similarly like how eslint works now, so you can publish as a separate package @org/tsdoc-config or tsdoc-config-name.

[octogonz]

I also want to make it an array so you can use extends to "mixin" more than one family of custom tags. For example "I want API Extractor's tags plus the tags for my documentation tool."

[octogonz]

After discussing this some more, @iclanton and I realized that tsdoc-config.json and tsdoc-metadata.json have different requirements.

tsdoc-config.json scenario:

• Used by ESLint to understand the TSDoc dialect for .ts files in the local project
• File needs to reference devDependencies (e.g. "extends": "@microsoft/api-extractor/dist/tsdocconfig-aedoc.json"), therefore...
• File is NOT suitable for publishing with an NPM package
• File needs to be available before we build anything, i.e. it's human authored and tracked by Git

tsdoc-metadata.json scenario:

• Used by e.g. API Extractor to understand the TSDoc dialect for .d.ts files from external NPM packages
• File must be published with the NPM package
• File needs to embed any extends references to avoid reliance on devDependencies, therefore...
• File must be machine-generated during the build

Both scenarios seem legitimate, although tsdoc-metadata.json is perhaps somewhat esoteric (many tools probably won't parse .d.ts files from external packages at all).

It's also worth noting that TypeScript's tsconfig.json can exist without any package.json.

Thus I'm now thinking that we should rename tsdoc-config.json to tsdocconfig.json and specify that it is always found next to tsconfig.json. Similar to tsconfig.json, the file should NOT be referenced from package.json at all. (Whereas the tsdocMetadata field still makes sense in package.json because it is published content.)

[octogonz]

We could also shorten the name from tsdoc-config.json to tsdoc.json.

[octogonz]

😊 Vote for the filename here: https://twitter.com/octogonz_/status/1195793782534393856

[octogonz]

We went with tsdoc.json since it seemed to be the clear winner in the poll.

A config loader was published as @microsoft/tsdoc-config, and is now supported by the eslint-plugin-tsdoc plugin for ESLint. Next we'll look at getting API Extractor to support this.