[api-extractor] Adds package-defined TSDoc tag support #1950

nicholasrice · 2020-06-20T06:53:39Z

I took a shot at implementing #519 based on the feedback of #1628. I'm new to this code-base so very open to feedback on any of this. I also tried to implement this without causing any breaking changes but please call out any issues I may have missed.

This change involves removing AedocDefinitions references from api-extractor and api-extractor-model. The ExtractorConfig has been updated to construct the tsdocConfiguration for the Extractor process. The tsdocConfiguration is configured from one of two source tsdoc.json files:

The tsdoc.json file in the project folder, if it exists
otherwise from the api-extractor's tsdoc.json

Once configured, the configuration is provided to parts of the process requiring a tsdocConfiguration.

api-extractor-model was also changed to remove it's dependency on AedocDefintions - instead expecting any non-standard TSDoc tags for a given package to be enumerated in the metadata field of the model.

I made a few decisions that I'm unsure about and also had a few questions:

Any non-standard tsdoc tag defined by the package is output by API Extractor to the metadata property of the produced {package}.api.json . This is so that tags can be ingested by the ApiPackage implementation and so that any non-standard tags can exist per package. This deviates from API Extractor: Add support for custom TSDoc tags #1628, where tags are provided to as part of the options config to the ApiModel implementation. If API Extractor: Add support for custom TSDoc tags #1628 is the preferred implementation (or if there is any other preferred implementation) please let me know and I'll adjust.
I've tagged the AedocDefinitions implementation as @deprecated . I believe it is technically internal though, so should it be removed entirely?
Where should this change be documented? Anything I need to do for documentation?
I didn't add any explicit tests for this because I'm unsure how exactly I should do that. Should I add a package in build-tests that contains custom TSDoc tags?
This doesn't change anything in api-documenter - should it to fully address [api-extractor] Support extensible AEDoc tags #519?

As always, open to any and all feedback.

closes #519

octogonz · 2020-06-29T18:38:43Z

apps/api-extractor-model/src/items/ApiDocumentedItem.ts

@@ -47,7 +46,21 @@ export class ApiDocumentedItem extends ApiItem {
    const documentedJson: IApiDocumentedItemJson = jsonObject as IApiDocumentedItemJson;

    if (documentedJson.docComment) {
-      const tsdocParser: tsdoc.TSDocParser = new tsdoc.TSDocParser(AedocDefinitions.tsdocConfiguration);
+      const tsdocConfiguration: tsdoc.TSDocConfiguration = new tsdoc.TSDocConfiguration();


This is a performance concern, since it will initialize a new TSDocConfiguration for every single doc comment.

Instead, we should construct it once and share it. The DeserializerContext could be used to store it:

rushstack/apps/api-extractor-model/src/model/ApiPackage.ts

Lines 160 to 168 in a30cdf5

const context: DeserializerContext = new DeserializerContext({

apiJsonFilename,

toolPackage: jsonObject.metadata.toolPackage,

toolVersion: jsonObject.metadata.toolVersion,

versionToDeserialize: versionToDeserialize

});

return ApiItem.deserialize(jsonObject, context) as ApiPackage;

}

Good call - DeserializerContext now stores the TSDocConfiguration and non-standard tags now configured in ApiPackage.loadFromJsonFile()

nicholasrice · 2020-07-24T19:47:54Z

@octogonz any chance you have some time to take another look at this? We're really looking forward to using this feature. Thanks in advance!

octogonz · 2020-07-24T21:24:35Z

I apologize -- I recently got sandbagged by a bunch of high priority work for the Heft and Rush projects, and have not been able to give much time to API Extractor. The pressure should hopefully be off by end of next week at the latest. This is a cool PR and I feel bad for delaying it!

nicholasrice · 2020-07-24T22:57:48Z

No worries - I totally understand ;) Thanks @octogonz!

kazupon · 2020-11-22T18:40:38Z

Is there any update?
I need this feature.

EisenbergEffect · 2021-01-14T18:05:36Z

Pinging @octogonz Not having this is really causing an issue for our open source efforts. Is there anything we need to do to help get this PR merged? I have engineering resources I can commit to seeing this PR through.

octogonz · 2021-01-15T20:42:50Z

Pinging @octogonz Not having this is really causing an issue for our open source efforts. Is there anything we need to do to help get this PR merged? I have engineering resources I can commit to seeing this PR through.

@EisenbergEffect I'll take a look at this today and follow up.

octogonz · 2021-01-16T04:59:16Z

Okay @EisenbergEffect I've merged master into this branch and read over the design discussion again.

Some thoughts:

