Skip to content

Conversation

@rkistner
Copy link
Contributor

This contains no functional changes, but does contain some breaking "api" changes for the @powersync/service-sync-rules package. This is primarily to clean up the code to make it easier to make larger changes that I'll be working on soon.

Main changes:

  1. Make individual query classes "immutable" for the most part. This removes the need for nullable fields while building the instances, making it clear which fields may actually be undefined.
  2. Change field names to camelCase for consistency.
  3. Remove some unused fields, and document the others.

@rkistner rkistner requested review from Copilot and simolus3 May 21, 2025 10:01
@changeset-bot
Copy link

changeset-bot bot commented May 21, 2025

🦋 Changeset detected

Latest commit: 7447e24

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 12 packages
Name Type
@powersync/service-sync-rules Minor
@powersync/service-module-postgres-storage Patch
@powersync/service-module-mongodb-storage Patch
@powersync/service-core-tests Patch
@powersync/service-core Patch
@powersync/service-module-mongodb Patch
@powersync/service-module-mysql Patch
@powersync/service-module-postgres Patch
@powersync/service-schema Patch
@powersync/service-image Patch
@powersync/service-module-core Patch
test-client Patch

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

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors the internal sync-rules implementation without altering core behavior, breaking the public API for the @powersync/service-sync-rules package to pave the way for larger upcoming changes.

  • Immutable query classes: constructors now accept fully initialized options objects, eliminating nullable build-time fields.
  • CamelCase renames: all previously snake_case properties (e.g. user_id, bucket_parameters) are now camelCase.
  • Cleanup: unused fields removed and remaining ones documented in interfaces.

Reviewed Changes

Copilot reviewed 28 out of 28 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
packages/sync-rules/src/request_functions.ts Renamed request parameter fields to camelCase
packages/sync-rules/src/events/SqlEventSourceQuery.ts Converted SqlEventSourceQuery.fromSql to return an immutable instance and use camelCase
packages/sync-rules/src/TableValuedFunctionSqlParameterQuery.ts Refactored to immutable TableValuedFunctionSqlParameterQuery with documented options
packages/sync-rules/src/StaticSqlParameterQuery.ts Refactored to immutable StaticSqlParameterQuery with documented options
packages/sync-rules/src/SqlParameterQuery.ts Refactored to immutable SqlParameterQuery, added options interface
packages/sync-rules/src/SqlDataQuery.ts Refactored to immutable SqlDataQuery, added options interface
packages/sync-rules/src/BaseSqlDataQuery.ts Introduced BaseSqlDataQueryOptions and documented all fields
packages/service-core/src/sync/sync.ts Renamed user_iduserId in the streaming sync logic
packages/service-core/src/sync/BucketChecksumState.ts Renamed user_iduserId in checkpoint state
packages/service-core-tests/src/tests/... Updated tests to match new camelCase properties
modules/module-postgres-storage/.../PostgresPersistedBatch.ts Updated storage inserts to use camelCase bucketParameters
modules/module-postgres-storage/.../PostgresBucketBatch.ts Switched from event_descriptors to eventDescriptors
modules/module-mongodb-storage/.../PersistedBatch.ts Updated persisted batch shape to use camelCase bucketParameters
modules/module-mongodb-storage/.../MongoBucketBatch.ts Switched from event_descriptors to eventDescriptors
.changeset/little-beans-carry.md Added a changeset for the version bumps
Comments suppressed due to low confidence (3)

packages/sync-rules/src/TableValuedFunctionSqlParameterQuery.ts:28

  • [nitpick] Using the property name "function" may be confusing since it's a JavaScript keyword; consider renaming it to something like "functionImpl" or "tableFunction" for clarity.
  function: TableValuedFunction;

modules/module-mongodb-storage/src/storage/implementation/PersistedBatch.ts:209

  • The emitted document uses a snake_case key "bucket_parameters" but the value is taken from camelCase bucketParameters. Verify this mapping matches your consumer expectations or standardize on one naming convention.
            bucket_parameters: result.bucketParameters

packages/sync-rules/src/SqlParameterQuery.ts:164

  • filter.inputParameters may be undefined if parsing failed; this direct access could throw. Consider guarding with filter.inputParameters? or ensuring the property is always initialized.
    const expandedParams = filter.inputParameters.filter((param) => param.expands);

Remove redundant field initializer

Co-authored-by: Copilot <[email protected]>
Copy link
Contributor

@simolus3 simolus3 left a comment

Choose a reason for hiding this comment

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

I agree that making the queries immutable is a nicer setup, so I'm happy with these changes (apart from a minor nit) 👍

@rkistner rkistner merged commit 5b39039 into main May 21, 2025
21 checks passed
@rkistner rkistner deleted the cleanup-sync-rules branch May 21, 2025 12:19
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.

3 participants