fix: Preserve @example titles - #476
Conversation
|
Pete Gonzalez (@octogonz) I took a stab at resolving microsoft/rushstack#4860. Would love your feedback when you have a moment. |
@example titles@example titles
| @@ -0,0 +1,10 @@ | |||
| { | |||
There was a problem hiding this comment.
Four exhibits, and what I'm thinking each one should do.
Exhibit A: a title above a code sample.
/**
* @example Adding two numbers
* ```ts
* add(1, 2);
* ```
*/Title is Adding two numbers. A documentation tool should typeset it as the heading for this example, in place of the generic Example 2 numbering. This is a clarification, not a change: tsdoc.org already says text on the tag line is the title. Nothing parses differently, we are just exposing what is already there.
Exhibit B: the whole example is on the tag line.
/**
* The CPU architecture.
* @example `"AMD64"`
*/Read literally, the spec says `"AMD64"` is a title for an example with no body. That is clearly not what the author meant, and this idiom appears about a dozen times in rushstack alone (rush-lib/src/logic/Telemetry.ts, node-core-library/src/JsonSchema.ts, lockfile-explorer-web). Our call: parse it identically to Exhibit A, so no author has to change anything, and let the renderer notice the body is empty and render the line inline rather than as a heading. This is the one place we are adding to the spec, and we are adding a rendering convention rather than a parsing rule.
Exhibit C: markup in the title.
/**
* @example Using {@link add} on negative numbers
* ...
*/The markup is markup, not literal text. {@link add} is a real DocLinkTag that API Extractor resolves and validates, and a code span in the same position is a DocCodeSpan. That is how it parses today and it must keep working. The spec is silent on the point, so we are filling a gap rather than changing anything.
This is the reason the title has to be a list of nodes rather than a string. A string title can only hold the literal characters Using {@link add} on negative numbers, which demotes the link to text, drops it out of API Extractor's reference validation, and leaves every renderer to either print the braces or re-parse the string itself. Same argument for a backslash escape on the title line.
Exhibit D: a modifier tag on the tag line.
/**
* @example Adding two numbers @internal
* ...
*/@internal stays a modifier tag, exactly as it does today. The title is Adding two numbers and stops there, because the block itself ends at the second tag. This is not a new rule, it is a guarantee that the title feature does not quietly repeal an existing one. API Extractor derives every release tag from modifierTagSet, so getting this wrong changes whether an API is trimmed from a release build.
Deferred to a future revision. It would be nice to have a restricted grammar for titles, with a diagnostic when the title line contains something that cannot be a title. It would be nice to normalize or deprecate Exhibit B rather than living with a rendering convention. It would be nice to settle @throws, whose own page describes a next-line convention while the tag kinds page claims it behaves like @example. And it would be nice to have a general mechanism, such as a flag on TSDocTagDefinition, so that first-line-is-a-title is declared per tag rather than hardcoded in NodeParser. None of these should hold up this PR. All of them need a deprecation cycle and a spec pass that this change cannot carry.
There was a problem hiding this comment.
Joshua Smithrud (@Josmithr) what do you think? Do you agree with these calls? Will they cause any migration trouble for your codebase?
There was a problem hiding this comment.
Yes, all of those make sense to me. I'd noted the markup limitation in the PR description and suspected it might be something we wanted to tackle here. The other cases are not ones I considered, and your reasoning makes sense to me. I'll go ahead and make these updates (and clarify details in the documentation).
There was a problem hiding this comment.
Updated.
| @@ -0,0 +1,10 @@ | |||
| { | |||
| "changes": [ | |||
There was a problem hiding this comment.
The bug being fixed, restated
Parse a comment containing @example Adding two numbers and emit it again, and the title moves off the tag line with a blank line inserted after it.
Input:
/**
* @example Adding two numbers
* ```ts
* add(1, 2);
* ```
*/Emitted output:
/**
* @example
*
* Adding two numbers
* ```ts
* add(1, 2);
* ```
*
*/That is an emitter layout bug, not a parser bug. The text was never lost, it just came back in the wrong place. The fix for it is much smaller than the fix for "expose the title on the AST", and the two should not be entangled.
TSDocEmitter already handles this shape for @returns and @defaultValue at the DocNodeKind.Block case: write a space after the tag, set _hangingParagraph = true, and the following paragraph attaches to the tag line. Extending that to @example round-trips all four shapes byte for byte: title plus fence, title plus blank line plus prose, title plus prose on the next line, and title with no body.
One caveat if you do this. It needs a guard for the untitled case. Naively adding @example to that tag name list also changes untitled examples from @example / blank line / text to @example / text, with a trailing space, which churns every committed api.json containing an untitled @example and inherits a pre-existing wart from @returns. Condition the space and the hanging paragraph on the block actually having tag-line text.
Compatibility: keep the title in content, expose it as a view
This is the main change I want to ask for.
Problem. The PR moves the title out of DocBlock.content and into a sibling field. For Exhibit A, content goes from
Section
Paragraph
PlainText " Adding two numbers"
SoftBreak
FencedCode
to
Section
Paragraph
SoftBreak
FencedCode
Every existing consumer renders content. Both of API Documenter's documenters do exactly that. So on release day, titles stop appearing in generated docs for every project that uses them, and every other downstream tool has the same problem until it is updated. The AST gains a property and the output loses text.
Solution. Leave content alone. Add derived views over it:
title: DocParagraph | undefinedis the first paragraph's nodes up to its firstSoftBreak, space-trimmed. It isundefinedwhen that paragraph begins with aSoftBreak, which is exactly the untitled case.body: DocSectionis everything else, with the remainder of the first paragraph re-wrapped in a synthesizedDocParagraph.
Exclude both from onGetChildNodes(), which keeps returning [blockTag, content]. Traversal, excerpt reconstruction, and emission stay untouched. DocParagraph's constructor already takes a childNodes array and DocNode has no parent pointers, so the views share nodes safely.
Put them on DocBlock, not on a new node type. The split is structural, so it does not need DocExampleBlock or DocNodeKind.ExampleBlock. Skipping the new kind avoids the two ways it breaks consumers: anyone with a default: throw on a node-kind switch now hits it, and anyone who already registered a custom node kind named ExampleBlock now throws at registerDocNodes. It also gives @throws and custom block tags the same accessors for free, so we are not setting up a DocThrowsBlock next quarter, and it keeps NodeParser free of per-tag branching.
Net effect: no parser change at all, one guarded emitter change, two accessors on DocBlock. That is a much smaller cut than the current PR and it delivers the same capability.
API Documenter consequences
The title is not useful until something renders it. Please plan a companion rushstack PR.
Two call sites to update:
MarkdownDocumenter._writeRemarksSectionemitsExampleorExample 1/Example 2headings and then rendersexampleBlock.content. The numbered heading is precisely the spec's stated fallback for an untitled example, so this is wheretitlebelongs.YamlDocumenterdoes the same intoyamlItem.exampleand wants the same treatment. Note itsif (example)guard drops empty entries, so Exhibit B under the PR's current design would vanish from YAML output entirely.
Build tests that need attention:
build-tests/api-documenter-test/src/DocClass1.tsis the only fixture using@exampleand both instances are untitled. Add a titled one and an Exhibit B one. This will move thedocCommentstring inbuild-tests/api-documenter-test/etc/api-documenter-test.api.json, which is emitter output, so that snapshot needs regenerating and the diff is worth reading closely.build-tests/api-documenter-scenariosis the only place that commits generated Markdown (etc/inheritedMembers/markdown/*.md). It has no@examplecoverage at all. A new scenario there is the only build test that would actually catch a rendering regression, sinceapi-documenter-testcommits onlyapi.jsonandapi.md.- API Extractor itself needs no change. It filters blocks by tag name rather than node kind,
DocCommentEnhancer._copyInheritedDocsdoes not touch custom blocks, and theapi.jsondoc comment round-trip is text based. Worth confirming theapi-extractor-scenariossnapshots come back clean.
@microsoft/tsdoc is pinned at ~0.16.0 in api-extractor, api-documenter, and api-extractor-model, so the bump moves all three together.
There was a problem hiding this comment.
This PR has been updated. The follow-up fix in API-Documenter should be straightforward, and I am happy to tackle that.
Summary
Per TSDoc's documented spec, text on the same line as an
@exampletag is the title for the example.Round-tripping such a block (parse → emit) previously moved the title off the tag line, inserting a blank line after it — an emitter layout bug (the title text was never lost, just re-emitted in the wrong place).
This PR fixes the emission so the title is preserved on the tag line, and exposes the text that appears on a block's tag line via two new derived accessors on
DocBlock.(incorrect line formatting)
(preserves necessary line formatting)
Changes
Emitter fix (the actual bug): in the
DocNodeKind.Blockcase,@examplenow receives the same hanging-paragraph treatment as@returns/@defaultValue, so its tag-line text is re-emitted on the tag line. This is guarded on the tag line actually having content, so an untitled@exampleis not emitted with a trailing space.New derived accessors on
DocBlock:tagLineContent: DocParagraph | undefined— the rich text on the same line as the block tag (leading nodes of the first paragraph up to the first line break, trimmed), orundefinedwhen the tag line has no non-whitespace content.bodyContent: DocSection— the block'scontentexcluding itstagLineContent.Both are views over
contentthat share the underlying nodes and are excluded fromonGetChildNodes(), so traversal, excerpts, and emission are untouched. They live onDocBlock, so any block tag can use them;@exampleinterprets itstagLineContentas the example's title (a tool may fall back to numeric indexing when it isundefined).Inline markup is preserved. Because the title stays in
content, markup such as{@link}tags and code spans on the tag line remains a realDocLinkTag/DocCodeSpan(resolved and validated as usual) rather than being flattened to literal text.Notes
DocBlock.content, so existing consumers (e.g. API Documenter) keep rendering it unchanged. Avoiding a newDocNodeKindalso avoids breakingdefault:node-kind switches and custom node registrations.tagLineContent(and emits with no trailing space).tagLineContenttherefore extends across those lines up to where the tag closes.