Skip to content

Api extractor for utilities #6406

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

Merged
merged 12 commits into from
Oct 10, 2018
Merged

Api extractor for utilities #6406

merged 12 commits into from
Oct 10, 2018

Conversation

kkjeer
Copy link
Contributor

@kkjeer kkjeer commented Sep 18, 2018

Pull request checklist

  • Addresses an existing issue: Fixes #0000
  • Include a change request file using $ npm run change

Description of changes

Enable the api-extractor task for the utilities package. Future PRs will enable api-extractor for other packages, add api-extractor to the CI build, and add code owners to *.api.ts files.

Focus areas to test

(optional)

Microsoft Reviewers: Open in CodeFlow

@kkjeer kkjeer requested a review from dzearing as a code owner September 18, 2018 21:29
@kkjeer kkjeer requested review from octogonz and iclanton September 18, 2018 21:31
@@ -15,7 +15,7 @@ export interface ICustomizableProps {

/**
* List of fields which can be customized.
* @default [ 'theme', 'styles' ]
* Default value: <pre>[ 'theme', 'styles' ]</pre>
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we change these from jsdoc commands?

Copy link

@octogonz octogonz Sep 19, 2018

Choose a reason for hiding this comment

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

@cliffkoh:

Why do we change these from jsdoc commands?

The <pre> HTML tag was just a temporary workaround because API Extractor does not yet support Markdown-style backticks. (That will be solved by the upcoming PR #827.)

Generally API Extractor is aligning with TSDoc, which has some differences from JSDoc. You can read about it here:
https://github.com/Microsoft/tsdoc

BTW there is an issue #27 proposing to support the @default tag in TSDoc. It hasn't been integrated yet because the spec for @default is still a little vague. Feel free to contribute to that issue if you think it's an important feature.

Copy link
Contributor

@cliffkoh cliffkoh left a comment

Choose a reason for hiding this comment

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

I'm concerned about the change away from jsdoc @ directives. This will be a problem for sure for main OUFR.

Copy link

@octogonz octogonz left a comment

Choose a reason for hiding this comment

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

:shipit:

@kkjeer
Copy link
Contributor Author

kkjeer commented Sep 20, 2018

This work item will wait until API Extractor @default tag support is resolved.

@kkjeer kkjeer closed this Sep 20, 2018
@kkjeer kkjeer reopened this Sep 25, 2018
@@ -86,8 +86,8 @@ export function getParent(child: HTMLElement, allowVirtualParents: boolean = tru
* Gets the elements which are child elements of the given element.
* If `allowVirtualChildren` is `true`, this method enumerates virtual child elements
* after the original children.
* @param parent
* @param allowVirtualChildren
* @param parent - The element to get the children of.
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the - between the parameter and the rest of the line something introduced by API Extractor? The jsdoc standard doesn't require it and in fact Typescript doesn't require it. Including it will result in the - superfluously showing up in intellisense

Copy link

@octogonz octogonz Sep 25, 2018

Choose a reason for hiding this comment

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

@cliffkoh Yes, and TSDoc will require the hyphen in "strict" mode. Omitting the hyphen would be ambiguous and error-prone, e.g. considering the other optional type annotations that JSDoc permits (which are not needed in TypeScript, but commonly occur nonetheless in TypeScript code bases).

I believe the IntelliSense you're referring to is coming from the VS Code plugin, not the TypeScript compiler itself. If you strongly object to the hyphen, you could open a TSDoc issue to discuss it. Otherwise I'd suggest to open a GitHub issue for VS Code; their doc comment analysis already has a number of other known issues -- this was part of the motivation for the compiler group getting involved with TSDoc.

@@ -26,13 +26,13 @@ const DefaultFields = ['theme', 'styles'];
* The styled HOC wrapper allows you to create a functional wrapper around a given component which will resolve
* getStyles functional props, and mix customized props passed in using concatStyleSets.
*
* @example
* ```tsx
* Example:
Copy link
Contributor

Choose a reason for hiding this comment

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

This is also regressive, but isn't as serious as the @defaultvalue. Would be good to understand where support for @example is on API Extractor's roadmap.

Choose a reason for hiding this comment

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

@cliffkoh The @example tag is already part of TSDoc and will be accepted in API Extractor 6. The syntax is discussed in microsoft/tsdoc#20

If you have suggestions/questions, please add them to that issue.

@kkjeer kkjeer added the Status: Blocked Resolution blocked by another issue label Sep 27, 2018
@kkjeer kkjeer removed the Status: Blocked Resolution blocked by another issue label Oct 9, 2018
@KevinTCoughlin KevinTCoughlin self-requested a review October 9, 2018 22:26
@dzearing dzearing merged commit ab236ea into microsoft:master Oct 10, 2018
@octogonz
Copy link

HURRAY!! :-P

@msft-github-bot
Copy link
Contributor

🎉@uifabric/[email protected] has been released which incorporates this pull request.:tada:

Handy Links:

@kkjeer kkjeer deleted the api-extractor branch October 10, 2018 17:05
@microsoft microsoft locked as resolved and limited conversation to collaborators Aug 30, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants