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): Update structure of ApiTransformationConfiguration and ApiItemTransformations #23381

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

Conversation

Josmithr
Copy link
Contributor

Updated the structure of ApiTransformationConfiguration to contain a transformations property of type ApiItemTransformations, rather than implementing that interface directly.

Also updates ApiItemTransformations methods to be keyed off of ApiItemKind, rather than being individually named.

E.g. A call like config.transformApiMethod(...) would become config.transformations["Method"](...).

This better aligns with similar transformational API surfaces in this library, like the renderers. It also makes it easier to ensure we capture all possible ApiItem kinds.

The createDefaultLayout property of ApiItemTransformations now lives directly in ApiTransformationConfiguration, but has been renamed to defaultSectionLayout.

@Josmithr Josmithr requested review from alexvy86, a team and Copilot December 20, 2024 01:05
@github-actions github-actions bot added public api change Changes to a public API base: main PRs targeted against main branch labels Dec 20, 2024

Choose a reason for hiding this comment

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

Copilot reviewed 5 out of 17 changed files in this pull request and generated no comments.

Files not reviewed (12)
  • tools/api-markdown-documenter/src/api-item-transforms/default-implementations/TransformApiInterface.ts: Evaluated as low risk
  • tools/api-markdown-documenter/src/api-item-transforms/configuration/Transformations.ts: Evaluated as low risk
  • tools/api-markdown-documenter/src/api-item-transforms/default-implementations/CreateSectionForApiItem.ts: Evaluated as low risk
  • tools/api-markdown-documenter/src/api-item-transforms/default-implementations/TransformApiClass.ts: Evaluated as low risk
  • tools/api-markdown-documenter/src/api-item-transforms/TransformApiModel.ts: Evaluated as low risk
  • tools/api-markdown-documenter/src/api-item-transforms/configuration/Configuration.ts: Evaluated as low risk
  • tools/api-markdown-documenter/src/api-item-transforms/default-implementations/TransformApiEnum.ts: Evaluated as low risk
  • tools/api-markdown-documenter/src/api-item-transforms/default-implementations/TransformApiFunctionLike.ts: Evaluated as low risk
  • tools/api-markdown-documenter/src/api-item-transforms/default-implementations/index.ts: Evaluated as low risk
  • tools/api-markdown-documenter/src/api-item-transforms/default-implementations/TransformApiItemWithoutChildren.ts: Evaluated as low risk
  • tools/api-markdown-documenter/src/api-item-transforms/test/Transformation.test.ts: Evaluated as low risk
  • tools/api-markdown-documenter/src/api-item-transforms/default-implementations/TransformApiModuleLike.ts: Evaluated as low risk
*
* @returns The list of {@link SectionNode}s that comprise the top-level section body for the API item.
*/
readonly defaultSectionLayout: (
Copy link
Contributor Author

@Josmithr Josmithr Dec 20, 2024

Choose a reason for hiding this comment

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

Extracted from ApiItemTransformations, since it really isn't a "transformation"; it's a utility used by transformations. It will also make it easier to convert ApiItemTransformations to a mapped type (over ApiItemKind) in the future.

/**
* {@inheritDoc ApiItemTransformationConfiguration.defaultSectionLayout}
*/
readonly defaultSectionLayout?: (
Copy link
Contributor Author

@Josmithr Josmithr Dec 20, 2024

Choose a reason for hiding this comment

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

I plan to deduplicate this with the duplicate signature above in a future change. For now, this is simpler given the mix of properties that are required in both "configuration" and "options", and the properties that are optional in "options".


logger.verbose(`Rendering section for ${apiItem.displayName}...`);
logger.verbose(`Generating documentation section for ${apiItem.displayName}...`);

let sections: SectionNode[];
switch (apiItem.kind) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note: the exhaustive switch statement is still required since each transformation is strongly typed over its ApiItem implementation. Hoping to simplify this in the future.

Copy link
Contributor

🔗 No broken links found! ✅

Your attention to detail is admirable.

linkcheck output


> fluid-framework-docs-site@0.0.0 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"


> fluid-framework-docs-site@0.0.0 serve
> docusaurus serve --no-open

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

> fluid-framework-docs-site@0.0.0 check-links
> linkcheck http://localhost:3000 --skip-file skipped-urls.txt

Crawling...

Stats:
  170138 links
    1596 destination URLs
    1825 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