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

Generate core API docs from TSDoc comments #32148

Merged
merged 46 commits into from
Apr 3, 2019

Conversation

rudolf
Copy link
Contributor

@rudolf rudolf commented Feb 27, 2019

Summary

Fixes #31717

Uses api-extractor and api-documenter to generate documentation for the Kibana core API from TSDoc comments in the source code. This does not integrate the generated docs into the kibana documentation workflow and won't get published to the website. Instead, these markdown files are meant to be viewed in your editor or on Github. Example of viewing docs in Github

Documentation can be generated using node scripts/check_core_api_changes --docs

Todo:

  • Take a first stab at fleshing out docs like parameter and method descriptions so they're actually useful. Also add a few missing documentation tags that currently causes api-extractor to fail with a non-zero exit code.
  • Add test that checks for core api changes by comparing the API signature with common/core_api_review/kibana.api.ts. Running node scripts/check_core_api_changes --accept will update the review file and documentation and has to be commited to pass the test.

Details

To generate the documentation the following steps are executed:

  1. npm run build:types (pre-existing functionality in our build system)
    Uses src/type_exports.ts to create target/types/type_exports.d.ts. These types are exported when someone does import {...} from 'kibana' (package.json has a "types" property pointing to ./kibana.d.ts which re-exports target/types/type_exports.d.ts)
  2. api-extractor uses target/types/type_exports.d.ts to determine the surface area of the public API and generates ./build/kibana.api.json.
    This is like an AST of the API and it's TSDoc comments used in (3) by api-documenter to render markdown documentation.
    It also generates an API review file build/kibana.api.ts and compares this file to src/core/public/kibana.api.md. A difference in the type signatures means the public API signature has been changed which will print out a warning. A developer has to "accept" the change by running node scripts/check_core_api_changes --accept and then committing the updated API review file and documentation.
  3. api-documenter creates documentation by "rendering" the contents of ./build/{public/server}/kibana.api.json as markdown files which we commit to the repository.

Since api-exporter doesn't support import * as Module from __, a technique we use to prevent type collisions in src/core/index.ts we treat the public and server api's as two different packages and run api-extractor twice. The biggest drawback is that api-extractor uses the name field in package.json as for api review files and documentation.

Checklist

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

For maintainers

@rudolf rudolf self-assigned this Feb 27, 2019
@rudolf rudolf added Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc Feature:New Platform labels Feb 27, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-platform

@rudolf rudolf added non-issue Indicates to automation that a pull request should not appear in the release notes v8.0.0 labels Feb 27, 2019
@rudolf rudolf requested a review from spalger February 28, 2019 14:06
@rudolf rudolf force-pushed the rm-core-api-docs branch 2 times, most recently from 9134c67 to 12c3bdf Compare March 1, 2019 09:09
@elasticmachine
Copy link
Contributor

💔 Build Failed

rudolf added 4 commits March 1, 2019 11:29
Uses api-extractor and api-documenter to generate documentation for
the Kibana core API from TSDoc comments in the source code.

Documentation can be generated using `npm run docs:api`.

I used --no-verify to ignore the following pre-commit hook errors:
1. Filenames MUST use snake_case - api-extractor.json
   It's possible to specify a different config file, but I prefer to keep the "standard" config file name.
2. UNHANDLED ERROR: Unable to find tsconfig.json file selecting "common/core_api_review/kibana.api.ts". Ensure one exists and it is listed in "src/dev/typescript/projects.ts"
   This is not a source file, so safe to ignore.
@rudolf rudolf force-pushed the rm-core-api-docs branch from 12c3bdf to 8bcc4a2 Compare March 1, 2019 11:25
@elastic elastic deleted a comment from elasticmachine Mar 1, 2019
@elastic elastic deleted a comment from elasticmachine Mar 1, 2019
@elastic elastic deleted a comment from elasticmachine Mar 1, 2019
@elastic elastic deleted a comment from elasticmachine Mar 1, 2019
@elastic elastic deleted a comment from elasticmachine Mar 1, 2019
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

I initially wanted to include this as a precommit hook, but it takes
quite long to execute (~12s) so might be better suited as a test or
as part of the release process.

The script currently fails because api-extractor uses an older version
of typescript.
@rudolf
Copy link
Contributor Author

rudolf commented Mar 1, 2019

Checking for core API changes takes quite long, which can get annoying in a pre-commit hook, so I think it's better suited as a test.

api-exporter doesn't yet use the latest typescript compiler introduced in (#32063) so it returns a non-zero exit code which means we can't enforce it as a test yet.

I suggest that we don't block on api-exporter and merge this so long, manually keeping docs updated. Once it supports the latest typescript we can update api-exporter and automate the detection of api changes as a test.

