Skip to content

chore: add command to generate reference docs#6506

Merged
maribethb merged 8 commits intoRaspberryPiFoundation:developfrom
maribethb:gen-api
Nov 9, 2022
Merged

chore: add command to generate reference docs#6506
maribethb merged 8 commits intoRaspberryPiFoundation:developfrom
maribethb:gen-api

Conversation

@maribethb
Copy link
Contributor

@maribethb maribethb commented Oct 7, 2022

The basics

  • I branched from develop
  • My pull request is against develop
  • My code follows the style guide
  • I ran npm run format and npm run lint

The details

Resolves

fixes #5854

Proposed Changes

Adds an npm script that will generate the reference docs using API Extractor.

Behavior Before Change

No reference docs because the old tool does not support TS.

Behavior After Change

https://developers.google.com/blockly/reference/js/blockly

Reason for Changes

History of Reference Documentation

Before the introduction of TypeScript, Blockly used JsDoc comments in the code both for documentation and for Closure Compiler. In fact, Blockly was one of the examples in the google-internal documentation about how to generate reference documentation for devsite (I have emailed the team that maintains this and suggested they find a new example). The tool that we used for that is very specific to Closure-compiler style code and JsDoc annotations, and it isn't compatible with our current codebase, so we had to find a new tool.

Comparison of reference documentation generators

We considered using TypeDoc or API Extractor.

  • TypeDoc
    • popular open source library for generating reference documentation
    • generates html files of a particular structure, where each class/namespace/etc is documented on one page
  • API Extractor
    • created by Microsoft
    • generates an intermediate json file containing the API, and then generates markdown from that json
    • the generated markdown is one file per documented function/object/etc. That means browsing the docs for each class involves viewing many different pages.
    • the intermediate json file can be more easily programmatically modified as needed
    • the json file can also be used for API analysis, such as to track changes to the public API in PRs

In a comparison between the two tools, the team preferred the look and feel of the API Extractor output. Also, we plan to use the additional functionality of API Extractor in the future to monitor for unintended changes in our public API. Therefore we chose to use API Extractor.

Changes needed for API Extractor to work

Previously, we used JsDoc-style comments, which are similar but not identical to TsDoc comments. Therefore we needed to change the style and contents of many comments. That work was done mostly in #6388 and included things like fixing syntax in comments, escaping special characters, using TsDoc-specific tags in place of JsDoc analogs, etc.

I also added configuration for the tool which had it ignore certain tags that are not part of the TsDoc spec that we currently use, and ignored some warnings that are not applicable to us or we don't want to fix.

I also added linting rules for TsDoc comments in #6353. The old linting rules did not adequately lint the TsDoc style comments. This process also involved comparing multiple available tools, which you can read about in that PR description.

Problems with API Extractor

  • API Extractor renames some classes/functions/etc inexplicably adding a _2 to some names.
    • "Fixed" this via a gulp task that removes _2 from the generated intermediate json file. This may be a bug in API Extractor and may occur when the class shadows the name of a browser global variable.
  • It does not handle the case where a namespace and a class share a name, even though this is a supported pattern in TS (namespace merging). For example, we have both a class and a namespace Block, and only the documentation for the namespace would be accessible.
    • Fixed this by patching API Extractor to append the category of each item to its filename, e.g. block_class
  • The generated markdown is intended to be used on a site where multiple packages are documented, so each API item has a breadcrumb trail that links to a package overview page. This is redundant and annoying when we only have one package.
    • Fixed this by patching API Extractor to not show the breadcrumb trail for the package overview page.
  • Each individual page needs to contain devsite metadata that links it to the blockly project.
    • Used gulp-header to add the metadata to each file.
  • Lack of a TOC page for devsite.
    • Generated one automatically by reading the contents of the docs directory and the blockly.md index file.

Using patch-package

API Extractor had to be patched for two reasons, shown above. This is accomplished by using the npm package patch-package. We now have a directory called patches/ and there is a script that runs on install that will automatically apply any patches in that directory to the relevant npm module.

To create a patch, you just modify the contents of a module in node_modules then run patch-package package-name and the patch will be added to the patches/ directory. To apply all the patches, you run patch-package with no arguments, and this is configured to be done automatically after running npm install thanks to a postinstall script.

It is unfortunate to have to patch random npm modules, yes. I commented on the patch with links to the underlying issues, and the format of the patch includes a git diff so it's easy to see what has changed.

Actually using these scripts

Just run npm run docs and the entire pipeline will be run. You do need to have already run npm run build and npm run package as the script will not automatically run them. The output will be located in a docs/ directory, and you can get the contents of that directory uploaded to devsite however you want. The missing link here (and good future work) is a script that can be run from google-internal to automatically upload this content to devsite.

Documentation

Will add information to our team site.

Additional Information

@maribethb maribethb requested a review from a team as a code owner October 7, 2022 21:30
@maribethb maribethb requested a review from cpcallen October 7, 2022 21:30
@github-actions github-actions bot added the PR: chore General chores (dependencies, typos, etc) label Oct 7, 2022
@maribethb maribethb linked an issue Oct 10, 2022 that may be closed by this pull request
8 tasks
@github-actions github-actions bot added PR: chore General chores (dependencies, typos, etc) and removed PR: chore General chores (dependencies, typos, etc) labels Oct 10, 2022
Copy link
Collaborator

@cpcallen cpcallen left a comment

Choose a reason for hiding this comment

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

This is a "Request changes" but, the only strictly mandatory changes are that you fix the indenting to agree with our style guide (+2 instead of +4) and use CONSTANT_CASE where applicable. The rest are please-considers (but please do actually consider).

@cpcallen
Copy link
Collaborator

cpcallen commented Nov 9, 2022

Would you like me to push a merge commit to resolve the conflicts in this PR?

@maribethb
Copy link
Contributor Author

Would you like me to push a merge commit to resolve the conflicts in this PR?

I was pushing the commit as you were typing this probably :D

I have no explanation for the CI failures though, other than I'm sure we've seen this before but I don't know why it keeps happening.

@maribethb maribethb merged commit 010a56b into RaspberryPiFoundation:develop Nov 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

PR: chore General chores (dependencies, typos, etc)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Set up API Extractor to generate API documentation

2 participants