-
Notifications
You must be signed in to change notification settings - Fork 653
[api-extractor] Apply @module comments to module namespace objects #5449
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
base: main
Are you sure you want to change the base?
Conversation
- Added @module tag definition to TSDoc configuration - Created ModuleDocComment helper to extract @module comments - Modified DtsRollupGenerator to apply @module comments to namespaces - Added logic to skip @module comments on regular declarations - Created test case moduleNamespaceComment to verify functionality Co-authored-by: iclanton <[email protected]>
| // If the @module comment seems to be attached to one of the regular API items, | ||
| // omit it. It gets emitted with the namespace declaration. | ||
| if (span.node.getText().match(/(?:\s|\*)@module(?:\s|\*)/gi)) { | ||
| span.modification.skipAll(); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot - This should be an error case, shouldn't it? The @module comment should only be valid for a module re-exported as a namespace, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently, @module comments on regular API items are silently skipped (similar to how @packageDocumentation is handled). The @module comment from the source file is only used when that module is re-exported as a namespace.
Should we report an error if a file contains @module but is NOT re-exported as a namespace? Or is silent skipping the right behavior? I'd like clarification on the intended validation rules.
- ModuleDocComment now accepts Collector parameter for error reporting - Detects and reports error for multiple @module tags in same file - Added MisplacedModuleTag error message ID - Maintains silent skip behavior for @module on individual declarations Co-authored-by: iclanton <[email protected]>
|
@copilot - this needs a changelog entry. Run |
- Created changelog entry for @module comment support - Removed unused ExtractorMessageId import from DtsRollupGenerator Co-authored-by: iclanton <[email protected]>
|
@copilot - Can you take a look at the build failures? |
I've checked the build locally and all builds and tests are passing successfully:
Could you provide more details about which specific build failures you're seeing? Are they:
This will help me identify and fix the issue. |
| import type { Collector } from '../collector/Collector'; | ||
| import { ExtractorMessageId } from '../api/ExtractorMessageId'; | ||
|
|
||
| export class ModuleDocComment { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DRY - this is essentially a copy+paste of the code+comments from PackageDocComment.ts, introducing a second full scan of the AST. And it does not conider a comment block containing both @module and @packageDocumentation, which probably should be an error. I feel like probably these tags should be detected in a single unified scan. But we need to generalize it a bit, because the @packageDocumentation code assumed that it can only appear in the entry point file, whereas @module can appear in any file.
|
|
||
| // Check if the source file has a @module comment and emit it before the namespace declaration | ||
| const sourceFile: ts.SourceFile = astEntity.astModule.sourceFile; | ||
| const moduleCommentRange: ts.TextRange | undefined = ModuleDocComment.tryFindInSourceFile( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The API Extractor architecture generally does not perform this kind of analysis in the generator layer. It should happen earlier (e.g. in the collector), and the comment needs to be parsed as TSDoc, not literally copy+pasted into the output without any analysis.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These comments also need to be emitted into the .api.json model file. If you implement the ApiModelGenerator part, then you will realize why parsing needs to happen upstream from the generators.
| "syntaxKind": "block" | ||
| }, | ||
| { | ||
| "tagName": "@module", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since @module is already defined by JSDoc, we should probably make it a standard TSDoc tag rather than an AEDoc custom tag. If the JSDoc syntax is inconsistent or has different semantics, then maybe TSDoc should consider a different name like @moduleDocumentation.
Summary
When API Extractor creates a namespace to represent an exported module namespace object (
export * as Foo from './Foo'), the@modulecomment from the source file is now applied to the namespace declaration.Fixes #5446
Details
This PR implements the following changes:
@moduletag definition totsdoc-base.jsonas a modifier tag@modulecomments from source files (mirrors thePackageDocCommentpattern)@moduleappears only once at the top of the file@moduletags in the same file@modulecomments from source files when processingAstNamespaceImport@modulecomments when they appear on regular API items to prevent duplicationMisplacedModuleTagerror ID for validation errorsThe implementation follows the same pattern used for
@packageDocumentationcomments.Example:
Source (Text.ts):
Input (index.ts):
Generated (rollup.d.ts):
How it was tested
moduleNamespaceCommentinbuild-tests/api-extractor-scenariosthat validates the functionality with the exact example from the issue@modulecomment on the namespace declaration without duplicating it on individual function declarations@moduletags in the same fileImpacted documentation
No documentation updates required. The
@moduletag is now supported as part of the TSDoc configuration for API Extractor.Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.