Adopt API extractor tool for @powersync/common#984
Conversation
|
rkistner
left a comment
There was a problem hiding this comment.
This looks like quite a nice approach! Some questions (mostly based on the description):
- Do these annotations affect the generated types, or does it only serve as "documentation" at the moment?
- If it does affect the types, how does this handle cases where we use the types in a different workspace package, but it's not intended for public consumption? Are those marked as internal or public?
We would mark those as |
|
Looking at the generated |
This would technically be a breaking change (since we still export these members from
So these warnings are todos in a sense, but we need the |
9505fad to
8a0aa31
Compare
We want to eventually split
@powersync/commoninto multiple packages to avoid exposing implementation details to users. As a first step, we should be explicit about what is and what isn't a public API. tsdoc has@publicand@internalannotations for this, adopting them consistently is a first step.This also adopts
@microsoft/api-extractorwith the default configuration. This helps with:@links,@params).src/index.tsneeds either a@public,@internal,@betaor@alphaannotation. This forces us to be explicit about where a member should be visible.@powersync/commonin ways we didn't intend. Thanks to TypeScript's structural typing and npm allowing multiple copies of a package, even subtle things like adding a new method to a class are breaking changes. We need to exercise much more caution when working on@powersync/common, having this tool makes API changes explicit (becauseetc/common.api.mdneeds to be re-generated).api-extractorgenerates a warning for these inetc/common.api.md. There are around remaining 130 instances of this in@powersync/common, I'll investigate fixes as part of the v2 work after removing implementation details from that package.A follow-up step (also on the v2 branch) is to not have a single
@internalmember in all of@powersync/common. Internal members should either be:DEFAULT_INDEX_COLUMN_OPTIONSwhich aren't used outside of@powersync/common).AbstractRemote,AbstractStreamingSyncImplementation): We should move these into a@powersync/shared-internalspackage without a stable public API.Some cases are a little trickier:
AbstractPowerSyncDatabase(the class) should be internal, but it's type should be public to users. This is also v2 work.This doesn't change our API or exported members in any way, so we can target
maininstead of thev2branch. This also doesn't need a changeset entry for that reason.