Skip to content

Commit 58e34d9

Browse files
authored
fix(api-markdown-documenter): Fixed an issue where MarkdownRenderer would emit trailing \ characters when processing certain summary comments. (#25933)
The issue here is somewhat complicated. The following is an attempt to provide a high-level-ish overview of what's going on. ## The Problem Keeping the description at a high-level, `CommonMark` specifies 2 ways to force an explicit line break: either ending a line with 2 spaces or ending a line with a backslash. - https://commonmark.org/help/tutorial/03-paragraphs.html `mdast-util-to-markdown`, the library we use to emit prefers the latter, as it is more explicit. In most cases, this is irrelevant, and Markdown parsers that adhere to `CommonMark` should tolerate this. Docusaurus doesn't appear to, causing spurious trailing backslash characters to appear in *some* of our API documentation. E.g. https://fluidframework.com/docs/api/fluid-framework/allowedtypesfullevaluated-typealias But our library shouldn't need to output forced line breaks. We build our Markdown AST trees manually, and so we control the output structure. We generally avoid explicit line breaks, and instead use `Paragraph` nodes when separating blocks of text. But for contents parsed by `TSDoc`, we _transform_ their parsed content, making certain assumptions about how the parser behaves. The case that was causing problems for us is a case where `TSDoc` fails to trim trailing whitespace from comment blocks (generally this manifests in the summary component). In particular, when a summary comment is followed by 2 or more modifier tags on a subsequent line, TSDoc fails to trim the summary comment, yielding something with trailing explicit line breaks. ### Example Given the following comment: ```typescript /** * I am a comment * */ ``` The TSDoc parser will yield a summary block of `I am a comment` But given the following comment: ```typescript /** * I am a comment * * @Sealed @public */ ``` The TSDoc parser will yield a summary block of `I am a comment \n` ## The Solution The solution is to better trim paragraph contents given to us from the `TSDoc` parser. Previously, we would trim any leading/trailing line breaks we encountered. Now, we also trim and leading/trailing whitespace content. This simplifies the Paragraph trees we hand off to `mdast-util-to-markdown`, removing the need for them to inject trailing `\` characters to force line breaks. [AB#53737](https://dev.azure.com/fluidframework/235294da-091d-4c29-84fc-cdfc3d90890b/_workitems/edit/53737)
1 parent 7bc8741 commit 58e34d9

File tree

10 files changed

+75
-27
lines changed

10 files changed

+75
-27
lines changed

tools/api-markdown-documenter/CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,11 @@
11
# @fluid-tools/api-markdown-documenter
22

3+
## 0.23.2
4+
5+
### 🐞 Bug Fixes
6+
7+
- Fixed an issue where `MarkdownRenderer` would emit trailing `\` characters when processing certain summary comments.
8+
39
## 0.23.1
410

511
### 🐞 Bug Fixes

tools/api-markdown-documenter/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@fluid-tools/api-markdown-documenter",
3-
"version": "0.23.1",
3+
"version": "0.23.2",
44
"description": "Processes .api.json files generated by API-Extractor and generates Markdown documentation from them.",
55
"homepage": "https://fluidframework.com",
66
"repository": {

tools/api-markdown-documenter/src/api-item-transforms/TsdocNodeTransforms.ts

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@ function transformTsdocSectionContent(
157157
* 2. Remove line break nodes adjacent to paragraph nodes.
158158
*
159159
* 3. Remove leading and trailing line breaks within the paragraph (see
160-
* {@link trimLeadingAndTrailingLineBreaks}).
160+
* {@link trimParagraphContents}).
161161
*
162162
* 4. Trim leading whitespace from first child if it is plain-text, and trim trailing whitespace from
163163
* last child if it is plain-text.
@@ -170,8 +170,8 @@ function transformTsdocParagraph(
170170
// To ensure we map the content correctly, we should scan the child list for matching open/close tags,
171171
// and map the subsequence to an "html" node.
172172

173-
// Trim leading and trailing line breaks, which are redundant in the context of a paragraph.
174-
const adjustedChildren = trimLeadingAndTrailingLineBreaks(node.nodes);
173+
// Trim leading and trailing whitespace and line breaks, which are redundant in the context of a paragraph.
174+
const adjustedChildren = trimParagraphContents(node.nodes);
175175

176176
// Transform child items into Documentation domain
177177
const transformedChildren: PhrasingContent[] = [];
@@ -698,29 +698,37 @@ function combineAdjacentPlainText(nodes: readonly PhrasingContent[]): PhrasingCo
698698
}
699699

700700
/**
701-
* Trims an line break nodes found at the beginning or end of the list.
702-
*
703-
* @remarks Useful for cleaning up {@link ParagraphNode} child contents, since leading and trailing
704-
* newlines are effectively redundant.
701+
* Trims any whitespace and line break nodes found at the beginning or end of the list of paragraph contents.
702+
* In the context of a Paragraph, such content is redundant.
705703
*/
706-
function trimLeadingAndTrailingLineBreaks(nodes: readonly DocNode[]): DocNode[] {
704+
function trimParagraphContents(nodes: readonly DocNode[]): DocNode[] {
707705
if (nodes.length === 0) {
708706
return [];
709707
}
710708

711709
let startIndex = 0;
712710
let endIndex = nodes.length - 1;
713711

714-
for (const node of nodes) {
712+
function canTrimNode(node: DocNode): boolean {
715713
if (node.kind === DocNodeKind.SoftBreak) {
714+
return true;
715+
}
716+
if (node.kind === DocNodeKind.PlainText) {
717+
return (node as DocPlainText).text.trim().length === 0;
718+
}
719+
return false;
720+
}
721+
722+
for (const node of nodes) {
723+
if (canTrimNode(node)) {
716724
startIndex++;
717725
} else {
718726
break;
719727
}
720728
}
721729

722730
for (let i = nodes.length - 1; i > startIndex; i--) {
723-
if (nodes[i].kind === DocNodeKind.SoftBreak) {
731+
if (canTrimNode(nodes[i])) {
724732
endIndex--;
725733
} else {
726734
break;

tools/api-markdown-documenter/src/api-item-transforms/test/TsdocNodeTransforms.test.ts

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -870,6 +870,37 @@ describe("Tsdoc node transformation tests", () => {
870870
},
871871
]);
872872
});
873+
874+
describe("Regression tests", () => {
875+
// TSDoc currently does something funny when parsing comments like the following case.
876+
// If the summary comment is followed by a line with multiple modifier tags, TSDoc doesn't seem to trim trailing whitespace and newlines from the summary.
877+
// This results in us generating paragraph content with trailing lines and whitespace that aren't desired.
878+
// This test exists to ensure we handle this case correctly and don't emit extra whitespace.
879+
it("Summary content parsed with trailing line break", () => {
880+
// Note
881+
const comment = `/**
882+
* I am a regression test.
883+
*
884+
* @sealed @beta
885+
*/`;
886+
const context = parser.parseString(comment);
887+
const summarySection = context.docComment.summarySection;
888+
889+
const result = transformTsdocSection(summarySection, transformOptions);
890+
891+
expect(result).to.deep.equal([
892+
{
893+
type: "paragraph",
894+
children: [
895+
{
896+
type: "text",
897+
value: "I am a regression test.",
898+
},
899+
],
900+
},
901+
]);
902+
});
903+
});
873904
});
874905

875906
// Test TODOs:

tools/api-markdown-documenter/src/test/snapshots/markdown/simple-suite-test/deep-config/test-suite-a/index.md

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -88,8 +88,8 @@ const foo = bar;
8888

8989
## Namespaces
9090

91-
| Namespace | Alerts | Description |
92-
| - | - | - |
93-
| [TestBetaNamespace](/test-suite-a/testbetanamespace-namespace/) | `Beta` | A namespace tagged as `@beta`. |
94-
| [TestModule](/test-suite-a/testmodule-namespace/) | | |
95-
| [TestNamespace](/test-suite-a/testnamespace-namespace/) | | Test Namespace |
91+
| Namespace | Alerts | Modifiers | Description |
92+
| - | - | - | - |
93+
| [TestBetaNamespace](/test-suite-a/testbetanamespace-namespace/) | `Beta` | `sealed` | A namespace tagged as `@beta`. |
94+
| [TestModule](/test-suite-a/testmodule-namespace/) | | | |
95+
| [TestNamespace](/test-suite-a/testnamespace-namespace/) | | | Test Namespace |

tools/api-markdown-documenter/src/test/snapshots/markdown/simple-suite-test/deep-config/test-suite-a/testbetanamespace-namespace/index.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ A namespace tagged as `@beta`.
99
<h2 id="testbetanamespace-signature">Signature</h2>
1010

1111
```typescript
12+
/** @sealed */
1213
export declare namespace TestBetaNamespace
1314
```
1415

tools/api-markdown-documenter/src/test/snapshots/markdown/simple-suite-test/default-config/test-suite-a/index.md

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -88,11 +88,11 @@ const foo = bar;
8888

8989
## Namespaces
9090

91-
| Namespace | Alerts | Description |
92-
| - | - | - |
93-
| [TestBetaNamespace](/test-suite-a/testbetanamespace-namespace/) | `Beta` | A namespace tagged as `@beta`. |
94-
| [TestModule](/test-suite-a/testmodule-namespace/) | | |
95-
| [TestNamespace](/test-suite-a/testnamespace-namespace/) | | Test Namespace |
91+
| Namespace | Alerts | Modifiers | Description |
92+
| - | - | - | - |
93+
| [TestBetaNamespace](/test-suite-a/testbetanamespace-namespace/) | `Beta` | `sealed` | A namespace tagged as `@beta`. |
94+
| [TestModule](/test-suite-a/testmodule-namespace/) | | | |
95+
| [TestNamespace](/test-suite-a/testnamespace-namespace/) | | | Test Namespace |
9696

9797
## Function Details
9898

tools/api-markdown-documenter/src/test/snapshots/markdown/simple-suite-test/default-config/test-suite-a/testbetanamespace-namespace/index.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ A namespace tagged as `@beta`.
99
<h2 id="testbetanamespace-signature">Signature</h2>
1010

1111
```typescript
12+
/** @sealed */
1213
export declare namespace TestBetaNamespace
1314
```
1415

tools/api-markdown-documenter/src/test/snapshots/markdown/simple-suite-test/flat-config/test-suite-a.md

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -85,11 +85,11 @@ const foo = bar;
8585

8686
# Namespaces
8787

88-
| Namespace | Alerts | Description |
89-
| - | - | - |
90-
| [TestBetaNamespace](docs/test-suite-a#testbetanamespace-namespace) | `Beta` | A namespace tagged as `@beta`. |
91-
| [TestModule](docs/test-suite-a#testmodule-namespace) | | |
92-
| [TestNamespace](docs/test-suite-a#testnamespace-namespace) | | Test Namespace |
88+
| Namespace | Alerts | Modifiers | Description |
89+
| - | - | - | - |
90+
| [TestBetaNamespace](docs/test-suite-a#testbetanamespace-namespace) | `Beta` | `sealed` | A namespace tagged as `@beta`. |
91+
| [TestModule](docs/test-suite-a#testmodule-namespace) | | | |
92+
| [TestNamespace](docs/test-suite-a#testnamespace-namespace) | | | Test Namespace |
9393

9494
# Interface Details
9595

@@ -1181,6 +1181,7 @@ A namespace tagged as `@beta`.
11811181
<h3 id="testbetanamespace-signature">Signature</h3>
11821182

11831183
```typescript
1184+
/** @sealed */
11841185
export declare namespace TestBetaNamespace
11851186
```
11861187

tools/api-markdown-documenter/src/test/test-data/simple-suite-test/test-suite-a.api.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -569,7 +569,7 @@
569569
{
570570
"kind": "Namespace",
571571
"canonicalReference": "test-suite-a!TestBetaNamespace:namespace",
572-
"docComment": "/**\n * A namespace tagged as `@beta`.\n *\n * @remarks\n *\n * Tests release level inheritance.\n *\n * @beta\n */\n",
572+
"docComment": "/**\n * A namespace tagged as `@beta`.\n *\n * @remarks\n *\n * Tests release level inheritance.\n *\n * @beta @sealed\n */\n",
573573
"excerptTokens": [
574574
{
575575
"kind": "Content",

0 commit comments

Comments
 (0)