The main idea is to move the AedocDefinitions.ts config into tsdoc.json file so that it is visible to other tools. The tsdoc.json schema currently can represent this data:

   configuration.addTagDefinitions(
     [
       AedocDefinitions.betaDocumentation,
       AedocDefinitions.internalRemarks,
       AedocDefinitions.preapprovedTag
     ],
     true
   );

...but it seems not able to represent this data:

   configuration.setSupportForTags(
     [
       StandardTags.alpha,
       StandardTags.beta,
       StandardTags.decorator,
       StandardTags.defaultValue,
       StandardTags.deprecated,
       StandardTags.eventProperty,
       StandardTags.example,
   . . .

Thus I think we need to add a "supportedTags" section to the tsdoc.json schema. That should be pretty easy.

This PR updates the .api.json file format to serialize the tsdoc.json config as part of ApiPackage. Since nothing about this serialization is unique to API Extractor, it seems better to implement the serialization in the @microsoft/tsdoc-config package rather than in @microsoft/api-extractor-model. This way it can more easily stay in sync with the tsdoc.json data model. (The data can still be embedded in API Extractor's .api.json file -- merely the serialization format will be owned by @microsoft/tsdoc-config. Maybe it is exactly the same JSON structure as tsdoc.json.)
When custom tags are defined, they need to inherit from API Extractor's tsdoc.json file using the "extends" field. I suspect this is already working, but we need to add a test to demonstrate how it works and validate the implementation.

I think we can implement all these things pretty quickly. Lemme see if I can make some PRs over the weekend.

octogonz · 2021-01-18T03:28:12Z

I think we can implement all these things pretty quickly.

Here's an upstream PR that adds the missing features: microsoft/tsdoc#277

EisenbergEffect · 2021-01-19T16:06:55Z

Thanks @octogonz !
@nicholasrice Can you continue to follow-up as needed? Thanks!

octogonz · 2021-01-22T06:20:34Z

Here's a second PR that sorts out a problem where the serialized TSDoc config includes the predefined standard tags, which would then conflict with the tags that are predefined when api-extractor-model deserializes that data: microsoft/tsdoc#279

Once that's published, I think we'll have all the pieces necessary to finally complete this PR. 😊🤞

nicholasrice · 2021-01-22T15:09:42Z

Thanks @octogonz! This is awesome to see; we're really looking forward to this feature.

…ags implementation

nicholasrice · 2021-03-23T02:46:13Z

Hey @octogonz! I'm in the process of working in the tsdoc and tsdoc-config package updates and implementing your feedback and I have a question for you. I see you implemented a saveToObject(), which I'm using to serialize the configuration state to the .api.json files.

To build up the TSDocConfiguration from the .api.json, I'm having to re-wind the syntaxKind fields from their friendly name (calculated in TSDocConfigFile._serializeTagDefinition) into the corresponding TSDocTagSyntaxKind, which doesn't seem like api-extractor-model's responsibility. Is there a way to re-construct the the TSDocTagDefinition objects from the output of saveToObject() that doesn't involve switching friendly-name values for TSDocTagSyntaxKind values myself? Or paraphs there is a different approach I should be taking to hydrate the configuration in api-extractor-model?

nicholasrice · 2021-03-23T18:25:01Z

apps/api-extractor-model/src/model/ApiPackage.ts

@@ -14,6 +14,18 @@ import { ApiDocumentedItem, IApiDocumentedItemOptions } from '../items/ApiDocume
 import { ApiEntryPoint } from './ApiEntryPoint';
 import { IApiNameMixinOptions, ApiNameMixin } from '../mixins/ApiNameMixin';
 import { DeserializerContext, ApiJsonSchemaVersion } from './DeserializerContext';
+import { TSDocConfiguration, TSDocTagDefinition, TSDocTagSyntaxKind } from '@microsoft/tsdoc';
+
+interface ITagConfigJson {


@octogonz defining these here is odd to me because they're essentially a copy of the interfaces defined in tsdoc-config and are the product of saveToObject(), but saveToObject() returns unknown. Should I do something to address this?

It was easy, so I went ahead and implemented the missing loadFromObject() operation in microsoft/tsdoc#288

nicholasrice · 2021-03-23T18:26:20Z

apps/api-extractor-model/src/model/ApiPackage.ts

+    tsdocConfiguration.addTagDefinitions(
+      tagDefinitions.map((definition) => {
+        const { syntaxKind } = definition;
+        const formattedSyntaxKind: TSDocTagSyntaxKind =


This is the re-mapping I am referring to in my previous comment. Should there be a mechanism from tsdoc-config to build up a TSDocConfigFile from the product of TSDocConfigFile.saveToObject()?

@nicholasrice The right solution would be to implement a corresponding TSDocConfigFile.loadFromObject() to handle loading of the object. Ideally it would be integrated with the TSDocConfigFile.loadFile() implementation, although there is an interesting nuance that loadFile() considers file paths and "extends" resolution which assume that the objects have meaningful file paths. So a bit of abstraction might be needed to handle that. (Perhaps that is why I procrastinated it?)

If you want to make a PR for TSDoc that would be great. However your PR has been open for a very long time, so if it helps speed things along, I would also be okay with merging a less ideal solution to get you unblocked. And then we could come back later and improve the tsdoc-config API.

I am away on vacation right now, but I will review this PR as soon as I get back. Sorry about the delays.

For future reference if you need help getting PRs merged @iclanton's team at Microsoft is a good point of contact, and also we now have a #contributor-helpline chatroom for getting attention of the maintainers. Thanks!

EisenbergEffect · 2021-03-31T20:23:49Z

Hey @octogonz 😄 Thanks for working with us on this. I think we're ready for another round of review from you. Nick had a few questions that he dropped above. Would be great to get these last few things wrapped, merged, and released. We're pretty excited for this work as we'll be able to make some huge improvements to our documentation and it will also enable some other teams we're working with as well. 🎊

octogonz · 2021-04-14T16:45:53Z

@EisenbergEffect I apologize for the delay. I'm on vacation -- will be back next week.

…e/custom-tags # Conflicts: # common/config/rush/nonbrowser-approved-packages.json # common/config/rush/pnpm-lock.yaml # common/config/rush/repo-state.json

…doc.json to the "api-documenter-test" project

octogonz · 2021-04-17T16:29:59Z

Okay @nicholasrice sorry again about the delays in getting this PR reviewed. I've updated your branch to use TSConfigFile.loadFromObject(), and also updated ApiJsonSchemaVersion to implement backwards compatibility for your change to the .api.json file format. I also moved api-extractor/tsdoc.json to api-extractor/extends/tsdoc-base.json to make it more clear that projects are expected to extend from this file.

After merging this PR we'll do a little more testing and then we should finally be ready to release this. After using it a bit, it is a pretty cool feature. I'm very excited about it! Thanks for all your work to make it happen.

EisenbergEffect · 2021-04-19T14:33:49Z

Awesome @octogonz ! Thank you so much for helping to move this through 😄

nicholasrice · 2021-04-19T16:50:31Z

Thank you @octogonz! It's awesome to see this go through, thanks for working with me on it :)

octogonz · 2021-04-20T07:01:39Z

This was released with API Extractor 7.14.0.

Docs here: https://api-extractor.com/pages/configs/tsdoc_json/

nicholasrice requested review from apostolisms, D4N14L, halfnibble, iclanton, octogonz, patmill and sachinjoseph as code owners June 20, 2020 06:53

octogonz reviewed Jun 29, 2020

View reviewed changes

nicholasrice force-pushed the users/nirice/custom-tags branch from 545836f to 4e8dd5c Compare July 10, 2020 16:38

nicholasrice force-pushed the users/nirice/custom-tags branch from 4e8dd5c to 0546564 Compare July 22, 2020 16:12

nicholasrice force-pushed the users/nirice/custom-tags branch from 0546564 to a1f3c2e Compare July 29, 2020 21:47

deyaaeldeen mentioned this pull request Jan 22, 2021

[track2] should we publish tsdoc-metadata.json Azure/azure-sdk-for-js#8662

Closed

nicholasrice added 7 commits March 22, 2021 10:38

adds tsdoc-config and updates tsdoc to have compatible type signatures

a3010f7

construct tsdoc configuration in ExtractorConfig

03b0127

wire tsdocConfiguration throughout the api-extractor

ff63440

remove references to AedocDefinitions and replace with configurable t…

5451da7

…ags implementation

update test fixtures

61bdcb6

rush change results

6bd65e4

move tsdoc configuration construction to DeserializerContext

7ef3840

nicholasrice added 2 commits March 22, 2021 10:59

rush update

32b306e

update tsdoc and tsdoc-config

9ecc08f

implement config emission and parsing, add test cases, and rush rebuild

52829d8

nicholasrice force-pushed the users/nirice/custom-tags branch from 0bb6a89 to 52829d8 Compare March 23, 2021 18:22

nicholasrice commented Mar 23, 2021

View reviewed changes

octogonz added 2 commits April 16, 2021 12:15

Merge remote-tracking branch 'remotes/origin/master' into users/niric…

0ce7230

…e/custom-tags # Conflicts: # common/config/rush/nonbrowser-approved-packages.json # common/config/rush/pnpm-lock.yaml # common/config/rush/repo-state.json

rush update

7d49b39

octogonz added a commit to microsoft/tsdoc that referenced this pull request Apr 16, 2021

Add TSDocConfigFile.loadFromObject() needed for microsoft/rushstack#1950

dbbf5df

octogonz mentioned this pull request Apr 16, 2021

Add TSDocConfigFile.loadFromObject() microsoft/tsdoc#288

Merged

octogonz added 9 commits April 16, 2021 16:26

Upgrade to tsdoc-config 0.15.0 which adds the loadFromObject() API

9bbf43f

rush update

ad628a8

Move tsdoc-config to nonbrowser-approved-packages.json

00b9aee

rush update

ff5f108

Use TSConfigFile.loadFromObject() to deserialize the TSDoc configuration

bbcbe92

Increment the ApiJsonSchemaVersion and implement backwards compatibility

156aed9

Move the AEDoc definitions from ./tsdoc.json --> extends/tsdoc-base.json

8a5d5c2

Add launch.json config for debugging Jest tests

f55ebc2

Update ExtractorConfig to load tsdoc-base.json, and add an example ts…

00ed463

…doc.json to the "api-documenter-test" project

octogonz approved these changes Apr 17, 2021

View reviewed changes

octogonz enabled auto-merge April 17, 2021 16:26

octogonz merged commit 87f2647 into microsoft:master Apr 17, 2021

Hotell mentioned this pull request Jul 28, 2022

[api-extractor]: support custom access modifiers for API stripping #3558

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[api-extractor] Adds package-defined TSDoc tag support #1950

[api-extractor] Adds package-defined TSDoc tag support #1950

nicholasrice commented Jun 20, 2020 •

edited

Loading

octogonz Jun 29, 2020

nicholasrice Jun 30, 2020

nicholasrice commented Jul 24, 2020

octogonz commented Jul 24, 2020

nicholasrice commented Jul 24, 2020

kazupon commented Nov 22, 2020

EisenbergEffect commented Jan 14, 2021

octogonz commented Jan 15, 2021

octogonz commented Jan 16, 2021

octogonz commented Jan 18, 2021

EisenbergEffect commented Jan 19, 2021

octogonz commented Jan 22, 2021

nicholasrice commented Jan 22, 2021

nicholasrice commented Mar 23, 2021

nicholasrice Mar 23, 2021

octogonz Apr 16, 2021

nicholasrice Mar 23, 2021

octogonz Apr 14, 2021

EisenbergEffect commented Mar 31, 2021

octogonz commented Apr 14, 2021

octogonz commented Apr 17, 2021 •

edited

Loading

EisenbergEffect commented Apr 19, 2021

nicholasrice commented Apr 19, 2021

octogonz commented Apr 20, 2021

	const context: DeserializerContext = new DeserializerContext({
	apiJsonFilename,
	toolPackage: jsonObject.metadata.toolPackage,
	toolVersion: jsonObject.metadata.toolVersion,
	versionToDeserialize: versionToDeserialize
	});

	return ApiItem.deserialize(jsonObject, context) as ApiPackage;
	}

[api-extractor] Adds package-defined TSDoc tag support #1950

[api-extractor] Adds package-defined TSDoc tag support #1950

Conversation

nicholasrice commented Jun 20, 2020 • edited Loading

octogonz Jun 29, 2020

Choose a reason for hiding this comment

nicholasrice Jun 30, 2020

Choose a reason for hiding this comment

nicholasrice commented Jul 24, 2020

octogonz commented Jul 24, 2020

nicholasrice commented Jul 24, 2020

kazupon commented Nov 22, 2020

EisenbergEffect commented Jan 14, 2021

octogonz commented Jan 15, 2021

octogonz commented Jan 16, 2021

octogonz commented Jan 18, 2021

EisenbergEffect commented Jan 19, 2021

octogonz commented Jan 22, 2021

nicholasrice commented Jan 22, 2021

nicholasrice commented Mar 23, 2021

nicholasrice Mar 23, 2021

Choose a reason for hiding this comment

octogonz Apr 16, 2021

Choose a reason for hiding this comment

nicholasrice Mar 23, 2021

Choose a reason for hiding this comment

octogonz Apr 14, 2021

Choose a reason for hiding this comment

EisenbergEffect commented Mar 31, 2021

octogonz commented Apr 14, 2021

octogonz commented Apr 17, 2021 • edited Loading

EisenbergEffect commented Apr 19, 2021

nicholasrice commented Apr 19, 2021

octogonz commented Apr 20, 2021

nicholasrice commented Jun 20, 2020 •

edited

Loading

octogonz commented Apr 17, 2021 •

edited

Loading