improvement(api-markdown-documenter): Update default suite configuration to leverage new hierarchy options #23602

Josmithr · 2025-01-17T20:07:00Z

The default suite structure has been updated as follows:
- Package and Namespace items now generate documents inside of their own folder hierarchy, yielding documents named "index".
- Enum and TypeAlias items now generate their own documents (rather than being rendered as sections under their parent document).

E.g., old output would likely look something like

- index.md
- package-a.md
- package-a/
    - foo.md
    - ...
- package-b.md
- package-b/
    - bar.md
    - ...

And the new output would likely look more like:

- index.md
- package-a/
    - index.md
    - foo.md
    - ...
- package-b/
    - index.md
    - bar.md
    - ...

Updates to the default-config test assets demonstrate the difference.

Also update the spase-config test suite to move the package documents under their own hierarchy to be more demonstrative of what an expected use case would likely look like. Other test cases have been updated to maintain existing behavior, accounting for the new defaults.

AB#24263

Copilot reviewed 35 out of 51 changed files in this pull request and generated no comments.

Files not reviewed (16)

tools/api-markdown-documenter/src/test/snapshots/html/simple-suite-test/default-config/index.html: Language not supported
tools/api-markdown-documenter/src/test/snapshots/html/simple-suite-test/default-config/test-suite-a/index.html: Language not supported
tools/api-markdown-documenter/src/test/snapshots/html/simple-suite-test/default-config/test-suite-a/testabstractclass-class.html: Language not supported
tools/api-markdown-documenter/src/test/snapshots/html/simple-suite-test/default-config/test-suite-a/testclass-class.html: Language not supported
tools/api-markdown-documenter/src/test/snapshots/html/simple-suite-test/default-config/test-suite-a/testemptyinterface-interface.html: Language not supported
tools/api-markdown-documenter/src/test/snapshots/html/simple-suite-test/default-config/test-suite-a/testenum-enum.html: Language not supported
tools/api-markdown-documenter/src/test/snapshots/html/simple-suite-test/default-config/test-suite-a/testinterface-interface.html: Language not supported
tools/api-markdown-documenter/src/test/snapshots/html/simple-suite-test/default-config/test-suite-a/testinterfaceextendingotherinterfaces-interface.html: Language not supported
tools/api-markdown-documenter/src/test/snapshots/html/simple-suite-test/default-config/test-suite-a/testinterfacewithindexsignature-interface.html: Language not supported
tools/api-markdown-documenter/src/test/snapshots/html/simple-suite-test/default-config/test-suite-a/testinterfacewithtypeparameter-interface.html: Language not supported
tools/api-markdown-documenter/src/test/snapshots/html/simple-suite-test/default-config/test-suite-a/testmappedtype-typealias.html: Language not supported
tools/api-markdown-documenter/src/test/snapshots/html/simple-suite-test/default-config/test-suite-a/testmodule-namespace/index.html: Language not supported
tools/api-markdown-documenter/src/test/snapshots/html/simple-suite-test/default-config/test-suite-a/testnamespace-namespace/index.html: Language not supported
tools/api-markdown-documenter/src/test/snapshots/html/simple-suite-test/default-config/test-suite-a/testnamespace-namespace/testclass-class.html: Language not supported
tools/api-markdown-documenter/src/test/snapshots/html/simple-suite-test/default-config/test-suite-a/testnamespace-namespace/testenum-enum.html: Language not supported
tools/api-markdown-documenter/src/test/snapshots/html/simple-suite-test/default-config/test-suite-a/testnamespace-namespace/testinterface-interface.html: Language not supported

github-actions · 2025-01-17T20:15:08Z

🔗 No broken links found! ✅

Your attention to detail is admirable.

linkcheck output


> [email protected] ci:check-links /home/runner/work/FluidFramework/FluidFramework/docs
> start-server-and-test "npm run serve -- --no-open" 3000 check-links

1: starting server using command "npm run serve -- --no-open"
and when url "[ 'http://127.0.0.1:3000' ]" is responding with HTTP status code 200
running tests using command "npm run check-links"


> [email protected] serve
> docusaurus serve --no-open

[SUCCESS] Serving "build" directory at: http://localhost:3000/

> [email protected] check-links
> linkcheck http://localhost:3000 --skip-file skipped-urls.txt

Crawling...

Stats:
  170508 links
    1603 destination URLs
    1842 URLs ignored
       0 warnings
       0 errors

alexvy86 · 2025-01-22T18:57:49Z

...-documenter/src/test/snapshots/html/simple-suite-test/default-config/test-suite-a/index.html

@@ -140,25 +140,25 @@ <h2>Functions</h2>
          </thead>
          <tbody>
            <tr>
-              <td><a href="./test-suite-a#testfunction-function">testFunction(testParameter, testOptionalParameter)</a></td>
+              <td><a href="./test-suite-a/#testfunction-function">testFunction(testParameter, testOptionalParameter)</a></td>


Does this do the expected thing? Go to the index.md file and then to the testfunction-function anchor?

It does, yeah. That said, it might be better to trim off the trailing slash for /index cases. Should be an easy enough change to make, so I'll do that before merging.