@rudolf rudolf added the review label Mar 1, 2019
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@rudolf rudolf removed request for a team April 2, 2019 07:55
@rudolf
Copy link
Contributor Author

rudolf commented Apr 2, 2019

@monfera Thanks for the review. This shouldn't impact any other teams, but due to a large merge commit, other teams were incorrectly tagged as code owners.

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@rudolf rudolf requested a review from mshustov April 2, 2019 12:03
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@@ -14,5 +14,5 @@ export interface PluginSetupContext

| Property | Type | Description |
| --- | --- | --- |
| [elasticsearch](./kibana-plugin-server.pluginsetupcontext.elasticsearch.md) | `{`<p/>` adminClient$: Observable<ClusterClient>;`<p/>` dataClient$: Observable<ClusterClient>;`<p/>` }` | |
| [elasticsearch](./kibana-plugin-server.pluginsetupcontext.elasticsearch.md) | <code>{`<p/>` adminClient$: Observable&lt;ClusterClient&gt;;`<p/>` dataClient$: Observable&lt;ClusterClient&gt;;`<p/>` }</code> | |
Copy link
Contributor

Choose a reason for hiding this comment

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

ooooh. they fix one thing and break another. let's don't block this PR
2019-04-02_22-07-22


| Parameter | Type | Description |
| --- | --- | --- |
| req | `{`<p/>` headers?: Headers;`<p/>` }` | Request the `ScopedClusterClient` instance will be scoped to. |
Copy link
Contributor

Choose a reason for hiding this comment

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

this is bummer 😞 ... probably we should file some issues in their repo and track progress.
not a blocker for the current PR

Copy link
Contributor

@mshustov mshustov left a comment

Choose a reason for hiding this comment

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

I think there are several issues in documenter, so it's not a blocker for the current PR, but nice to fix later

  • cross links in types. when a typeA is mentioned in TypeB.md, there should be a link to typeA.md page
  • weird alignments in typescript code blocks
  • escaped line break for object types {<p/> adminClient$: Observable<ClusterClient>;<p/> dataClient$: Observable<ClusterClient>;<p/> }
  • warning comment for autogenerated documentation page

only problem in the code
https://github.com/elastic/kibana/pull/32148/files/51b8bc1f09219075eeb7209e5a93c3db6c91474e#r270865425

@elasticmachine
Copy link
Contributor

💔 Build Failed

@rudolf
Copy link
Contributor Author

rudolf commented Apr 3, 2019

After the latest merge there are still undocumented classes and missing exports. Some of them are related to #34416 . I want to merge this to plug the leaky bucket of new changes being committed to master which causes api-extractor to fail. After #34416 we can do a cleanup of the exported types and documentation.

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@rudolf rudolf merged commit 5c45797 into elastic:master Apr 3, 2019
@rudolf rudolf deleted the rm-core-api-docs branch April 3, 2019 10:26
joshdover pushed a commit to rudolf/kibana that referenced this pull request Apr 3, 2019
* Generate core API docs from TSDoc comments

Uses api-extractor and api-documenter to generate documentation for
the Kibana core API from TSDoc comments in the source code.

Documentation can be generated using `npm run docs:api`.

I used --no-verify to ignore the following pre-commit hook errors:
1. Filenames MUST use snake_case - api-extractor.json
   It's possible to specify a different config file, but I prefer to keep the "standard" config file name.
2. UNHANDLED ERROR: Unable to find tsconfig.json file selecting "common/core_api_review/kibana.api.ts". Ensure one exists and it is listed in "src/dev/typescript/projects.ts"
   This is not a source file, so safe to ignore.

* Flesh out API docs a little bit

* Ignore snake_case check for api-extractor.json

* Ignore api-extractor's review file from pre-commit check

* Try to fix build failing by using masters yarn.lock

* I'm being stupid

* Found a better home for ignoring common/core_api_review/kibana.api.ts

* Node script for detecting core API changes

I initially wanted to include this as a precommit hook, but it takes
quite long to execute (~12s) so might be better suited as a test or
as part of the release process.

The script currently fails because api-extractor uses an older version
of typescript.

* Fix tslint precommit hook ignore condition

* Write tsdoc-metadata.json into ./build

* Add LogMeta and ElasticSearch to exported types & docs

* Suppress logging when running api-extractor from script

* Improve check_core_api_changes script and run as test

* Inline api-extractor.json config

* Fix check_core_api_changes --help flag

* LogMeta TSDoc comments

* check_core_api_changes: fail if api-extractor produces warnings or errors

And print more useful messages to the console

* Move ignored ts files list into dev/file

* Add back build:types since api-exporter cannot operate on source files

* Upgrade api-exporter/documenter

* api-extractor: independantly analyze core/public and core/server

Becasue of microsoft/rushstack#1029
api-extractor can't use core/index.ts as a single entry point for
analyzing the public and server API's as isolated namespaces.

Instead we analyze these projects separately. This introduces other
problems like the api review files and documentation always being
called "kibana." from the package.json filename.

* Build types as part of build task

* Include types in typescript browser compilation

* Force inclusion of core/public for building types

* Fix api review filename in api-exporter errors

* Update docs and API review files

* Fix api-extractor warnings

* Remove ts file ignored list since it's no longer necessary

* Rename exported api package name

* Review comments

* Export other missing types

* Upgrade api-documenter to latest beta

* Export more missing types

* Fix warnings and add api-exporter to Jenkins tests

* Correctly handle runBuildTypes() exceptions

* Fix another swallowed exception

* Fix api-extractor warnings after master merge
rudolf added a commit that referenced this pull request Apr 4, 2019
* Generate core API docs from TSDoc comments (#32148)

* Generate core API docs from TSDoc comments

Uses api-extractor and api-documenter to generate documentation for
the Kibana core API from TSDoc comments in the source code.

Documentation can be generated using `npm run docs:api`.

I used --no-verify to ignore the following pre-commit hook errors:
1. Filenames MUST use snake_case - api-extractor.json
   It's possible to specify a different config file, but I prefer to keep the "standard" config file name.
2. UNHANDLED ERROR: Unable to find tsconfig.json file selecting "common/core_api_review/kibana.api.ts". Ensure one exists and it is listed in "src/dev/typescript/projects.ts"
   This is not a source file, so safe to ignore.

* Flesh out API docs a little bit

* Ignore snake_case check for api-extractor.json

* Ignore api-extractor's review file from pre-commit check

* Try to fix build failing by using masters yarn.lock

* I'm being stupid

* Found a better home for ignoring common/core_api_review/kibana.api.ts

* Node script for detecting core API changes

I initially wanted to include this as a precommit hook, but it takes
quite long to execute (~12s) so might be better suited as a test or
as part of the release process.

The script currently fails because api-extractor uses an older version
of typescript.

* Fix tslint precommit hook ignore condition

* Write tsdoc-metadata.json into ./build

* Add LogMeta and ElasticSearch to exported types & docs

* Suppress logging when running api-extractor from script

* Improve check_core_api_changes script and run as test

* Inline api-extractor.json config

* Fix check_core_api_changes --help flag

* LogMeta TSDoc comments

* check_core_api_changes: fail if api-extractor produces warnings or errors

And print more useful messages to the console

* Move ignored ts files list into dev/file

* Add back build:types since api-exporter cannot operate on source files

* Upgrade api-exporter/documenter

* api-extractor: independantly analyze core/public and core/server

Becasue of microsoft/rushstack#1029
api-extractor can't use core/index.ts as a single entry point for
analyzing the public and server API's as isolated namespaces.

Instead we analyze these projects separately. This introduces other
problems like the api review files and documentation always being
called "kibana." from the package.json filename.

* Build types as part of build task

* Include types in typescript browser compilation

* Force inclusion of core/public for building types

* Fix api review filename in api-exporter errors

* Update docs and API review files

* Fix api-extractor warnings

* Remove ts file ignored list since it's no longer necessary

* Rename exported api package name

* Review comments

* Export other missing types

* Upgrade api-documenter to latest beta

* Export more missing types

* Fix warnings and add api-exporter to Jenkins tests

* Correctly handle runBuildTypes() exceptions

* Fix another swallowed exception

* Fix api-extractor warnings after master merge

* Update yarn.lock

* Fix erraneous type

* Revert "Update yarn.lock"

This reverts commit 85a8093.

* Revert "Fix erraneous type"

This reverts commit 7f0550c.

* Backport #32440

* Update core api signature and docs
@rudolf
Copy link
Contributor Author

rudolf commented Apr 16, 2019

  • cross links in types. when a typeA is mentioned in TypeB.md, there should be a link to typeA.md page

I've created microsoft/rushstack#1237

  • weird alignments in typescript code blocks

I created microsoft/rushstack#1239

  • escaped line break for object types {<p/> adminClient$: Observable<ClusterClient>;<p/> dataClient$: Observable<ClusterClient>;<p/> }

Upstream PR microsoft/rushstack#1234

  • warning comment for autogenerated documentation page

Fixed in #34817

@lukeelmers lukeelmers mentioned this pull request May 12, 2020
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:New Platform non-issue Indicates to automation that a pull request should not appear in the release notes review Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc v7.2.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Document new platform service APIs
9 participants