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

refactor(api-markdown-documenter): Allow deeper customization of output folder structure #23366

Open
wants to merge 67 commits into
base: main
Choose a base branch
from

Conversation

Josmithr
Copy link
Contributor

@Josmithr Josmithr commented Dec 18, 2024

Previously, users could control certain aspects of the output documentation suite's file-system hierarchy via the documentBoundaries and hierarchyBoundaries properties of the transformation configuration.
One particular limitation of this setup was that items yielding folder-wise hierarchy (hierarchyBoundaries) could never place their own document inside of their own hierarchy.
This naturally lent itself to a pattern where output would commonly be formatted as:

- foo.md
- foo
    - bar.md
    - baz.md

This pattern works fine for many site generation systems - a link to /foo will end up pointing foo.md and a link to /foo/bar will end up pointing to foo/bar.md.
But some systems (e.g. Docusaurus) don't handle this well, and instead prefer setups like the following:

- foo
    - index.md
    - bar.md
    - baz.md

With the previous configuration options, this pattern was not possible, but now is.
Additionally, this pattern is more commonly accepted, so lack of support for this was a real detriment.

Such patterns can now be produced via the consolidated hierarchy property, while still allowing full file-naming flexibility.

Notes for reviewers

I would recommend starting with Hierarchy.ts - it contains the new configuration options related to hierarchy. The rest of the PR is predominantly respecting that new configuration setup.

To keep things relatively simple, system defaults and test configurations have been intentionally made to preserve existing system default behaviors. As a result, you'll notice that none of the end-to-end tests have updated collateral. However, the intention is to update default behaviors, and to add more end-to-end test configurations. For now, I have left a handful of TODOs in configuration defaults and test configurations - I will address those in a follow-up PR.

@github-actions github-actions bot added public api change Changes to a public API base: main PRs targeted against main branch labels Dec 18, 2024
@Josmithr Josmithr force-pushed the api-markdown-documenter/hierarchy-config branch from a9d189d to 3071431 Compare December 18, 2024 23:35
[ApiItemKind.Variable]: HierarchyKind.Document,
};

// TODO
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Future PR

@@ -6,7 +6,7 @@
import type { Logger } from "./Logging.js";

/**
* Common base interface for configuration interfaces.
* Common base interface for configurations that take a logger.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should have gone in with the previous PR that renamed this type.

testConfigs,
],

// TODO
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Future PR

testConfigs,
],

// TODO
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Future PR

@@ -393,23 +391,32 @@ export function createBreadcrumbParagraph(
apiItem: ApiItem,
config: ApiItemTransformationConfiguration,
): ParagraphNode {
// Get ordered ancestry of document items
const ancestry = getAncestralHierarchy(apiItem, (hierarchyItem) =>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This helper was a bit too general and only used in 2 places. I removed it and inlined the logic in the appropriate places.

@Josmithr Josmithr marked this pull request as ready for review January 8, 2025 22:41
@Josmithr Josmithr requested review from alexvy86 and a team January 8, 2025 22:41
Copy link
Contributor

github-actions bot commented Jan 8, 2025

🔗 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:
  170492 links
    1603 destination URLs
    1838 URLs ignored
       0 warnings
       0 errors


Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
base: main PRs targeted against main branch public api change Changes to a public API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant