Skip to content

Commit

Permalink
[EngSys] Generate subpath export review files (#28730)
Browse files Browse the repository at this point in the history
### Packages impacted by this PR

@azure/notifications-hub

### Issues associated with this PR

Contributes to #28750 but completion requires updating all the packages
to use this

### Describe the problem that is addressed by this PR

Building off of @jeremymeng's work in #27646, this PR configures API
review files for packages containing subpath exports.

While APIView is not yet ready to support multiple json files, this
provides us with appropriate API reviews tracking any public API changes.

Follow-up PRs will add support for this in APIView, but this feels like
a quick win and step forward.

### What are the possible designs available to address the problem? If
there are more than one possible design, why was the one in this PR
chosen?

A few alternatives, and I am open to feedback:

1. Update the EngSys script to allow multiple DocModel json files - I
chose to go with a flag to disable generating them instead, so that we
can build off of it in pieces
2. File naming convention - I already got and applied some feedback on
that, thanks Timo
  • Loading branch information
maorleger authored Mar 28, 2024
1 parent eb6db07 commit f1a5ba1
Show file tree
Hide file tree
Showing 6 changed files with 1,244 additions and 30 deletions.
2 changes: 1 addition & 1 deletion .vscode/cspell.json
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,7 @@
"words": ["fourty", "Milli", "rollupby"]
},
{
"filename": "sdk/notificationhubs/notification-hubs/review/notification-hubs.api.md",
"filename": "sdk/notificationhubs/notification-hubs/review/*.api.md",
"words": ["fcmv"]
},
{
Expand Down
3 changes: 2 additions & 1 deletion common/tools/dev-tool/src/commands/index.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license

import { subCommand, makeCommandInfo } from "../framework/command";
import { makeCommandInfo, subCommand } from "../framework/command";

import { createPrinter } from "../util/printer";

const log = createPrinter("dev-tool");
Expand Down
90 changes: 64 additions & 26 deletions common/tools/dev-tool/src/commands/run/extract-api.ts
Original file line number Diff line number Diff line change
@@ -1,36 +1,42 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.

import { leafCommand, makeCommandInfo } from "../../framework/command";
import {
Extractor,
ExtractorConfig,
ExtractorLogLevel,
ExtractorResult,
IConfigApiReport,
IConfigDocModel,
IConfigDtsRollup,
IConfigFile,
} from "@microsoft/api-extractor";
import { createReadStream, createWriteStream } from "node:fs";
import { leafCommand, makeCommandInfo } from "../../framework/command";

import archiver from "archiver";
import { createPrinter } from "../../util/printer";
import { resolveProject } from "../../util/resolveProject";
import path from "path";
import { readFile } from "fs-extra";
import { readdir } from "node:fs/promises";
import { createReadStream, createWriteStream } from "node:fs";
import archiver from "archiver";
import { resolveProject } from "../../util/resolveProject";

export const commandInfo = makeCommandInfo(
"extract-api",
"Runs api-extractor multiple times for all exports.",
{},
{
"subpath-doc-model": {
shortName: "sdc",
kind: "boolean",
default: false,
description:
"When true, generates api.json docModel files for each subpath export. Otherwise only generates for the main entry point. Markdown files are always generated for each subpath export.",
},
},
);

const log = createPrinter("extract-api");

function getSuffix(exportPath: string): string {
return exportPath.replaceAll("/", "\u2215"); // replace with division slash to avoid file system issue
}

function extractApi(
extractorConfigObject: IConfigFile,
apiExtractorJsonPath: string,
Expand Down Expand Up @@ -60,7 +66,7 @@ function extractApi(
}
}

export default leafCommand(commandInfo, async () => {
export default leafCommand(commandInfo, async (options) => {
const projectInfo = await resolveProject(process.cwd());
const packageJsonPath = path.join(projectInfo.path, "package.json");
const packageJson = JSON.parse((await readFile(packageJsonPath)).toString("utf-8"));
Expand All @@ -85,30 +91,50 @@ export default leafCommand(commandInfo, async () => {
// sub path exports extraction
const exports = packageJson["exports"];
if (exports) {
for (const exprt of Object.keys(exports)) {
log.debug(`extracting Api for "${exprt}"`);
const suffix = getSuffix(exprt);
const newPublicTrimmedPath = extractorConfigObject.dtsRollup.publicTrimmedFilePath.replace(
".d.ts",
`-${suffix}.d.ts`,
);
const newApiJsonPath = extractorConfigObject.docModel?.apiJsonFilePath?.replace(
".api.json",
`-${suffix}.api.json`,
);
const newApiReportName = extractorConfigObject.apiReport?.reportFileName?.replace(
".api.md",
`-${suffix}.api.md`,
log.info("Detected subpath exports, extracting markdown for each subpath.");
for (const exportPath of Object.keys(exports)) {
const customizations = {
isSubpathExport: exportPath !== ".",
exportBaseName: path.parse(exportPath).name,
generateDocModel: exportPath === "." || options["subpath-doc-model"],
};

// Place the subpath export rollup file in the directory from which it is exported
let publicTrimmedFilePath = path.parse(extractorConfigObject.dtsRollup.publicTrimmedFilePath);
const newPublicTrimmedPath = path.join(
publicTrimmedFilePath.dir,
exportPath,
publicTrimmedFilePath.base,
);

// Leave filenames unchanged for the root export
let newApiJsonPath = extractorConfigObject.docModel?.apiJsonFilePath;
let newApiReportName = extractorConfigObject.apiReport?.reportFileName;

if (customizations.isSubpathExport) {
// Append the subpath export name to the api.json and api.md file names
// e.g. for ./rest export: <unscopedPackageName>.api.json -> <unscopedPackageName>-rest.api.json
// e.g. for ./rest export: <unscopedPackageName>.api.md -> <unscopedPackageName>-rest.api.md
newApiJsonPath = extractorConfigObject.docModel?.apiJsonFilePath?.replace(
".api.json",
`-${customizations.exportBaseName}.api.json`,
);
newApiReportName = extractorConfigObject.apiReport?.reportFileName?.replace(
".api.md",
`-${customizations.exportBaseName}.api.md`,
);
}

const newDtsRollupOptions: IConfigDtsRollup = {
...extractorConfigObject.dtsRollup,
publicTrimmedFilePath: newPublicTrimmedPath,
};
const newDocModel: IConfigDocModel = {
...extractorConfigObject.docModel,
enabled: true,
enabled: customizations.generateDocModel,
apiJsonFilePath: newApiJsonPath,
};

const newApiReport: IConfigApiReport = {
...extractorConfigObject.apiReport,
enabled: true, // or should we disable? not sure if we need look at multiple api.md
Expand All @@ -119,12 +145,24 @@ export default leafCommand(commandInfo, async () => {
dtsRollup: newDtsRollupOptions,
docModel: newDocModel,
apiReport: newApiReport,
mainEntryPointFilePath: exports[exprt].types,
mainEntryPointFilePath: exports[exportPath].types,
};

// subpath exports often use top-level exports, but API Extractor is not aware of this and
// flags them as forgotten exports.
if (
customizations.isSubpathExport &&
updatedConfigObject.messages?.extractorMessageReporting
) {
updatedConfigObject.messages.extractorMessageReporting["ae-forgotten-export"] = {
addToApiReportFile: false,
logLevel: ExtractorLogLevel.None,
};
}
succeed = extractApi(updatedConfigObject, apiExtractorJsonPath, packageJsonPath);
}
} else {
log.info("No subpath exports detected, extracting api for main entry point.");
// normal extraction
succeed = extractApi(extractorConfigObject, apiExtractorJsonPath, packageJsonPath);
}
Expand Down
4 changes: 2 additions & 2 deletions sdk/notificationhubs/notification-hubs/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,11 @@
"bundle": "tsc -p . && dev-tool run bundle",
"build:samples": "echo Obsolete",
"build:test": "npm run bundle",
"build": "npm run build:test && api-extractor run --local",
"build": "npm run build:test && npm run extract-api",
"check-format": "dev-tool run vendored prettier --list-different --config ../../../.prettierrc.json --ignore-path ../../../.prettierignore \"src/**/*.ts\" \"samples-dev/**/*.ts\" \"test/**/*.ts\" \"*.{js,json}\"",
"clean": "rimraf --glob dist dist-* temp types *.tgz *.log",
"execute:samples": "dev-tool samples run samples-dev",
"extract-api": "tsc -p . && api-extractor run --local",
"extract-api": "tsc -p . && dev-tool run extract-api",
"format": "dev-tool run vendored prettier --write --config ../../../.prettierrc.json --ignore-path ../../../.prettierignore \"src/**/*.ts\" \"test/**/*.ts\" \"samples-dev/**/*.ts\" \"*.{js,json}\"",
"integration-test:browser": "echo skipped",
"integration-test:node": "dev-tool run test:node-js-input -- --timeout 600000 \"dist-esm/test/internal/**/*.spec.js\" \"dist-esm/test/public/**/*.spec.js\"",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
## API Report File for "@azure/notification-hubs"

> Do not edit this file. It is a report generated by [API Extractor](https://api-extractor.com/).
```ts

/// <reference types="node" />

import { ClientOptions } from '@azure-rest/core-client';
import { HttpHeaders } from '@azure/core-rest-pipeline';
import { OperationOptions } from '@azure-rest/core-client';
import { OperationState } from '@azure/core-lro';
import { PagedAsyncIterableIterator } from '@azure/core-paging';
import { PipelineRequest } from '@azure/core-rest-pipeline';
import { PipelineResponse } from '@azure/core-rest-pipeline';
import { SimplePollerLike } from '@azure/core-lro';

// @public
export function beginSubmitNotificationHubJob(context: NotificationHubsClientContext, notificationHubJob: NotificationHubJob, polledOperationOptions?: PolledOperationOptions): Promise<NotificationHubJobPoller>;

// @public
export function cancelScheduledNotification(context: NotificationHubsClientContext, notificationId: string, options?: OperationOptions): Promise<NotificationHubsResponse>;

// @public
export function createClientContext(connectionString: string, hubName: string, options?: NotificationHubsClientOptions): NotificationHubsClientContext;

// @public
export function createOrUpdateInstallation(context: NotificationHubsClientContext, installation: Installation, options?: OperationOptions): Promise<NotificationHubsResponse>;

// @public
export function createOrUpdateRegistration(context: NotificationHubsClientContext, registration: RegistrationDescription, options?: OperationOptions): Promise<RegistrationDescription>;

// @public
export function createRegistration(context: NotificationHubsClientContext, registration: RegistrationDescription, options?: OperationOptions): Promise<RegistrationDescription>;

// @public
export function createRegistrationId(context: NotificationHubsClientContext, options?: OperationOptions): Promise<string>;

// @public
export function deleteInstallation(context: NotificationHubsClientContext, installationId: string, options?: OperationOptions): Promise<NotificationHubsResponse>;

// @public
export function deleteRegistration(context: NotificationHubsClientContext, registrationId: string, options?: EntityOperationOptions): Promise<NotificationHubsResponse>;

// @public
export function getFeedbackContainerUrl(context: NotificationHubsClientContext, options?: OperationOptions): Promise<string>;

// @public
export function getInstallation(context: NotificationHubsClientContext, installationId: string, options?: OperationOptions): Promise<Installation>;

// @public
export function getNotificationHubJob(context: NotificationHubsClientContext, jobId: string, options?: OperationOptions): Promise<NotificationHubJob>;

// @public
export function getNotificationOutcomeDetails(context: NotificationHubsClientContext, notificationId: string, options?: OperationOptions): Promise<NotificationDetails>;

// @public
export function getRegistration(context: NotificationHubsClientContext, registrationId: string, options?: OperationOptions): Promise<RegistrationDescription>;

// @public
export function listNotificationHubJobs(context: NotificationHubsClientContext, options?: OperationOptions): Promise<NotificationHubJob[]>;

// @public
export function listRegistrations(context: NotificationHubsClientContext, options?: RegistrationQueryLimitOptions): PagedAsyncIterableIterator<RegistrationDescription>;

// @public
export function listRegistrationsByChannel(context: NotificationHubsClientContext, channel: RegistrationChannel, options?: RegistrationQueryLimitOptions): PagedAsyncIterableIterator<RegistrationDescription>;

// @public
export function listRegistrationsByTag(context: NotificationHubsClientContext, tag: string, options?: RegistrationQueryLimitOptions): PagedAsyncIterableIterator<RegistrationDescription>;

// @public
export interface NotificationHubsClientContext {
// @internal (undocumented)
createHeaders(operationName: string, rawHeaders?: Record<string, string>): Promise<HttpHeaders>;
// @internal (undocumented)
requestUrl(): URL;
// @internal (undocumented)
sendRequest(request: PipelineRequest): Promise<PipelineResponse>;
}

// @public
export function scheduleNotification(context: NotificationHubsClientContext, scheduledTime: Date, notification: Notification, options?: ScheduleNotificationOptions): Promise<NotificationHubsMessageResponse>;

// @public
export function sendNotification(context: NotificationHubsClientContext, notification: Notification, options?: DirectSendNotificationOptions | SendNotificationOptions): Promise<NotificationHubsMessageResponse>;

// @public
export function submitNotificationHubJob(context: NotificationHubsClientContext, job: NotificationHubJob, options?: OperationOptions): Promise<NotificationHubJob>;

// @public
export function updateInstallation(context: NotificationHubsClientContext, installationId: string, installationPatches: JsonPatch[], options?: OperationOptions): Promise<NotificationHubsResponse>;

// @public
export function updateRegistration(context: NotificationHubsClientContext, registration: RegistrationDescription, options?: OperationOptions): Promise<RegistrationDescription>;

// (No @packageDocumentation comment for this package)

```
Loading

0 comments on commit f1a5ba1

Please sign in to comment.