Skip to content
This repository has been archived by the owner on Mar 4, 2020. It is now read-only.

Preparing for API Extractor integration #2121

Merged
merged 3 commits into from Nov 20, 2019
Merged

Preparing for API Extractor integration #2121

merged 3 commits into from Nov 20, 2019

Conversation

ecraig12345
Copy link
Member

@ecraig12345 ecraig12345 commented Nov 20, 2019

(now from a branch instead of a fork)

Fabric uses API Extractor to help review API changes and generate docs. @kenotron and I would like to introduce it for FluentUI too, so we can use the API review functionality for converged components (and optionally existing components). It will also be a faster replacement for react-docgen-typescript.

This PR doesn't actually enable API Extractor or change anything about doc generation, but it introduces the config files and some required formatting and export changes. (I found these issues by running npx @microsoft/api-extractor run <config> in each project.)

Formatting

API Extractor requires that @param and @returns do not contain type annotations, and requires the parameter name in @param to be followed by a - (example: @param foo - description here). So I added dashes, got rid of all the remaining type annotations, and moved the type info to the function signature itself in cases where it wasn't already there.

Exports (and Babel upgrade)

(These more significant changes are grouped in the first commit for easy viewing.)

There were two re-exports which caused problems, because API Extractor doesn't currently support this type of re-export (from packages/react/src/lib/index.ts):

import * as commonPropTypes from './commonPropTypes'
export { commonPropTypes }

A workaround is to export a named module instead:

import {
  CreateCommonConfig as CreateCommonConfigLocal,
  createCommon as createCommonLocal,
} from './commonPropTypes'

export module commonPropTypes {
  export type CreateCommonConfig = CreateCommonConfigLocal
  export const createCommon = createCommonLocal
}

However, namespaces weren't supported by Babel until very recent versions. So I also had to upgrade Babel (and enable namespace support: ['@babel/preset-typescript', { allowNamespaces: true }]), which then required upgrading react-hot-loader to eliminate a very spammy warning introduced by the new Babel version.

I'd appreciate help/suggestions for verifying that the Babel upgrade didn't introduce any issues.

(The other problematic export was in packages/react/src/index.ts and was much simpler to deal with since it involved exporting values not types.)

Microsoft Reviewers: Open in CodeFlow

@DustyTheBot
Copy link
Collaborator

Warnings
⚠️ There are no updates provided to CHANGELOG. Ensure there are no publicly visible changes introduced by this PR.
⚠️ Package (or peer) dependencies changed. Make sure you have approval before merging!

Changed dependencies are detected.

Changed dependencies in packages/internal-tooling/package.json

package before after
@babel/core ^7.3.4 ^7.6.4
@babel/plugin-proposal-class-properties ^7.3.4 ^7.5.5
@babel/plugin-transform-runtime ^7.3.4 ^7.6.2
@babel/preset-env ^7.3.4 ^7.6.3
@babel/preset-react ^7.0.0 ^7.6.3
@babel/preset-typescript ^7.3.3 ^7.6.0

Generated by 🚫 dangerJS

Copy link
Member

@levithomason levithomason left a comment

Choose a reason for hiding this comment

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

👍

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.

None yet

3 participants