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] Add an experimental new ApiItem.canonicalReference property #1406

Merged
merged 25 commits into from
Aug 8, 2019
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
6e980e4
Set up the new ApiItem.canonicalReference property
octogonz Jul 20, 2019
efaf045
Implement buildCanonicalReference() for each ApiItem subclass
octogonz Jul 20, 2019
b108208
Reintroduce the new "canonicalReference" to the .api.json file format
octogonz Jul 20, 2019
8f9550b
rush rebuild
octogonz Jul 20, 2019
9846fc3
rush change
octogonz Jul 20, 2019
36774d6
Fix a bug where a function with only one declaration was assigned an …
octogonz Jul 21, 2019
6e515f4
rush rebuild
octogonz Jul 22, 2019
aaf4d13
Fix how buildCanonicalReference() handles unnamed declarations
octogonz Jul 22, 2019
d772f41
Apply suggestions from code review
octogonz Jul 22, 2019
5a280f5
Update to use revised TSDoc API
octogonz Jul 22, 2019
6da8406
Merge remote-tracking branch 'remotes/origin/master' into octogonz/ae…
octogonz Jul 23, 2019
3889972
Update to use new TSDoc API
octogonz Jul 23, 2019
dec3e00
Workaround for double-bracketing of symbols
octogonz Jul 23, 2019
d3ff003
Replace _getCanonicalReferenceName() workaround with new TSDoc API De…
octogonz Jul 24, 2019
1d5ebe0
Merge remote-tracking branch 'remotes/origin/master' into octogonz/ae…
octogonz Jul 24, 2019
1049df1
Upgrade TSDoc
octogonz Jul 24, 2019
7a76ff6
rush change
octogonz Jul 24, 2019
8c247c0
Improve error reporting when the `DeclarationReference` parser fails
octogonz Jul 24, 2019
fc7c49a
Merge remote-tracking branch 'remotes/origin/master' into octogonz/ae…
octogonz Aug 7, 2019
238711f
rush update --full
octogonz Aug 7, 2019
132c874
Upgrade TSDoc
octogonz Aug 7, 2019
519e330
Bump ApiJsonSchemaVersion since we published V_1002 before this PR wa…
octogonz Aug 7, 2019
32fb006
rush rebuild with new schemaVersion
octogonz Aug 7, 2019
75fe2c3
PR Feedback: Rename ApiItem_setParent to ApiItem_onParentChanged to c…
octogonz Aug 7, 2019
55a4264
PR Feedback: Add `EntryPoint.importPath` property to clarify how ApiE…
octogonz Aug 8, 2019
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
56 changes: 48 additions & 8 deletions apps/api-extractor-model/src/items/ApiItem.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license.
// See LICENSE in the project root for license information.

import { DeclarationReference } from '@microsoft/tsdoc/lib/beta/DeclarationReference';
import { Constructor, PropertiesOf } from '../mixins/Mixin';
import { ApiPackage } from '../model/ApiPackage';
import { ApiParameterListMixin } from '../mixins/ApiParameterListMixin';
import { DeserializerContext } from '../model/DeserializerContext';
import { InternalError } from '@microsoft/node-core-library';

/**
* The type returned by the {@link ApiItem.kind} property, which can be used to easily distinguish subclasses of
Expand Down Expand Up @@ -44,14 +46,13 @@ export interface IApiItemOptions {

export interface IApiItemJson {
kind: ApiItemKind;
canonicalReference: string;
}

/**
* PRIVATE
* Allows ApiItemContainerMixin to assign the parent.
*/
// PRIVATE - Allows ApiItemContainerMixin to assign the parent.
//
// tslint:disable-next-line:variable-name
export const ApiItem_parent: unique symbol = Symbol('ApiItem._parent');
export const ApiItem_setParent: unique symbol = Symbol('ApiItem._setParent');

/**
* The abstract base class for all members of an `ApiModel` object.
Expand All @@ -62,7 +63,8 @@ export const ApiItem_parent: unique symbol = Symbol('ApiItem._parent');
* @public
*/
export class ApiItem {
public [ApiItem_parent]: ApiItem | undefined;
private _canonicalReference: DeclarationReference | undefined;
private _parent: ApiItem | undefined;

public static deserialize(jsonObject: IApiItemJson, context: DeserializerContext): ApiItem {
// The Deserializer class is coupled with a ton of other classes, so we delay loading it
Expand All @@ -84,6 +86,7 @@ export class ApiItem {
/** @virtual */
public serializeInto(jsonObject: Partial<IApiItemJson>): void {
jsonObject.kind = this.kind;
jsonObject.canonicalReference = this.canonicalReference.toString();
octogonz marked this conversation as resolved.
Show resolved Hide resolved
}

/**
Expand All @@ -94,6 +97,22 @@ export class ApiItem {
throw new Error('ApiItem.kind was not implemented by the child class');
}

/**
* Warning: This API is used internally by API extractor but is not yet ready for general usage.
*
* @remarks
*
* Returns a `DeclarationReference` object using the experimental new declaration reference notation.
*
* @beta
*/
public get canonicalReference(): DeclarationReference {
if (!this._canonicalReference) {
octogonz marked this conversation as resolved.
Show resolved Hide resolved
this._canonicalReference = this.buildCanonicalReference();
}
return this._canonicalReference;
}

/**
* Returns a string key that can be used to efficiently retrieve an `ApiItem` from an `ApiItemContainerMixin`.
* The key is unique within the container. Its format is undocumented and may change at any time.
Expand All @@ -105,7 +124,7 @@ export class ApiItem {
* @virtual
*/
public get containerKey(): string {
throw new Error('ApiItem.containerKey was not implemented by the child class');
throw new InternalError('ApiItem.containerKey was not implemented by the child class');
}

/**
Expand Down Expand Up @@ -135,7 +154,7 @@ export class ApiItem {
* @virtual
*/
public get parent(): ApiItem | undefined {
return this[ApiItem_parent];
return this._parent;
}

/**
Expand Down Expand Up @@ -215,6 +234,27 @@ export class ApiItem {
public getSortKey(): string {
return this.containerKey;
}

/**
* PRIVATE
*
* @privateRemarks
* Allows ApiItemContainerMixin to assign the parent.
*
* @internal
*/
public [ApiItem_setParent](parent: ApiItem | undefined): void {
octogonz marked this conversation as resolved.
Show resolved Hide resolved
this._parent = parent;
this._canonicalReference = undefined;
}

/**
* Builds the cached object used by the `canonicalReference` property.
* @virtual
*/
protected buildCanonicalReference(): DeclarationReference {
throw new InternalError('ApiItem.canonicalReference was not implemented by the child class');
}
}

/**
Expand Down
6 changes: 3 additions & 3 deletions apps/api-extractor-model/src/mixins/ApiItemContainerMixin.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license.
// See LICENSE in the project root for license information.s

import { ApiItem, ApiItem_parent, IApiItemJson, IApiItemOptions, IApiItemConstructor } from '../items/ApiItem';
import { ApiItem, ApiItem_setParent, IApiItemJson, IApiItemOptions, IApiItemConstructor } from '../items/ApiItem';
import { ApiNameMixin } from './ApiNameMixin';
import { DeserializerContext } from '../model/DeserializerContext';

Expand Down Expand Up @@ -135,7 +135,7 @@ export function ApiItemContainerMixin<TBaseClass extends IApiItemConstructor>(ba
throw new Error('Another member has already been added with the same name and containerKey');
}

const existingParent: ApiItem | undefined = member[ApiItem_parent];
const existingParent: ApiItem | undefined = member.parent;
if (existingParent !== undefined) {
throw new Error(`This item has already been added to another container: "${existingParent.displayName}"`);
}
Expand All @@ -145,7 +145,7 @@ export function ApiItemContainerMixin<TBaseClass extends IApiItemConstructor>(ba
this[_membersSorted] = false;
this[_membersByContainerKey].set(member.containerKey, member);

member[ApiItem_parent] = this;
member[ApiItem_setParent](this);
}

public tryGetMemberByKey(containerKey: string): ApiItem | undefined {
Expand Down
15 changes: 15 additions & 0 deletions apps/api-extractor-model/src/mixins/ApiNameMixin.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license.
// See LICENSE in the project root for license information.s

import { StringChecks } from '@microsoft/tsdoc/lib/parser/StringChecks';
import { ApiItem, IApiItemJson, IApiItemConstructor, IApiItemOptions } from '../items/ApiItem';
import { DeserializerContext } from '../model/DeserializerContext';

Expand Down Expand Up @@ -46,6 +47,9 @@ export interface ApiNameMixin extends ApiItem {

/** @override */
serializeInto(jsonObject: Partial<IApiItemJson>): void;

/** @internal */
_getCanonicalReferenceName(): string;
}

/**
Expand Down Expand Up @@ -94,6 +98,17 @@ export function ApiNameMixin<TBaseClass extends IApiItemConstructor>(baseClass:

jsonObject.name = this.name;
}

/** @internal */
public _getCanonicalReferenceName(): string {
// TODO: This is a temporary workaround for a limitation of the experimental DeclarationReference API.
// We will remove this when the final implementation is in place.
octogonz marked this conversation as resolved.
Show resolved Hide resolved
if (StringChecks.explainIfInvalidUnquotedIdentifier(this.name)) {
return JSON.stringify(this.name);
} else {
return this.name;
}
}
}

return MixedClass;
Expand Down
12 changes: 12 additions & 0 deletions apps/api-extractor-model/src/model/ApiCallSignature.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license.
// See LICENSE in the project root for license information.

import { DeclarationReference, Meaning, Navigation } from '@microsoft/tsdoc/lib/beta/DeclarationReference';
import { ApiItemKind } from '../items/ApiItem';
import { IApiDeclaredItemOptions, ApiDeclaredItem } from '../items/ApiDeclaredItem';
import { IApiParameterListMixinOptions, ApiParameterListMixin } from '../mixins/ApiParameterListMixin';
Expand Down Expand Up @@ -69,4 +70,15 @@ export class ApiCallSignature extends ApiTypeParameterListMixin(ApiParameterList
public get containerKey(): string {
return ApiCallSignature.getContainerKey(this.overloadIndex);
}

/** @beta @override */
public buildCanonicalReference(): DeclarationReference {
const parent: DeclarationReference = this.parent
? this.parent.canonicalReference
// .withMeaning() requires some kind of component
: DeclarationReference.empty().addNavigationStep(Navigation.Members, '(parent)');
return parent
.withMeaning(Meaning.Signature)
.withOverloadIndex(this.overloadIndex);
}
}
8 changes: 8 additions & 0 deletions apps/api-extractor-model/src/model/ApiClass.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license.
// See LICENSE in the project root for license information.

import { DeclarationReference, Meaning, Navigation } from '@microsoft/tsdoc/lib/beta/DeclarationReference';
import { ApiItemKind } from '../items/ApiItem';
import { ApiDeclaredItem, IApiDeclaredItemOptions, IApiDeclaredItemJson } from '../items/ApiDeclaredItem';
import { ApiItemContainerMixin, IApiItemContainerMixinOptions } from '../mixins/ApiItemContainerMixin';
Expand Down Expand Up @@ -116,4 +117,11 @@ export class ApiClass extends ApiItemContainerMixin(ApiNameMixin(ApiTypeParamete

jsonObject.implementsTokenRanges = this.implementsTypes.map(x => x.excerpt.tokenRange);
}

/** @beta @override */
public buildCanonicalReference(): DeclarationReference {
return (this.parent ? this.parent.canonicalReference : DeclarationReference.empty())
.addNavigationStep(Navigation.Exports, this.name)
.withMeaning(Meaning.Class);
}
}
12 changes: 12 additions & 0 deletions apps/api-extractor-model/src/model/ApiConstructSignature.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license.
// See LICENSE in the project root for license information.

import { DeclarationReference, Meaning, Navigation } from '@microsoft/tsdoc/lib/beta/DeclarationReference';
import { ApiItemKind } from '../items/ApiItem';
import { IApiDeclaredItemOptions, ApiDeclaredItem } from '../items/ApiDeclaredItem';
import { IApiParameterListMixinOptions, ApiParameterListMixin } from '../mixins/ApiParameterListMixin';
Expand Down Expand Up @@ -82,4 +83,15 @@ export class ApiConstructSignature extends ApiTypeParameterListMixin(ApiParamete
public get containerKey(): string {
return ApiConstructSignature.getContainerKey(this.overloadIndex);
}

/** @beta @override */
public buildCanonicalReference(): DeclarationReference {
const parent: DeclarationReference = this.parent
? this.parent.canonicalReference
// .withMeaning() requires some kind of component
: DeclarationReference.empty().addNavigationStep(Navigation.Members, '(parent)');
return parent
.withMeaning(Meaning.Signature)
.withOverloadIndex(this.overloadIndex);
}
}
12 changes: 12 additions & 0 deletions apps/api-extractor-model/src/model/ApiConstructor.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license.
// See LICENSE in the project root for license information.

import { DeclarationReference, Meaning, Navigation } from '@microsoft/tsdoc/lib/beta/DeclarationReference';
import { ApiItemKind } from '../items/ApiItem';
import { IApiDeclaredItemOptions, ApiDeclaredItem } from '../items/ApiDeclaredItem';
import { IApiParameterListMixinOptions, ApiParameterListMixin } from '../mixins/ApiParameterListMixin';
Expand Down Expand Up @@ -62,4 +63,15 @@ export class ApiConstructor extends ApiParameterListMixin(ApiReleaseTagMixin(Api
public get containerKey(): string {
return ApiConstructor.getContainerKey(this.overloadIndex);
}

/** @beta @override */
public buildCanonicalReference(): DeclarationReference {
const parent: DeclarationReference = this.parent
? this.parent.canonicalReference
// .withMeaning() requires some kind of component
: DeclarationReference.empty().addNavigationStep(Navigation.Members, '(parent)');
return parent
.withMeaning(Meaning.Constructor)
.withOverloadIndex(this.overloadIndex);
}
}
21 changes: 21 additions & 0 deletions apps/api-extractor-model/src/model/ApiEntryPoint.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license.
// See LICENSE in the project root for license information.

import { DeclarationReference, ModuleSource } from '@microsoft/tsdoc/lib/beta/DeclarationReference';
import { ApiItem, ApiItemKind } from '../items/ApiItem';
import { ApiItemContainerMixin, IApiItemContainerMixinOptions } from '../mixins/ApiItemContainerMixin';
import { IApiNameMixinOptions, ApiNameMixin } from '../mixins/ApiNameMixin';
import { ApiPackage } from './ApiPackage';

/**
* Constructor options for {@link ApiEntryPoint}.
Expand Down Expand Up @@ -51,4 +53,23 @@ export class ApiEntryPoint extends ApiItemContainerMixin(ApiNameMixin(ApiItem))
// No prefix needed, because ApiEntryPoint is the only possible member of an ApiPackage
return this.name;
}

/** @beta @override */
public buildCanonicalReference(): DeclarationReference {
let modulePath: string = '';

if (this.parent instanceof ApiPackage) {
modulePath = this.parent.name;
}

if (this.name) {
modulePath += this.name;
octogonz marked this conversation as resolved.
Show resolved Hide resolved
}

if (modulePath) {
return new DeclarationReference(new ModuleSource(modulePath));
}

return DeclarationReference.empty();
octogonz marked this conversation as resolved.
Show resolved Hide resolved
}
}
8 changes: 8 additions & 0 deletions apps/api-extractor-model/src/model/ApiEnum.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license.
// See LICENSE in the project root for license information.

import { DeclarationReference, Meaning, Navigation } from '@microsoft/tsdoc/lib/beta/DeclarationReference';
import { ApiItemKind } from '../items/ApiItem';
import { ApiDeclaredItem, IApiDeclaredItemOptions } from '../items/ApiDeclaredItem';
import { ApiReleaseTagMixin, IApiReleaseTagMixinOptions } from '../mixins/ApiReleaseTagMixin';
Expand Down Expand Up @@ -71,4 +72,11 @@ export class ApiEnum extends ApiItemContainerMixin(ApiNameMixin(ApiReleaseTagMix
}
super.addMember(member);
}

