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

Add api-extractor to help reviewing API changes #116

Merged
merged 1 commit into from
Oct 3, 2024

Conversation

jeremymeng
Copy link
Collaborator

Description

This PR adds @microsoft/api-extractor dev dependency to help review API changes and identify issues of missing exports in public API surface.

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

import { AmqpError } from 'rhea';
import { Connection as Connection_2 } from 'rhea';
Copy link
Collaborator

Choose a reason for hiding this comment

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

is it possible to avoid these?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's a known api-extractor issue when there are types with the same names. I can live with it even it is not great.


// @public (undocumented)
export namespace EventContext {
// Warning: (ae-forgotten-export) The symbol "Link" needs to be exported by the entry point index.d.ts
Copy link
Collaborator

Choose a reason for hiding this comment

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

should we export?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I plan to have a separate PR

constructor(connection: Connection, session: Session_2);
// (undocumented)
begin(): void;
// Warning: (ae-forgotten-export) The symbol "SessionCloseOptions" needs to be exported by the entry point index.d.ts
Copy link
Collaborator

Choose a reason for hiding this comment

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

this

//
// @public
export class Connection extends Entity {
// Warning: (ae-forgotten-export) The symbol "CreatedRheaConnectionOptions" needs to be exported by the entry point index.d.ts
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks for this PR. I think not having an api document caused all these missed exports.

Copy link
Collaborator

@HarshaNalluru HarshaNalluru left a comment

Choose a reason for hiding this comment

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

Looks good. But we should consider exporting the missed pieces and publish a minor version for completion. (maybe as a separate PR)

@jeremymeng jeremymeng merged commit 1547bfe into amqp:master Oct 3, 2024
3 checks passed
@jeremymeng jeremymeng deleted the add-api-extractor branch October 3, 2024 21:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants