Move implementation details into separate package#993
Draft
simolus3 wants to merge 12 commits into
Draft
Conversation
🦋 Changeset detectedLatest commit: 98c6f74 The changes in this PR will be included in the next version bump. This PR includes changesets to release 11 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This moves
@internalmembers from@powersync/commoninto the new@powersync/shared-internalspackage. The idea is that@powersync/shared-internalswould never be imported by users, so we can change it freely.@powersync/commonstill contains and exports common interfaces relevant across multiple SDKs.This PR effectively solves two long-term goals:
@powersync/common(which is then re-exported from our SDKs) made it very easy to get this wrong. By moving things to@powersync/shared-internals(which we obviously wouldn't re-export), we're able to encapsulate implementation details much better.AbstractPowerSyncDatabasewas both a class and effectively a public interface. We often forgot about the second part, and added new methods without realizing that this is a breaking change! Since implementation details are now part of@powersync/shared-internals, interfaces are extracted explicitly.For the most part, this PR just moves code around: Identify stuff marked as
@internalin@powersync/common, move it into the internals package, update imports, repeat. There are two exceptions, which I'll outline below.AbstractPowerSyncDatabase
This class is both an interface (users imported it from
@powersync/commonas a type) and an implementation detail (it's an abstract class only meant to be extended from our SDKs). This makes it very easy to leak implementation details into the interface, so this introduces a split:@powersync/common, this introducesCommonPowerSyncDatabase: A TypeScript interface containing public members from the oldAbstractPowerSyncDatabaseclass. When writing APIs that need to work across SDKs (attachments, React hooks, ...), this is the type to use.@powersync/shared-internals,BasePowerSyncDatabaseimplements that interface, and our existing SDKs continue to extend that class.There is a crucial detail here: We can't have an
export class PowerSyncDatabase extends BasePowerSyncDatabaseanymore, as that would leak everything defined onBasePowerSyncDatabaseinto the implicit interface forPowerSyncDatabase. So, we make the actual implementation class private and only export the constructor, which is declared to return aCommonPowerSyncDatabase.SyncStatus
The sync status was a mix of public and private fields, with e.g. sync streams being derived from core extension data structures only. I don't want to keep those structures in
@powersync/commonbecause they're an implementation detail, so this restructuresSyncStatusinto a public interface and an internal implementation (in@powersync/shared-internals). A similar transformation is applied to some CRUD classes.