/** @beta @override */
public buildCanonicalReference(): DeclarationReference {
return (this.parent ? this.parent.canonicalReference : DeclarationReference.empty())
.addNavigationStep(Navigation.Exports, this._getCanonicalReferenceName())
Copy link
Member

Choose a reason for hiding this comment

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

Will you only have ApiEnum nodes that are guaranteed to be exported, or do you anticipate non-exported enums? This is perfectly legal TypeScript:

// foo.d.ts
const enum NotExported { Foo, Bar }
export function f(): NotExported;

The return type of f should have the reference foo!~NotExported:enum, but this indicates the enum will always be an export and not a local (i.e. non-export).

Copy link
Collaborator Author

@octogonz octogonz Jul 22, 2019

Choose a reason for hiding this comment

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

Today API Extractor considers NotExported to be essentially nameless (e.g. its name is free to be mangled in the .d.ts rollup). We don't expect there to be a documentation page for it. The ae-forgotten-export warning is reported, and currently it does not appear in the .api.json file at all.

The philosophy is like:

"If the type is so important that you'd make a website page for it, then shouldn't it have a name so we can talk about it? Shouldn't that name be exported so that developers can defer to it in a type annotation?"

This is subjective, however. If people can provide compelling justifications, we may need to support ~ in the future.

.withMeaning(Meaning.Enum);
}
}
8 changes: 8 additions & 0 deletions apps/api-extractor-model/src/model/ApiEnumMember.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license.
// See LICENSE in the project root for license information.

import { DeclarationReference, Meaning, Navigation } from '@microsoft/tsdoc/lib/beta/DeclarationReference';
import { ApiItemKind } from '../items/ApiItem';
import { ApiDeclaredItem, IApiDeclaredItemOptions, IApiDeclaredItemJson } from '../items/ApiDeclaredItem';
import { ApiReleaseTagMixin, IApiReleaseTagMixinOptions } from '../mixins/ApiReleaseTagMixin';
Expand Down Expand Up @@ -86,4 +87,11 @@ export class ApiEnumMember extends ApiNameMixin(ApiReleaseTagMixin(ApiDeclaredIt

jsonObject.initializerTokenRange = this.initializerExcerpt.tokenRange;
}

/** @beta @override */
public buildCanonicalReference(): DeclarationReference {
return (this.parent ? this.parent.canonicalReference : DeclarationReference.empty())
.addNavigationStep(Navigation.Exports, this._getCanonicalReferenceName())
.withMeaning(Meaning.EnumMember);
}
}
9 changes: 9 additions & 0 deletions apps/api-extractor-model/src/model/ApiFunction.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license.
// See LICENSE in the project root for license information.

import { DeclarationReference, Meaning, Navigation } from '@microsoft/tsdoc/lib/beta/DeclarationReference';
import { ApiItemKind } from '../items/ApiItem';
import { IApiDeclaredItemOptions, ApiDeclaredItem } from '../items/ApiDeclaredItem';
import { IApiParameterListMixinOptions, ApiParameterListMixin } from '../mixins/ApiParameterListMixin';
Expand Down Expand Up @@ -63,4 +64,12 @@ export class ApiFunction extends ApiNameMixin(ApiTypeParameterListMixin(ApiParam
public get containerKey(): string {
return ApiFunction.getContainerKey(this.name, this.overloadIndex);
}

/** @beta @override */
public buildCanonicalReference(): DeclarationReference {
return (this.parent ? this.parent.canonicalReference : DeclarationReference.empty())
.addNavigationStep(Navigation.Exports, this._getCanonicalReferenceName())
octogonz marked this conversation as resolved.
Show resolved Hide resolved
.withMeaning(Meaning.Function)
.withOverloadIndex(this.overloadIndex);
}
}
12 changes: 12 additions & 0 deletions apps/api-extractor-model/src/model/ApiIndexSignature.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license.
// See LICENSE in the project root for license information.

import { DeclarationReference, Meaning, Navigation } from '@microsoft/tsdoc/lib/beta/DeclarationReference';
import { ApiItemKind } from '../items/ApiItem';
import { IApiDeclaredItemOptions, ApiDeclaredItem } from '../items/ApiDeclaredItem';
import { IApiParameterListMixinOptions, ApiParameterListMixin } from '../mixins/ApiParameterListMixin';
Expand Down Expand Up @@ -59,4 +60,15 @@ export class ApiIndexSignature extends ApiParameterListMixin(ApiReleaseTagMixin(
public get containerKey(): string {
return ApiIndexSignature.getContainerKey(this.overloadIndex);
}

/** @beta @override */
public buildCanonicalReference(): DeclarationReference {
const parent: DeclarationReference = this.parent
? this.parent.canonicalReference
// .withMeaning() requires some kind of component
: DeclarationReference.empty().addNavigationStep(Navigation.Members, '(parent)');
return parent
.withMeaning(Meaning.Signature)
.withOverloadIndex(this.overloadIndex);
}
}
Loading