Conversation
Moved SQLite schema metadata and comparison primitives into the Sqlite module, including SqliteSchema, SqliteIndex, eqSqliteIndex, and schema/snapshot readers. Updated local-first flow to use sqliteSchema naming and dependencies throughout Db, Evolu, Sync, and related helpers, and switched schema migration logic to the new ensureSqliteSchema/getEvoluSqliteSchema path. Added root-run abort cleanup for createSqlite to prevent leaked driver lifetimes and introduced testCreateSqlite plus testCreateCrdtMessage helpers to support the new Task-based construction flow.
Moved LeaderLock and LeaderLockDep from Platform to Task as concurrency primitives and added createInMemoryLeaderLock for runtimes without a cross-process lock manager. Updated React Native to use the shared in-memory leader lock and removed the platform-specific leader lock module and export. Updated web runtime to expose createLeaderLock from Task using the Web Locks API and switched the DB worker bootstrap to construct leaderLock via createLeaderLock.
Reworked common test helpers to create SQLite through Task-based deps and removed obsolete local-first test utilities. Updated Sqlite, Schema, Storage, Task, web, and react-native tests to use the new helper flow, added coverage for sqlite schema/index primitives and snapshot utilities, and added a regression test asserting sqlite is disposed on run abort. Extended concurrency coverage with createInMemoryLeaderLock tests and added integration-style Evolu test wiring for worker/sqlite interactions.
Replace the previous testCreateSqlite helper with a new testCreateRunWithSqlite that returns a Run preloaded with an in-memory Sqlite instance. Update Sqlite.ts to add the helper and adjust imports. Update tests and test deps to use the new helper (destructuring sqlite from run.deps or passing run.deps to helpers like getSqliteSchema/getSqliteSnapshot). Also update the changelog entry to reference the new helper name. This simplifies test setup by removing the extra step of creating a run and then adding sqlite separately.
Moved SQLite schema metadata and comparison primitives into the Sqlite module, including SqliteSchema, SqliteIndex, eqSqliteIndex, and schema/snapshot readers. Updated local-first flow to use sqliteSchema naming and dependencies throughout Db, Evolu, Sync, and related helpers, and switched schema migration logic to the new ensureSqliteSchema/getEvoluSqliteSchema path. Added root-run abort cleanup for createSqlite to prevent leaked driver lifetimes and introduced testCreateSqlite plus testCreateCrdtMessage helpers to support the new Task-based construction flow.
Moved LeaderLock and LeaderLockDep from Platform to Task as concurrency primitives and added createInMemoryLeaderLock for runtimes without a cross-process lock manager. Updated React Native to use the shared in-memory leader lock and removed the platform-specific leader lock module and export. Updated web runtime to expose createLeaderLock from Task using the Web Locks API and switched the DB worker bootstrap to construct leaderLock via createLeaderLock.
Reworked common test helpers to create SQLite through Task-based deps and removed obsolete local-first test utilities. Updated Sqlite, Schema, Storage, Task, web, and react-native tests to use the new helper flow, added coverage for sqlite schema/index primitives and snapshot utilities, and added a regression test asserting sqlite is disposed on run abort. Extended concurrency coverage with createInMemoryLeaderLock tests and added integration-style Evolu test wiring for worker/sqlite interactions.
Replace the previous testCreateSqlite helper with a new testCreateRunWithSqlite that returns a Run preloaded with an in-memory Sqlite instance. Update Sqlite.ts to add the helper and adjust imports. Update tests and test deps to use the new helper (destructuring sqlite from run.deps or passing run.deps to helpers like getSqliteSchema/getSqliteSnapshot). Also update the changelog entry to reference the new helper name. This simplifies test setup by removing the extra step of creating a run and then adding sqlite separately.
Align in-memory Worker/SharedWorker/MessageChannel fallbacks with native behavior by making message delivery asynchronous (macrotask-queued). Add flushScheduled to PortState and scheduleFlush using setTimeout(0) to flush queued messages. Introduce testWaitForMacrotask helper and update tests to await macrotask delivery. Replace exhaustiveCheck calls with console.error for unknown message types to avoid synchronous throws. Also refactor test SQLite deps (testCreateSqliteDeps) and update integration test setup for Evolu accordingly. Changes span Worker, Test, Shared, and many test files to reflect the new asynchronous semantics.
Introduce testWaitForWorkerMessage helper in Worker.ts to advance multi‑hop in-memory worker message delivery deterministically (two macrotask advances). Replace ad-hoc Promise.resolve / manual macrotask waits in multiple local-first tests with this helper. Adjust Evolu tests to queue evolu outputs until the evolu port is ready, await worker message propagation in many async tests, and update several tests to create/find test channels when directly invoking message handlers. These changes make worker-related test timing more reliable and simplify message flow handling in tests.
Change Evolu.exportDatabase from a Task/Deferred-based API to a plain Promise-based API. Replace Deferred usage with a Promise-with-resolvers (exportDatabasePending), reject the pending promise when the Evolu instance is disposed, and update Task imports accordingly. Update the minimal React example to use the new promise-based API and remove commented-out code. Adjust unit tests to reflect the new API and semantics (rename tests, assert rejection on dispose, verify shared pending export behavior and that run-wrapped aborts don't cancel the shared promise). Files updated: Evolu.ts, Evolu.test.ts, EvoluMinimalExample.tsx.
Add the 'coverage' folder to the package.json "clean" script so running the clean task also removes coverage artifacts (rimraf node_modules .turbo .eslintcache out bun.lock coverage). This prevents stale coverage data from persisting between runs.
Refactor createResources to accept a TimeDep (deps.time) so timeouts use deps.time.setTimeout/clearTimeout and TimeoutId. Change disposalDelay type from number to Duration (e.g. "100ms") and update related types/imports. Update call sites (local-first/Sync) to pass Millis/Duration and use the new curried signature (deps)({ config }). Expand and adapt Resources tests to use testCreateTime(), advance virtual time, and cover disposal scheduling, cancellation, disposal clearing, falsy resource values, default disposal delay, and various consumer/resource behaviors.
Align in-memory Worker/SharedWorker/MessageChannel fallbacks with native behavior by making message delivery asynchronous (macrotask-queued). Add flushScheduled to PortState and scheduleFlush using setTimeout(0) to flush queued messages. Introduce testWaitForMacrotask helper and update tests to await macrotask delivery. Replace exhaustiveCheck calls with console.error for unknown message types to avoid synchronous throws. Also refactor test SQLite deps (testCreateSqliteDeps) and update integration test setup for Evolu accordingly. Changes span Worker, Test, Shared, and many test files to reflect the new asynchronous semantics.
Introduce testWaitForWorkerMessage helper in Worker.ts to advance multi‑hop in-memory worker message delivery deterministically (two macrotask advances). Replace ad-hoc Promise.resolve / manual macrotask waits in multiple local-first tests with this helper. Adjust Evolu tests to queue evolu outputs until the evolu port is ready, await worker message propagation in many async tests, and update several tests to create/find test channels when directly invoking message handlers. These changes make worker-related test timing more reliable and simplify message flow handling in tests.
Change Evolu.exportDatabase from a Task/Deferred-based API to a plain Promise-based API. Replace Deferred usage with a Promise-with-resolvers (exportDatabasePending), reject the pending promise when the Evolu instance is disposed, and update Task imports accordingly. Update the minimal React example to use the new promise-based API and remove commented-out code. Adjust unit tests to reflect the new API and semantics (rename tests, assert rejection on dispose, verify shared pending export behavior and that run-wrapped aborts don't cancel the shared promise). Files updated: Evolu.ts, Evolu.test.ts, EvoluMinimalExample.tsx.
Refactor createResources to accept a TimeDep (deps.time) so timeouts use deps.time.setTimeout/clearTimeout and TimeoutId. Change disposalDelay type from number to Duration (e.g. "100ms") and update related types/imports. Update call sites (local-first/Sync) to pass Millis/Duration and use the new curried signature (deps)({ config }). Expand and adapt Resources tests to use testCreateTime(), advance virtual time, and cover disposal scheduling, cancellation, disposal clearing, falsy resource values, default disposal delay, and various consumer/resource behaviors.
miccy
added
upstream
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request refactors the SQLite integration to use throw-first semantics, adds comprehensive schema inspection capabilities, and improves cross-platform abstractions. The changes align with Evolu's coding guidelines and significantly improve code maintainability and test fidelity.
Changes:
- Refactored
SqliteandSqliteDriverAPIs to throw exceptions instead of wrapping errors inResult<..., SqliteError>, simplifying error handling throughout the codebase - Added SQLite schema metadata primitives (
SqliteSchema,SqliteIndex,getSqliteSchema,getSqliteSnapshot) for programmatic database inspection - Moved
LeaderLockfrom platform-specific files to common Task.ts withcreateInMemoryLeaderLock, and created platform-specificcreateLeaderLockin web package using Web Locks API - Changed in-memory worker message delivery from synchronous to asynchronous using macrotask timing for better test fidelity
- Updated
Resources.tsto useDurationstrings and injected time dependencies instead of raw milliseconds - Changed
Evolu.exportDatabasefromTasktoPromisewith proper rejection on disposal - Renamed
DbSchematoSqliteSchemaand related functions (ensureDbSchema→ensureSqliteSchema,getDbSchema→getSqliteSchema) - Added
testCreateRunWithSqlitehelper to streamline test setup
Reviewed changes
Copilot reviewed 35 out of 36 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/sync-guard-common-v8.mts | Added filtering logic to exclude active sync branch and temporary markers from unmerged branch reports |
| packages/web/src/Task.ts | Added createLeaderLock() factory using Web Locks API with in-memory fallback |
| packages/web/src/Platform.ts | Removed leaderLock export (moved to Task.ts) |
| packages/react-native/src/Platform.ts | Deleted file, React Native now uses createInMemoryLeaderLock |
| packages/react-native/src/shared.ts | Updated to use createInMemoryLeaderLock instead of platform-specific lock |
| packages/common/src/Worker.ts | Changed message delivery to asynchronous using setTimeout for better test fidelity |
| packages/common/src/Sqlite.ts | Added schema inspection APIs, testCreateRunWithSqlite helper, automatic disposal registration |
| packages/common/src/Resources.ts | Changed to use Duration strings and injected TimeDep for timeout management |
| packages/common/src/Task.ts | Added createInMemoryLeaderLock implementation using mutexes |
| packages/common/src/Platform.ts | Removed LeaderLock and LeaderLockDep interfaces (moved to Task.ts) |
| packages/common/src/local-first/Evolu.ts | Changed exportDatabase from Task to Promise, properly rejects on disposal |
| packages/common/src/local-first/Schema.ts | Renamed DbSchema to SqliteSchema, related functions updated |
| packages/common/src/local-first/Db.ts | Updated to use SqliteSchema and ensureSqliteSchema |
| packages/common/src/local-first/Sync.ts | Updated dependency types from DbSchemaDep to SqliteSchemaDep |
| packages/common/src/local-first/Storage.ts | Moved getNextStoredBytes function, reformatted comments |
| packages/common/src/local-first/Shared.ts | Replaced exhaustiveCheck with console.error for unknown message types |
| packages/common/test/* | Updated all tests for async worker message delivery using testWaitForWorkerMessage() |
| packages/common/test/_deps.ts | Refactored testCreateSqliteDeps for cleaner backward compatibility |
| package.json | Updated bun to 1.3.10, turbo to 2.8.11, added coverage cleanup to clean script |
| .changeset/sqlite-refactor.md | Updated changeset documentation with new schema inspection primitives |
7 tasks
Co-authored-by: miccy <9729864+miccy@users.noreply.github.com>
[WIP] Add changes based on review feedback for PR #51
|
Caution Review failedThe pull request is closed. ℹ️ Recent review infoConfiguration used: Organization UI Review profile: CHILL Plan: Free ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (35)
📝 WalkthroughWalkthroughTento pull request provádí komplexní refaktorování SQLite API, přesunutí leadership lock mehanismů a reorganizaci správy schémat. Zahrnuje změny v typech, eliminaci error wrappingu, asynchronní doručování zpráv a přidání nových veřejných primitiv pro schémata a testovací nástroje. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~55 minut Poem
Note 🎁 Summarized by CodeRabbit FreeThe PR author is not assigned a seat. To perform a comprehensive line-by-line review, please assign a seat to the pull request author through the subscription management page by visiting https://app.coderabbit.ai/login. Comment |
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 pull request introduces a major refactor of the SQLite integration, focusing on simplifying API semantics, improving schema inspection capabilities, and aligning cross-platform abstractions. The most important changes are grouped below by theme.
SQLite API Refactor and Schema Inspection
SqliteandSqliteDriverAPIs to use throw-first semantics: methods likeexecandexportnow return results directly instead of wrapped inResult<..., SqliteError>, andSqliteErrorplumbing is removed. The driver now determines read vs write internally, and protocol boundaries handle errors explicitly. [1] [2]SqliteSchema,SqliteIndex,eqSqliteIndex,getSqliteSchema, andgetSqliteSnapshot, enabling programmatic inspection and comparison of database schema and contents.testCreateRunWithSqlitehelper to streamline test setup with in-memory SQLite instances.Cross-Platform and Type Improvements
Platform.ts. [1] [2]Resources.tsto use duration strings (e.g.,"100ms","1s") and centralized timeout handling through dependency injection, improving consistency and flexibility. [1] [2] [3] [4] [5] [6]Dependency and Tooling Updates
package.json, includingbunandturbo, and updated thecleanscript to also remove coverage artifacts. [1] [2] [3]Miscellaneous Improvements
prettier-plugin-sql-cstfor consistent query formatting.These changes collectively simplify error handling, improve schema introspection, and modernize the codebase for easier maintenance and extensibility.
Summary by CodeRabbit
Poznámky k verzi
Nové funkce
Refactor