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

[api-extractor] Release tags aren't respected for const exports #936

Closed
nvh opened this issue Nov 7, 2018 · 4 comments
Closed

[api-extractor] Release tags aren't respected for const exports #936

nvh opened this issue Nov 7, 2018 · 4 comments
Labels
bug Something isn't working as intended effort: easy Probably a quick fix. Want to contribute? :-)

Comments

@nvh
Copy link

nvh commented Nov 7, 2018

Reproduction

Create a export const variableDeclaration and add a @beta tag to it

Expected

variableDeclaration won't show up in the public rollup .d.ts file

Observed

It does.

Remarks

There is an example of this in the test files in this repo:

The variable is defined here as @beta
https://github.com/Microsoft/web-build-tools/blob/master/build-tests/api-extractor-test-04/src/index.ts#L26

But shows up in the public rollup file here:
https://github.com/Microsoft/web-build-tools/blob/master/build-tests/api-extractor-test-04/dist/public/api-extractor-test-04.d.ts#L53

@nvh
Copy link
Author

nvh commented Nov 7, 2018

Some more investigation:

/**
 * @internal
 */
export const variableDeclaration1: string = "hello"
export /** @internal */ const variableDeclaration2: string = "hello"
export const /** @internal */ variableDeclaration3: string = "hello"

gets translated into

declare const variableDeclaration1: string;

declare const variableDeclaration2: string;

declare const /** @internal */ variableDeclaration3: string;

@octogonz octogonz added bug Something isn't working as intended help wanted If you're looking to contribute, this issue is a good place to start! effort: easy Probably a quick fix. Want to contribute? :-) labels Nov 7, 2018
@octogonz
Copy link
Collaborator

octogonz commented Nov 7, 2018

So as I mentioned in Gitter, the variable declaration syntax is tricky in general because you can write something like this:

export const a = 1, b = 2, [c, d] = [3, 4];

Where would you expect to find the doc comment for "b" or "c" in this expression?

Based on your findings above, I'm thinking it's pretty reasonable for API Extractor to ask that exported variable declarations must be separated into individual statements. So the above line would need to be rewritten in this equivalent form, if you want API Extractor to analyze it:

/** docs */
export const a = 1;
/** docs */
export const b = 2;
/** docs */
export const c = 3;
/** docs */
export const d = 4;

With that change, the .d.ts rollup fix should be pretty straightforward. In DtsRollupGenerator.ts we would need to check for ts.SyntaxKind.VariableDeclaration or whatever and find the right node to pass to getJSDocCommentRanges(). There is already something similar happening in _modifySpan().

@octogonz octogonz removed the help wanted If you're looking to contribute, this issue is a good place to start! label Nov 15, 2018
@octogonz
Copy link
Collaborator

octogonz commented Nov 15, 2018

@nvh I will fix this. I'm encountering the same issue in my API Extractor 7 PR branch, which now supports API reports for variable declarations. I'll see if I can also backport the fix to AE6.

@octogonz
Copy link
Collaborator

@nvh this should be fixed in API Extractor 6.1.5

octogonz pushed a commit that referenced this issue Nov 20, 2018
Delete deadwood files

Rename src -> src-old

Bringing over files

Bring over more files

Bring over more files

Bring over more files

Bring over more files

The dtsRollup generator now builds and works correctly

Initial skeleton of AstModel concept

Initial prototype of mixins approach

Fix serialization of AstMemberMixin

Add canonical selector and StaticMixin

Replace canonical selector with canonical reference, and support member overloading

Add ApiInterface and PropertySignature with declaration merging for interfaces

Rename "parameters" to "options" to avoid confusion with TypeScript function parameters

Implement parameter serialization

Deserialization is now working

Set up ApiDeclaration base class for tracking signatures

Rename StaticMixin.ts -> ApiStaticMixin.ts

Add signature

ApiParameter is now a proper ApiDeclaration

Convert ApiDeclaration to a mixin

upgrade tsdoc

rush update

Bring over AedocDefinitions.ts

Doc comments are now parsed and serialized into .api.json

ApiParameter returns docs using tryGetBlockByName

Upgrade tsdoc

rush update

Write a header at the top of the .api.json file

- Remove debug code
- Wire up the .api.json writer feature
- Remove the externalJsonFileFolders feature; if someone really needs this, we'll redesign it

Remove obsolete code from TypeScriptHelpers.ts

Update IndentedWriter to use StringBuilder

Add TSDocFlavor field

The `@packageDocumentation` comment is now being emitted again in the .d.ts rollups, sharing the same scanner as ModelBuilder

Bring over Markup.ts and MarkupElement.ts

Add tsdoc dependency for api-documenter

Update MarkdownRenderer.ts to build again

Expose ModelBuilder as public API

Temporary workaround for declarations that get emitted weirdly by the compiler

Add custom DocNode subclasses that will eliminate the need for MarkupElement

Make ApiModel and ApiPackage easier to traverse

Add ApiItem.getHierarchy()

Add ApiReleaseTagMixin

Add a test that repros the bug where EcmaScript symbols aren't rolled up properly

Fix the bug where EcmaScript symbols were not rolled up correctly

rush update

Add jest as a dependency so we can debug tests

MarkdownDocumenter.ts is now more or less working using TSDoc instead of Markup

Fix various files to use the updated TSDoc API

Add isBaseClassOf() helper for mixins

Export ReleaseTag

Add ApiMethodSignature and ApiProperty classes

Add ApiResultTypeMixin

Add ApiPropertyItem

Fix ApiProperty.kind

Set up new skeleton for MarkdownDocumenter.ts

Function parameters and return value are now documented

Generalize MarkdownRenderer to support link resolution

Implement a basic TSDoc declaration reference resolver

Rename MarkdownRenderer.ts -- MarkdownEmitter.ts

Move the test files

Rename MarkdownRenderer --> MarkdownEmitter

Convert MarkdownEmitter into a non-static class, and separate SimpleWriter into its own source file

Separate the non-TSDoc handling into a CustomMarkdownEmitter subclass

If the link text is omitted, automatically generate it based on the ApiItem name

Add ApiDeclarationMixin.getSignatureWithModifiers()

YamlDocumenter and OfficeYamlDocumenter finally build again

Move documenter classes under "documenters" folder

Move documenter classes under "documenters" folder

Minor fixes

Moving file

Updating imports

Move the core DtsRollupGenerator analysis down into ExtractorContext

Rename ModelBuilder to ApiModelGenerator

Rename ModelBuilder to ApiModelGenerator

Rename ExtractorContext --> Collector, and DtsEntry --> CollectorEntity

Rename ExtractorContext --> Collector, and DtsEntry --> CollectorEntity

Separated package state into CollectorPackage, refactored ApiModelGenerator to reuse the analysis from Collector

Bring over ReviewFileGenerator.ts

Release tag calculation is now centralized in Collector

Fix for GitHub issue #936 where doc comments were not being read for variable declarations

Fix some miscellaneous issues for YamlDocumenter.ts

Complete _writeAedocSynopsis()

Fix .d.ts rollup emitting of doc comments for packages and variable declarations

The API Review File now sorts nested members and injects _getAedocSynopsis() comments while preserving .d.ts indentation

Don't show redundant release tags in the API review file

Fix an issue where getSortKeyIgnoringUnderscore() was case-sensitive and

Fix an issue where DeclarationMetadata.needsDocumentation was false when the doc comment was completely missing

The test *.d.ts files are now built correctly

Disabled API Extractor for projects with incompatibilities:

