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

fix: Drizzle transactions not using lock context for React-Native #470

Merged
merged 3 commits into from
Jan 17, 2025

Conversation

Chriztiaan
Copy link
Contributor

@Chriztiaan Chriztiaan commented Jan 16, 2025

Description

As part of #463 locks were fixed for React-Native, which resulted in Drizzle transactions on RN projects to never be executed. This was due to the original Drizzle transaction implementation not using the correct lock context. I could confirm that this issue was happening for both RNQS and OPSqlite, but not for web.

Fix

PowerSyncSQLiteSession has been broken into PowerSyncSQLiteSession and PowerSyncSQLiteBaseSession, which ensures that a transaction uses the constrained LockContext instead of the db context (allowing transactions to be executed in the lock context).

This also makes room to eventually implement nested transactions/save points in PowerSyncSqliteBaseSession.

Test

To verify this works across the board, I tested the fix for RNQS, OPSqlite, and Web.


The source files have also been renamed to better match class names.

Copy link

changeset-bot bot commented Jan 16, 2025

🦋 Changeset detected

Latest commit: f783ecc

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

This PR includes changesets to release 1 package
Name Type
@powersync/drizzle-driver 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

@Chriztiaan Chriztiaan marked this pull request as ready for review January 16, 2025 09:21
Copy link
Collaborator

@stevensJourney stevensJourney 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 to me.

@Chriztiaan Chriztiaan merged commit 86a753f into main Jan 17, 2025
6 checks passed
@Chriztiaan Chriztiaan deleted the fix/drizzle-react-native-transactions branch January 17, 2025 19:18
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