Skip to content

refactor(api-markdown-documenter): Remove support for unsafe escaped text support from PlainTextNode #24786

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

Merged
merged 11 commits into from
Jun 10, 2025

Conversation

Josmithr
Copy link
Contributor

@Josmithr Josmithr commented Jun 8, 2025

This type previously supported an unsafe escape hatch for text escaping.
This support is no longer needed and has been removed.

@Josmithr Josmithr requested review from alexvy86, jumyhre, a team and Copilot June 8, 2025 22:02
@github-actions github-actions bot added public api change Changes to a public API base: main PRs targeted against main branch labels Jun 8, 2025
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Refactors how escaped text is handled by extracting it into its own EscapedTextNode type, removes the escaped flag from PlainTextNode, and updates renderers, transformations, and tests accordingly.

  • Introduces EscapedTextNode class and related mapping in domain index
  • Updates markdown and HTML renderers/transformers to handle escaped text separately
  • Removes escaped parameter from PlainTextNode and adjusts tests

Reviewed Changes

Copilot reviewed 20 out of 20 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
tools/api-markdown-documenter/src/renderers/markdown-renderer/test/RenderDocument.test.ts Adds test for escaped markdown rendering
tools/api-markdown-documenter/src/renderers/markdown-renderer/default-renderers/RenderPlainText.ts Removes node.escaped branch and refactors whitespace splitter export
tools/api-markdown-documenter/src/renderers/markdown-renderer/default-renderers/RenderEscapedText.ts Implements renderer for EscapedTextNode
tools/api-markdown-documenter/src/renderers/markdown-renderer/default-renderers/index.ts Exports and registers renderEscapedText
tools/api-markdown-documenter/src/documentation-domain/PlainTextNode.ts Drops escaped flag from PlainTextNode constructor
tools/api-markdown-documenter/src/documentation-domain/EscapedTextNode.ts Adds new EscapedTextNode class
tools/api-markdown-documenter/src/documentation-domain-to-html/default-transformations/EscapedTextToHtml.ts Implements HTML transformer for EscapedTextNode
tools/api-markdown-documenter/src/api-item-transforms/TsdocNodeTransforms.ts Emits EscapedTextNode from TSDoc escapes
tools/api-markdown-documenter/api-report/**/* Updates public API to include EscapedTextNode and remove escaped param
Comments suppressed due to low confidence (4)

tools/api-markdown-documenter/src/renderers/markdown-renderer/default-renderers/index.ts:85

  • The renderer key is a raw property escapedText. For consistency with other renderers, use [DocumentationNodeType.EscapedText] as the key so it maps correctly to the node type.
escapedText: (node, writer, context): void =>

tools/api-markdown-documenter/src/documentation-domain-to-html/default-transformations/EscapedTextToHtml.ts:18

  • The doc comment refers to PlainTextNode but this function handles EscapedTextNode. Update the reference to {@link EscapedTextNode}.
* Transform a {@link PlainTextNode} to HTML.

tools/api-markdown-documenter/src/renderers/markdown-renderer/test/RenderDocument.test.ts:64

  • The test references SectionNode but it is not imported at the top. Add SectionNode to the import list to prevent missing-symbol errors.
new SectionNode([

tools/api-markdown-documenter/src/documentation-domain-to-html/configuration/Transformation.ts:94

  • The transformation key uses a raw property escapedText. To ensure the escaped node is picked up, use [DocumentationNodeType.EscapedText] instead of an unscoped property.
escapedText: (node, context) => escapedTextToHtml(node as EscapedTextNode, context),

@Josmithr Josmithr marked this pull request as draft June 10, 2025 00:30
@Josmithr Josmithr changed the title refactor(api-markdown-documenter): Extract escaped text representation into its own type refactor(api-markdown-documenter): Remove support for unsafe escaped text support from PlainTextNode Jun 10, 2025
@Josmithr Josmithr marked this pull request as ready for review June 10, 2025 00:39
@@ -78,7 +78,6 @@
"chalk": "^4.1.2",
"hast-util-format": "^1.1.0",
"hast-util-from-html": "^2.0.3",
"hast-util-raw": "^9.0.2",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

No longer needed!

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:
  222981 links
    1707 destination URLs
    1939 URLs ignored
       0 warnings
       0 errors


@Josmithr Josmithr merged commit 9c49415 into microsoft:main Jun 10, 2025
33 checks passed
@Josmithr Josmithr deleted the api-markdown-documenter/EscapedText branch June 10, 2025 15:30
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.

2 participants