Could we be explicit and add index.md to the target instead? I'd be even more surprised if things work without that slash in the middle 😆 . I thought it at least made it realize test-suite-a is a folder, not a file that contain the anchor.

Actually, that makes a bunch of changes to other test cases. I'll put up a separate PR for that change.

Could we be explicit and add index.md to the target instead? I'd be even more surprised if things work without that slash in the middle 😆 . I thought it at least made it realize test-suite-a is a folder, not a file that contain the anchor.

For link resolution, I think it ultimately depends on the system being used. This code is trying to target the lowest common denominator. Explicitly including index might be more reasonable, but I would need to test it.

Alright. Tested. Including index ends up breaking Docusaurus. And it previously broke Hugo (which is why the special-cased handling for index was added originally.

Since the link paths are routes rather than file paths (the layer involved here doesn't know what kinds of files are being created, so it has to work on file-agnostic routes) and site generators don't generally create routes for /index, those links end up being invalid.

Technically, either choice of including or excluding the trailing / in this case could break a consumer based on their ecosystem. Given this, I'm actually leaning towards preserving the / in these cases. Preserving it also technically reduces the risk of ambiguity, since the following scenario is possible and technically valid, even if this library won't produce it:

foo.md foo/index.md

Where /foo might resolve to foo.md, and /foo/ might resolve to foo/index.md.

alexvy86

One question, but otherwise lgtm. I didn't look at the new files in test assets in detail, I'm assuming the content is identical to the one that used to be in other files previously.

alexvy86 · 2025-01-22T21:34:36Z

tools/api-markdown-documenter/src/api-item-transforms/configuration/Hierarchy.ts

 			case ApiItemKind.Package: {
-				return getUnscopedPackageName(apiItem as ApiPackage);
+				return "index";


So this bit doesn't impact how the link is rendered, right?

The only change would be from .../foo to .../foo/. The default folder-naming policy uses the same getUnscopedPackageName helper for package items.

E.g.: https://github.com/microsoft/FluidFramework/pull/23602/files#diff-7eaefbdaaddc08378099d0cc9a0203382345c98a59fa0a2af143e2e4e1aab332R3

Josmithr added 4 commits January 17, 2025 19:42

improvement: Update default suite structure

56ba953

style: Prettier

c977f81

test: Fix tests

900286a

test: Fix test hierarchy configs

6264894

github-actions bot added the base: main PRs targeted against main branch label Jan 17, 2025

Josmithr requested review from alexvy86 and a team January 17, 2025 20:12

Josmithr marked this pull request as ready for review January 17, 2025 20:12

Copilot bot review requested due to automatic review settings January 17, 2025 20:12

Copilot AI reviewed Jan 17, 2025

View reviewed changes

alexvy86 reviewed Jan 22, 2025

View reviewed changes

alexvy86 approved these changes Jan 22, 2025

View reviewed changes

Josmithr merged commit 89ade74 into microsoft:main Jan 22, 2025
55 checks passed

Josmithr deleted the api-markdown-documenter/update-defaults branch January 22, 2025 22:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

improvement(api-markdown-documenter): Update default suite configuration to leverage new hierarchy options #23602

improvement(api-markdown-documenter): Update default suite configuration to leverage new hierarchy options #23602

Josmithr commented Jan 17, 2025 •

edited by azure-boards bot

Loading

github-actions bot commented Jan 17, 2025

alexvy86 Jan 22, 2025

Josmithr Jan 22, 2025

alexvy86 Jan 22, 2025

Josmithr Jan 22, 2025

Josmithr Jan 22, 2025 •

edited

Loading

Josmithr Jan 22, 2025

Josmithr Jan 22, 2025

alexvy86 left a comment

alexvy86 Jan 22, 2025

Josmithr Jan 22, 2025 •

edited

Loading

improvement(api-markdown-documenter): Update default suite configuration to leverage new hierarchy options #23602

improvement(api-markdown-documenter): Update default suite configuration to leverage new hierarchy options #23602

Conversation

Josmithr commented Jan 17, 2025 • edited by azure-boards bot Loading

Choose a reason for hiding this comment

github-actions bot commented Jan 17, 2025

linkcheck output

alexvy86 Jan 22, 2025

Choose a reason for hiding this comment

Josmithr Jan 22, 2025

Choose a reason for hiding this comment

alexvy86 Jan 22, 2025

Choose a reason for hiding this comment

Josmithr Jan 22, 2025

Choose a reason for hiding this comment

Josmithr Jan 22, 2025 • edited Loading

Choose a reason for hiding this comment

Josmithr Jan 22, 2025

Choose a reason for hiding this comment

Josmithr Jan 22, 2025

Choose a reason for hiding this comment

alexvy86 left a comment

Choose a reason for hiding this comment

alexvy86 Jan 22, 2025

Choose a reason for hiding this comment

Josmithr Jan 22, 2025 • edited Loading

Choose a reason for hiding this comment

Josmithr commented Jan 17, 2025 •

edited by azure-boards bot

Loading

Josmithr Jan 22, 2025 •

edited

Loading

Josmithr Jan 22, 2025 •

edited

Loading