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

Update website code to use the new api-markdown-documenter package for generating API docs #11754

Merged
merged 38 commits into from Sep 2, 2022

Conversation

Josmithr
Copy link
Contributor

@Josmithr Josmithr commented Aug 31, 2022

Summary

This PR updates the website (/docs) build to use @fluid-tools/api-markdown-documenter to generate API documentation from the .api.json files generated by API-Extractor.

Formatting

This version of the package attempts to maintain parity with the current rendering as much as is reasonable. The only exceptions are changes where I felt the new behavior was superior and maintaining parity would require more work than keeping the better behavior. There are also a handful of other minor improvements that were easy enough to make, that I put them in v1. Changes include:

  • Dynamic column rendering in tables for properties that may or may not appear (e.g. the Modifiers column - we now only render this column in tables if any entry has a value to display).
  • Adds rendering for index signatures (which API-Documenter missed)
  • Adds Return Type column to function/method tables
  • Adds Type column to variable/parameter tables
  • Sub-headings are used in more places (e.g. where we simply used bold formatting to denote sub-sections, including Signature)
  • More explicit heading identifiers to ensure links can be uniquely resolved
  • The formatting of the injected Hugo frontmatter has been prettified to make it a bit more human-readable.

For reference, I added a couple of before and after documents under __pr-temp that demonstrates some of the formatting changes. You can view the raw markdown contents as well as the preview rendering here.

Related changes

This PR also fixes links and comment formatting issues in a number of places. Some are fixes related to the new infra, but others are simply issues I discovered while auditing formatting changes.

@github-actions github-actions bot added the base: main PRs targeted against main branch label Aug 31, 2022
docs/package.json Outdated Show resolved Hide resolved
@github-actions github-actions bot added area: loader Loader related issues area: website documentation Improvements or additions to documentation labels Sep 1, 2022
@Josmithr
Copy link
Contributor Author

Josmithr commented Sep 2, 2022

Links to the root "Packages" document are broken. Fix PR is here: #11803

Update: resolved

@Josmithr Josmithr marked this pull request as ready for review September 2, 2022 19:39
@Josmithr Josmithr requested review from msfluid-bot and a team as code owners September 2, 2022 19:39
@Josmithr Josmithr requested a review from a team as a code owner September 2, 2022 21:45
@github-actions github-actions bot added the area: framework Framework is a tag for issues involving the developer framework. Eg Aqueduct label Sep 2, 2022
@@ -0,0 +1,623 @@
{
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Remove these files before merging :)

Copy link
Member

@tylerbutler tylerbutler left a comment

Choose a reason for hiding this comment

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

Looks fine. I'm still a bit confused why frontmatter is something that has to be done outside of APi documenter. It seems reasonable to expect API documenter to support adding front-matter to output. Creating the front-matter also seems more complex that it should be. Maybe because it's JS? Anything non-trivial I prefer in TS for safety and maintainability. In particular, front matter should be typed, IMO.

@Josmithr
Copy link
Contributor Author

Josmithr commented Sep 2, 2022

Looks fine. I'm still a bit confused why frontmatter is something that has to be done outside of APi documenter. It seems reasonable to expect API documenter to support adding front-matter to output. Creating the front-matter also seems more complex that it should be. Maybe because it's JS? Anything non-trivial I prefer in TS for safety and maintainability. In particular, front matter should be typed, IMO.

Yeah, I had been debating whether or not it made sense to include a hook for that in the library. My initial thinking was that it was too use-case specific, but now I think my opinion has changed. Adding a hook for this is probably reasonable. I'll file a task to do that.

Regarding the complexity, I agree. I'm also not sure were really using most of the information we gather there. I want to do a follow-up pass to see what data we actually need so we can reduce both the logic complexity, and the amount of front-matter we append to each document.

@github-actions
Copy link
Contributor

github-actions bot commented Sep 2, 2022

🔗 Found some broken links! 💔

Run a link check locally to find them. See
https://github.com/microsoft/FluidFramework/wiki/Checking-for-broken-links-in-the-documentation for more information.

linkcheck output


> fluidframework-docs@0.25.0 linkcheck /home/runner/work/FluidFramework/FluidFramework/docs
> npm run linkcheck:fast -- --external


> fluidframework-docs@0.25.0 linkcheck:fast /home/runner/work/FluidFramework/FluidFramework/docs
> linkcheck http://localhost:1313 --skip-file skipped-urls.txt "--external"

Crawling...

http://localhost:1313/css/style.min.f22f6c22fcc37a0d857a17c9c1202a67c28deef90cc26a52b7539b903f623331.css
- (1:1272) url(...) => https://c.s-microsoft.com/static/fonts/segoe-ui/west-european/semilight/latest.eot? (HTTP 503)

http://localhost:1313/docs/apis/core-interfaces/ifluidcodedetailscomparer-interface
- (885:360) 'Array.sort' => https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/sort#Description (HTTP 200 but missing anchor)
- (896:168) 'https://..' => https://github.com/npm/node-semver#usage (HTTP 200 but missing anchor)
- (904:25) 'Array.sort' => https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/sort#Description (HTTP 200 but missing anchor)
- (950:25) 'https://..' => https://github.com/npm/node-semver#usage (HTTP 200 but missing anchor)

http://localhost:1313/docs/apis/core-interfaces/ifluidcodedetailscomparer-interface/
- (885:360) 'Array.sort' => https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/sort#Description (HTTP 200 but missing anchor)
- (896:168) 'https://..' => https://github.com/npm/node-semver#usage (HTTP 200 but missing anchor)
- (904:25) 'Array.sort' => https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/sort#Description (HTTP 200 but missing anchor)
- (950:25) 'https://..' => https://github.com/npm/node-semver#usage (HTTP 200 but missing anchor)


Summary of most serious issues:

http://localhost:1313/css/style.min.f22f6c22fcc37a0d857a17c9c1202a67c28deef90cc26a52b7539b903f623331.css
- (1:1272) url(...) => https://c.s-microsoft.com/static/fonts/segoe-ui/west-european/semilight/latest.eot? (HTTP 503)


Stats:
  114625 links
    1229 destination URLs
       1 URLs ignored
       8 warnings
       1 errors


@Josmithr Josmithr merged commit 2d91485 into microsoft:main Sep 2, 2022
@Josmithr Josmithr deleted the use-api-markdown-documenter branch September 2, 2022 23:00
@github-actions
Copy link
Contributor

github-actions bot commented Sep 2, 2022

This commit is queued for merging with the next branch! Please ignore this PR for now. Contact @microsoft/fluid-cr-infra for help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: framework Framework is a tag for issues involving the developer framework. Eg Aqueduct area: loader Loader related issues area: website base: main PRs targeted against main branch documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants