[Fix] SQL options (#252)

stevensJourney · web-flow · commit 32e342a5cb3a · 2024-07-29T13:00:53.000+02:00
diff --git a/.changeset/few-hounds-return.md b/.changeset/few-hounds-return.md
@@ -0,0 +1,6 @@
+---
+'@powersync/common': patch
+'@powersync/web': patch
+---
+
+Fix: correctly apply SQLOpen flags. This fixes an issue where `PowerSyncDatabase` constructor `flags` options were not used when opening SQLite connections in web.
diff --git a/demos/example-electron/package.json b/demos/example-electron/package.json
@@ -19,6 +19,8 @@
   },
   "license": "MIT",
   "dependencies": {
+    "@emotion/react": "^11.13.0",
+    "@emotion/styled": "^11.13.0",
     "@journeyapps/wa-sqlite": "~0.2.0",
     "@mui/icons-material": "^5.15.16",
     "@mui/material": "^5.15.16",
diff --git a/packages/common/src/client/AbstractPowerSyncDatabase.ts b/packages/common/src/client/AbstractPowerSyncDatabase.ts
@@ -14,8 +14,10 @@ import { SyncStatus } from '../db/crud/SyncStatus';
 import { UploadQueueStats } from '../db/crud/UploadQueueStatus';
 import { Schema } from '../db/schema/Schema';
 import { BaseObserver } from '../utils/BaseObserver';
+import { ControlledExecutor } from '../utils/ControlledExecutor';
 import { mutexRunExclusive } from '../utils/mutex';
 import { quoteIdentifier } from '../utils/strings';
+import { SQLOpenFactory, SQLOpenOptions, isDBAdapter, isSQLOpenFactory, isSQLOpenOptions } from './SQLOpenFactory';
 import { PowerSyncBackendConnector } from './connection/PowerSyncBackendConnector';
 import { BucketStorageAdapter, PSInternalTable } from './sync/bucket/BucketStorageAdapter';
 import { CrudBatch } from './sync/bucket/CrudBatch';
@@ -24,12 +26,10 @@ import { CrudTransaction } from './sync/bucket/CrudTransaction';
 import {
   AbstractStreamingSyncImplementation,
   DEFAULT_CRUD_UPLOAD_THROTTLE_MS,
-  StreamingSyncImplementationListener,
+  PowerSyncConnectionOptions,
   StreamingSyncImplementation,
-  PowerSyncConnectionOptions
+  StreamingSyncImplementationListener
 } from './sync/stream/AbstractStreamingSyncImplementation';
-import { ControlledExecutor } from '../utils/ControlledExecutor';
-import { SQLOpenFactory, SQLOpenOptions, isDBAdapter, isSQLOpenFactory, isSQLOpenOptions } from './SQLOpenFactory';
 
 export interface DisconnectAndClearOptions {
   /** When set to false, data in local-only tables is preserved. */
@@ -139,6 +139,14 @@ export const DEFAULT_POWERSYNC_DB_OPTIONS = {
  */
 export const DEFAULT_LOCK_TIMEOUT_MS = 120_000; // 2 mins
 
+/**
+ * Tests if the input is a {@link PowerSyncDatabaseOptionsWithSettings}
+ * @internal
+ */
+export const isPowerSyncDatabaseOptionsWithSettings = (test: any): test is PowerSyncDatabaseOptionsWithSettings => {
+  return typeof test == 'object' && isSQLOpenOptions(test.database);
+};
+
 export abstract class AbstractPowerSyncDatabase extends BaseObserver<PowerSyncDBListener> {
   /**
    * Transactions should be queued in the DBAdapter, but we also want to prevent
@@ -182,8 +190,8 @@ export abstract class AbstractPowerSyncDatabase extends BaseObserver<PowerSyncDB
       this._database = database;
     } else if (isSQLOpenFactory(database)) {
       this._database = database.openDB();
-    } else if (isSQLOpenOptions(database)) {
-      this._database = this.openDBAdapter(database);
+    } else if (isPowerSyncDatabaseOptionsWithSettings(options)) {
+      this._database = this.openDBAdapter(options);
     }
 
     this.bucketStorageAdapter = this.generateBucketStorageAdapter();
@@ -223,7 +231,7 @@ export abstract class AbstractPowerSyncDatabase extends BaseObserver<PowerSyncDB
   /**
    * Opens the DBAdapter given open options using a default open factory
    */
-  protected abstract openDBAdapter(options: SQLOpenOptions): DBAdapter;
+  protected abstract openDBAdapter(options: PowerSyncDatabaseOptionsWithSettings): DBAdapter;
 
   protected abstract generateSyncStreamImplementation(
     connector: PowerSyncBackendConnector
diff --git a/packages/react-native/src/db/PowerSyncDatabase.ts b/packages/react-native/src/db/PowerSyncDatabase.ts
@@ -1,11 +1,11 @@
 import {
   AbstractPowerSyncDatabase,
   AbstractStreamingSyncImplementation,
-  PowerSyncBackendConnector,
-  SqliteBucketStorage,
   BucketStorageAdapter,
   DBAdapter,
-  SQLOpenOptions
+  PowerSyncBackendConnector,
+  PowerSyncDatabaseOptionsWithSettings,
+  SqliteBucketStorage
 } from '@powersync/common';
 import { ReactNativeRemote } from '../sync/stream/ReactNativeRemote';
 import { ReactNativeStreamingSyncImplementation } from '../sync/stream/ReactNativeStreamingSyncImplementation';
@@ -32,8 +32,8 @@ export class PowerSyncDatabase extends AbstractPowerSyncDatabase {
    * Opens a DBAdapter using React Native Quick SQLite as the
    * default SQLite open factory.
    */
-  protected openDBAdapter(options: SQLOpenOptions): DBAdapter {
-    const defaultFactory = new ReactNativeQuickSqliteOpenFactory(options);
+  protected openDBAdapter(options: PowerSyncDatabaseOptionsWithSettings): DBAdapter {
+    const defaultFactory = new ReactNativeQuickSqliteOpenFactory(options.database);
     return defaultFactory.openDB();
   }
 
diff --git a/packages/web/src/db/PowerSyncDatabase.ts b/packages/web/src/db/PowerSyncDatabase.ts
@@ -1,29 +1,28 @@
 import {
   type AbstractStreamingSyncImplementation,
-  type PowerSyncBackendConnector,
   type BucketStorageAdapter,
+  type PowerSyncBackendConnector,
   type PowerSyncCloseOptions,
   type PowerSyncConnectionOptions,
   AbstractPowerSyncDatabase,
-  SqliteBucketStorage,
-  DEFAULT_POWERSYNC_CLOSE_OPTIONS,
   DBAdapter,
-  SQLOpenOptions,
+  DEFAULT_POWERSYNC_CLOSE_OPTIONS,
+  PowerSyncDatabaseOptions,
   PowerSyncDatabaseOptionsWithDBAdapter,
   PowerSyncDatabaseOptionsWithOpenFactory,
   PowerSyncDatabaseOptionsWithSettings,
-  PowerSyncDatabaseOptions
+  SqliteBucketStorage
 } from '@powersync/common';
 import { Mutex } from 'async-mutex';
-import { WebRemote } from './sync/WebRemote';
+import { WASQLiteOpenFactory } from './adapters/wa-sqlite/WASQLiteOpenFactory';
+import { DEFAULT_WEB_SQL_FLAGS, resolveWebSQLFlags, WebSQLFlags } from './adapters/web-sql-flags';
 import { SharedWebStreamingSyncImplementation } from './sync/SharedWebStreamingSyncImplementation';
 import { SSRStreamingSyncImplementation } from './sync/SSRWebStreamingSyncImplementation';
+import { WebRemote } from './sync/WebRemote';
 import {
   WebStreamingSyncImplementation,
   WebStreamingSyncImplementationOptions
 } from './sync/WebStreamingSyncImplementation';
-import { WASQLiteOpenFactory } from './adapters/wa-sqlite/WASQLiteOpenFactory';
-import { DEFAULT_WEB_SQL_FLAGS, resolveWebSQLFlags, WebSQLFlags } from './adapters/web-sql-flags';
 
 export interface WebPowerSyncFlags extends WebSQLFlags {
   /**
@@ -92,8 +91,11 @@ export class PowerSyncDatabase extends AbstractPowerSyncDatabase {
 
   async _initialize(): Promise<void> {}
 
-  protected openDBAdapter(options: SQLOpenOptions): DBAdapter {
-    const defaultFactory = new WASQLiteOpenFactory({ ...options, flags: this.resolvedFlags });
+  protected openDBAdapter(options: WebPowerSyncDatabaseOptionsWithSettings): DBAdapter {
+    const defaultFactory = new WASQLiteOpenFactory({
+      ...options.database,
+      flags: resolveWebPowerSyncFlags(options.flags)
+    });
     return defaultFactory.openDB();
   }
 
diff --git a/packages/web/tests/open.test.ts b/packages/web/tests/open.test.ts
@@ -1,11 +1,11 @@
-import { describe, expect, it } from 'vitest';
 import { AbstractPowerSyncDatabase } from '@powersync/common';
 import {
   PowerSyncDatabase,
   WASQLiteDBAdapter,
-  WASQLitePowerSyncDatabaseOpenFactory,
-  WASQLiteOpenFactory
+  WASQLiteOpenFactory,
+  WASQLitePowerSyncDatabaseOpenFactory
 } from '@powersync/web';
+import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest';
 import { testSchema } from './utils/testDb';
 
 const testId = '2290de4f-0488-4e50-abed-f8e8eb1d0b42';
@@ -18,6 +18,45 @@ export const basicTest = async (db: AbstractPowerSyncDatabase) => {
 };
 
 describe('Open Methods', () => {
+  let originalSharedWorker: typeof SharedWorker;
+  let originalWorker: typeof Worker;
+
+  const sharedWorkerProxyHandler = {
+    construct(target: typeof SharedWorker, args: any[]) {
+      const [url, options] = args;
+
+      // Call the original constructor
+      const instance = new target(url, options);
+      return instance;
+    }
+  };
+  const workerProxyHandler = {
+    construct(target: typeof Worker, args: any[]) {
+      const [url, options] = args;
+
+      // Call the original constructor
+      const instance = new target(url, options);
+      return instance;
+    }
+  };
+
+  beforeAll(() => {
+    // Store the original SharedWorker constructor
+    originalSharedWorker = SharedWorker;
+    originalWorker = Worker;
+
+    // Create a proxy to intercept the worker constructors
+    // The vi.SpyOn does not work well with constructors
+    window.SharedWorker = new Proxy(SharedWorker, sharedWorkerProxyHandler);
+    window.Worker = new Proxy(Worker, workerProxyHandler);
+  });
+
+  afterAll(() => {
+    // Restore Worker
+    window.SharedWorker = originalSharedWorker;
+    window.Worker = originalWorker;
+  });
+
   it('Should open PowerSync clients from old factory methods', async () => {
     const db = new WASQLitePowerSyncDatabaseOpenFactory({
       dbFilename: `test-legacy.db`,
@@ -46,4 +85,46 @@ describe('Open Methods', () => {
 
     await basicTest(db);
   });
+
+  it('Should use shared worker for multiple tabs', async () => {
+    const sharedSpy = vi.spyOn(sharedWorkerProxyHandler, 'construct');
+
+    const db = new PowerSyncDatabase({ database: { dbFilename: 'options-test.db' }, schema: testSchema });
+
+    await basicTest(db);
+
+    expect(sharedSpy).toBeCalledTimes(1);
+  });
+
+  it('Should use dedicated worker when multiple tabs disabled', async () => {
+    const sharedSpy = vi.spyOn(sharedWorkerProxyHandler, 'construct');
+    const dedicatedSpy = vi.spyOn(workerProxyHandler, 'construct');
+
+    const db = new PowerSyncDatabase({
+      database: { dbFilename: 'options-test.db' },
+      schema: testSchema,
+      flags: { enableMultiTabs: false }
+    });
+
+    await basicTest(db);
+
+    expect(sharedSpy).toBeCalledTimes(0);
+    expect(dedicatedSpy).toBeCalledTimes(1);
+  });
+
+  it('Should not use workers when specified', async () => {
+    const sharedSpy = vi.spyOn(sharedWorkerProxyHandler, 'construct');
+    const dedicatedSpy = vi.spyOn(workerProxyHandler, 'construct');
+
+    const db = new PowerSyncDatabase({
+      database: { dbFilename: 'options-test.db' },
+      schema: testSchema,
+      flags: { useWebWorker: false }
+    });
+
+    await basicTest(db);
+
+    expect(sharedSpy).toBeCalledTimes(0);
+    expect(dedicatedSpy).toBeCalledTimes(0);
+  });
 });
diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml
