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

refactor(tinylicious-client): Clean up package exports #21370

Merged
merged 2 commits into from
Jun 11, 2024

Conversation

Josmithr
Copy link
Contributor

This package's primary export TinyliciousClient (as well as its related types) have all been made internal. Unfortunately, a bug in API-Extractor related to how it resolves export default statements was causing the TinyliciousClient type to appear in API reports when it shouldn't have.

Additionally, this package was re-exporting a couple of telemetry types to make use by external users more convenient (so they wouldn't have to import the core-interfaces package). But since this entire package is effectively internal-only now, these don't serve much purpose. I have removed them.

@Josmithr Josmithr requested a review from a team June 11, 2024 00:18
@github-actions github-actions bot added public api change Changes to a public API base: main PRs targeted against main branch labels Jun 11, 2024
export { ITelemetryBaseLogger }

export { TinyliciousClient }
export default TinyliciousClient;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The exports of TinyliciousClient should not have appeared in any of these reports, since it is tagged as @internal.

I've filed a bug with API-Extractor about this issue: microsoft/rushstack#4775

Copy link
Contributor

@alexvy86 alexvy86 left a comment

Choose a reason for hiding this comment

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

Looks fine. Do we know if TinyliciousClient was ever public? Is there already an old changeset when we first made it internal explicitly, or should we add that here?

@Josmithr
Copy link
Contributor Author

Looks fine. Do we know if TinyliciousClient was ever public? Is there already an old changeset when we first made it internal explicitly, or should we add that here?

Git blame shows it being internal since our initial tagging pass. So, it has never been explicitly marked as public. I think we're probably okay.

@Josmithr Josmithr merged commit bfb6132 into microsoft:main Jun 11, 2024
30 checks passed
@Josmithr Josmithr deleted the tinylicious-client/clean-up-exports branch June 11, 2024 17:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
base: main PRs targeted against main branch public api change Changes to a public API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants