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] Fix an issue where .d.ts trimming did not work for exported variable declaration #945

Merged
merged 4 commits into from
Nov 16, 2018
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -486,9 +486,33 @@ export class DtsRollupGenerator {
// In the near future we will overhaul the AEDoc parser to separate syntactic/semantic analysis,
// at which point this will be wired up to the same ApiDocumentation layer used for the API Review files
private _getReleaseTagForDeclaration(declaration: ts.Node): ReleaseTag {
let nodeForComment: ts.Node = declaration;

if (ts.isVariableDeclaration(declaration)) {
// Variable declarations are special because they can be combined into a list. For example:
//
// /** A */ export /** B */ const /** C */ x = 1, /** D **/ [ /** E */ y, z] = [3, 4];
//
// The compiler will only emit comments A and C in the .d.ts file, so in general there isn't a well-defined
// way to document these parts. API Extractor requires you to break them into separate exports like this:
//
// /** A */ export const x = 1;
//
// But _getReleaseTagForDeclaration() still receives a node corresponding to "x", so we need to walk upwards
// and find the containing statement in order for getJSDocCommentRanges() to read the comment that we expect.
const statement: ts.VariableStatement | undefined = TypeScriptHelpers.findFirstParent(declaration,
ts.SyntaxKind.VariableStatement) as ts.VariableStatement | undefined;
if (statement !== undefined) {
// For a compound declaration, fall back to looking for C instead of A
if (statement.declarationList.declarations.length === 1) {
nodeForComment = statement;
}
}
}

const sourceFileText: string = declaration.getSourceFile().text;

for (const commentRange of TypeScriptHelpers.getJSDocCommentRanges(declaration, sourceFileText) || []) {
for (const commentRange of TypeScriptHelpers.getJSDocCommentRanges(nodeForComment, sourceFileText) || []) {
// NOTE: This string includes "/**"
const commentTextRange: tsdoc.TextRange = tsdoc.TextRange.fromStringRange(
sourceFileText, commentRange.pos, commentRange.end);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,8 @@ export declare class TypeReferencesInAedoc {

declare const unexportedCustomSymbol: unique symbol;

/* Excluded from this release type: VARIABLE */

/**
* Example decorator
* @public
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,8 @@ export declare class TypeReferencesInAedoc {

declare const unexportedCustomSymbol: unique symbol;

export declare const VARIABLE: string;
Copy link
Member

@iclanton iclanton Nov 15, 2018

Choose a reason for hiding this comment

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

Should this have a comment? #WontFix

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good point.

I have fixed this issue in my AE7 branch, but I won't backport the fix to AE6, because in this branch the _getReleaseTagForDeclaration() logic doesn't save the comment that it found.

AE7 will be out soon.


In reply to: 234018505 [](ancestors = 234018505)


/**
* Example decorator
* @public
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,8 @@ export declare class TypeReferencesInAedoc {

declare const unexportedCustomSymbol: unique symbol;

/* Excluded from this release type: VARIABLE */
Copy link
Member

@iclanton iclanton Nov 15, 2018

Choose a reason for hiding this comment

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

Is this right? Shouldn't there be a variable here? #Resolved

Copy link
Collaborator Author

@octogonz octogonz Nov 15, 2018

Choose a reason for hiding this comment

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

It is marked as /** @alpha */ whereas you're looking at the dist/public output #Resolved


/**
* Example decorator
* @public
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,3 +119,4 @@ class TypeReferencesInAedoc {
export function virtual(target: Object, propertyKey: string | symbol, descriptor: TypedPropertyDescriptor<any>): void;

// WARNING: Unsupported export: fullyExportedCustomSymbol
// WARNING: Unsupported export: VARIABLE
2 changes: 2 additions & 0 deletions build-tests/api-extractor-test-01/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,3 +107,5 @@ export { default as IInterfaceAsDefaultExport } from './IInterfaceAsDefaultExpor
export { ReexportedClass3 as ReexportedClass } from './ReexportedClass3/ReexportedClass3';

export { TypeReferencesInAedoc } from './TypeReferencesInAedoc';

export { VARIABLE } from './variableDeclarations';
5 changes: 5 additions & 0 deletions build-tests/api-extractor-test-01/src/variableDeclarations.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license.
// See LICENSE in the project root for license information.

/** @alpha */
export const VARIABLE: string = 'hello';
Original file line number Diff line number Diff line change
Expand Up @@ -50,4 +50,4 @@ export declare class PublicClass {

/* Excluded from this release type: RegularEnum */

export declare const variableDeclaration: string;
/* Excluded from this release type: variableDeclaration */
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"changes": [
{
"packageName": "@microsoft/api-extractor",
"comment": "Fix an issue where .d.ts trimming did not work for exported variable declarations (GitHub #936)",
"type": "patch"
}
],
"packageName": "@microsoft/api-extractor",
"email": "pgonzal@users.noreply.github.com"
}