"The symbol X was also exported as Y; this is not supported yet" (GitHub #950)

"Program Bug: The symbol GulpTask is being imported after it was already registered as non-imported" (GitHub #949)

Add `"tsdocFlavor": "AEDoc"` to projects that build with API Extractor

Add missing release tags for declarations that are now picked up by AE7

Don't report release tag errors for the default export

Fix table of contents generation

The MarkdownDocumenter and YamlDocumenter test output is now correct

Improve appearance of VariableDeclaration in .api.ts files

Delete left over files from src-old/

Add a test illustrating usage of the `@example` tag

MarkdownDocumenter now supports the `@example` tag from TSDoc

Move IndentedWriter.ts under src/api

Bring over IStringBuilder from TSDoc

Merge SimpleWriter and IndentedWriter into a single class

MarkdownDocumenter now writes `@deprecated` warnings
octogonz pushed a commit that referenced this issue Nov 22, 2018
Delete deadwood files

Rename src -> src-old

Bringing over files

Bring over more files

Bring over more files

Bring over more files

Bring over more files

The dtsRollup generator now builds and works correctly

Initial skeleton of AstModel concept

Initial prototype of mixins approach

Fix serialization of AstMemberMixin

Add canonical selector and StaticMixin

Replace canonical selector with canonical reference, and support member overloading

Add ApiInterface and PropertySignature with declaration merging for interfaces

Rename "parameters" to "options" to avoid confusion with TypeScript function parameters

Implement parameter serialization

Deserialization is now working

Set up ApiDeclaration base class for tracking signatures

Rename StaticMixin.ts -> ApiStaticMixin.ts

Add signature

ApiParameter is now a proper ApiDeclaration

Convert ApiDeclaration to a mixin

upgrade tsdoc

rush update

Bring over AedocDefinitions.ts

Doc comments are now parsed and serialized into .api.json

ApiParameter returns docs using tryGetBlockByName

Upgrade tsdoc

rush update

Write a header at the top of the .api.json file

- Remove debug code
- Wire up the .api.json writer feature
- Remove the externalJsonFileFolders feature; if someone really needs this, we'll redesign it

Remove obsolete code from TypeScriptHelpers.ts

Update IndentedWriter to use StringBuilder

Add TSDocFlavor field

The `@packageDocumentation` comment is now being emitted again in the .d.ts rollups, sharing the same scanner as ModelBuilder

Bring over Markup.ts and MarkupElement.ts

Add tsdoc dependency for api-documenter

Update MarkdownRenderer.ts to build again

Expose ModelBuilder as public API

Temporary workaround for declarations that get emitted weirdly by the compiler

Add custom DocNode subclasses that will eliminate the need for MarkupElement

Make ApiModel and ApiPackage easier to traverse

Add ApiItem.getHierarchy()

Add ApiReleaseTagMixin

Add a test that repros the bug where EcmaScript symbols aren't rolled up properly

Fix the bug where EcmaScript symbols were not rolled up correctly

rush update

Add jest as a dependency so we can debug tests

MarkdownDocumenter.ts is now more or less working using TSDoc instead of Markup

Fix various files to use the updated TSDoc API

Add isBaseClassOf() helper for mixins

Export ReleaseTag

Add ApiMethodSignature and ApiProperty classes

Add ApiResultTypeMixin

Add ApiPropertyItem

Fix ApiProperty.kind

Set up new skeleton for MarkdownDocumenter.ts

Function parameters and return value are now documented

Generalize MarkdownRenderer to support link resolution

Implement a basic TSDoc declaration reference resolver

Rename MarkdownRenderer.ts -- MarkdownEmitter.ts

Move the test files

Rename MarkdownRenderer --> MarkdownEmitter

Convert MarkdownEmitter into a non-static class, and separate SimpleWriter into its own source file

Separate the non-TSDoc handling into a CustomMarkdownEmitter subclass

If the link text is omitted, automatically generate it based on the ApiItem name

Add ApiDeclarationMixin.getSignatureWithModifiers()

YamlDocumenter and OfficeYamlDocumenter finally build again

Move documenter classes under "documenters" folder

Move documenter classes under "documenters" folder

Minor fixes

Moving file

Updating imports

Move the core DtsRollupGenerator analysis down into ExtractorContext

Rename ModelBuilder to ApiModelGenerator

Rename ModelBuilder to ApiModelGenerator

Rename ExtractorContext --> Collector, and DtsEntry --> CollectorEntity

Rename ExtractorContext --> Collector, and DtsEntry --> CollectorEntity

Separated package state into CollectorPackage, refactored ApiModelGenerator to reuse the analysis from Collector

Bring over ReviewFileGenerator.ts

Release tag calculation is now centralized in Collector

Fix for GitHub issue #936 where doc comments were not being read for variable declarations

Fix some miscellaneous issues for YamlDocumenter.ts

Complete _writeAedocSynopsis()

Fix .d.ts rollup emitting of doc comments for packages and variable declarations

The API Review File now sorts nested members and injects _getAedocSynopsis() comments while preserving .d.ts indentation

Don't show redundant release tags in the API review file

Fix an issue where getSortKeyIgnoringUnderscore() was case-sensitive and

Fix an issue where DeclarationMetadata.needsDocumentation was false when the doc comment was completely missing

The test *.d.ts files are now built correctly

Disabled API Extractor for projects with incompatibilities:

"The symbol X was also exported as Y; this is not supported yet" (GitHub #950)

"Program Bug: The symbol GulpTask is being imported after it was already registered as non-imported" (GitHub #949)

Add `"tsdocFlavor": "AEDoc"` to projects that build with API Extractor

Add missing release tags for declarations that are now picked up by AE7

Don't report release tag errors for the default export

Fix table of contents generation

The MarkdownDocumenter and YamlDocumenter test output is now correct

Improve appearance of VariableDeclaration in .api.ts files

Delete left over files from src-old/

Add a test illustrating usage of the `@example` tag

MarkdownDocumenter now supports the `@example` tag from TSDoc

Move IndentedWriter.ts under src/api

Bring over IStringBuilder from TSDoc

Merge SimpleWriter and IndentedWriter into a single class

MarkdownDocumenter now writes `@deprecated` warnings

rush update

Manually reapply the fix from PR #948 which got lost in the merge

Improve how Sort.compareByValue() handles "null" and "undefined", and add a unit test

Add test cases that illustrate Enums being handled incorrectly

Update .api.ts files

Fix the issue where Enums were sorted incorrectly by Span.

Add a test case for access modifiers

Add AstDeclaration.modifierFlags and trim private members from *.api.ts and *.api.json
javier-garcia-meteologica pushed a commit to javier-garcia-meteologica/rushstack that referenced this issue Jun 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working as intended effort: easy Probably a quick fix. Want to contribute? :-)
Projects
None yet
Development

No branches or pull requests

2 participants