Switched to manageDestinationExternally option instead, escaping all internal management of the destination table.

Author: Chriztiaan

packages/common/src/client/triggers/TriggerManager.ts

@@ -225,18 +225,18 @@ export interface CreateDiffTriggerOptions extends BaseCreateDiffTriggerOptions {
   destination: string;
 
   /**
-   * The destination table persists beyond this trigger's lifetime and is not automatically dropped when the trigger is removed.
-   * Additionally, if the trigger already exists with the same destination, it will be reused instead of failing with a name conflict error.
+   * When true, the diff trigger will not create or drop the destination table.
+   * The caller is responsible for ensuring the table exists with the correct
+   * schema before creating the trigger and for dropping it when no longer needed.
    */
-  persistDestination?: boolean;
+  manageDestinationExternally?: boolean;
 }
 
 /**
  * @experimental
  * Callback to drop a trigger after it has been created.
- * When invoked with force=true, it will also drop the destination table even if `persistDestination` was true.
  */
-export type TriggerRemoveCallback = (force?: boolean) => Promise<void>;
+export type TriggerRemoveCallback = () => Promise<void>;
 
 /**
  * @experimental

packages/common/src/client/triggers/TriggerManagerImpl.ts

@@ -201,7 +201,7 @@ export class TriggerManagerImpl implements TriggerManager {
       columns,
       when,
       hooks,
-      persistDestination = false,
+      manageDestinationExternally = false,
       // Fall back to the provided default if not given on this level
       useStorage = this.defaultConfig.useStorageByDefault
     } = options;
@@ -269,11 +269,11 @@ export class TriggerManagerImpl implements TriggerManager {
      * we need to ensure we can cleanup the created resources.
      * We unfortunately cannot rely on transaction rollback.
      */
-    const cleanup = async (force?: boolean) => {
+    const cleanup = async () => {
       disposeWarningListener();
       return this.db.writeLock(async (tx) => {
         await this.removeTriggers(tx, triggerIds);
-        if (!persistDestination || force) {
+        if (!manageDestinationExternally) {
           await tx.execute(/* sql */ `DROP TABLE IF EXISTS ${destination};`);
         }
         await releaseStorageClaim?.();
@@ -283,16 +283,18 @@ export class TriggerManagerImpl implements TriggerManager {
     const setup = async (tx: LockContext) => {
       // Allow user code to execute in this lock context before the trigger is created.
       await hooks?.beforeCreate?.(tx);
-      await tx.execute(/* sql */ `
-        CREATE ${tableTriggerTypeClause} TABLE ${persistDestination ? 'IF NOT EXISTS ' : ''}${destination} (
-          operation_id INTEGER PRIMARY KEY AUTOINCREMENT,
-          id TEXT,
-          operation TEXT,
-          timestamp TEXT,
-          value TEXT,
-          previous_value TEXT
-        )
-      `);
+      if (!manageDestinationExternally) {
+        await tx.execute(/* sql */ `
+          CREATE ${tableTriggerTypeClause} TABLE ${destination} (
+            operation_id INTEGER PRIMARY KEY AUTOINCREMENT,
+            id TEXT,
+            operation TEXT,
+            timestamp TEXT,
+            value TEXT,
+            previous_value TEXT
+          )
+        `);
+      }
 
       if (operations.includes(DiffTriggerOperation.INSERT)) {
         const insertTriggerId = this.generateTriggerName(DiffTriggerOperation.INSERT, destination, id);
@@ -471,9 +473,9 @@ export class TriggerManagerImpl implements TriggerManager {
         hooks
       });
 
-      return async (force?: boolean) => {
+      return async () => {
         abortOnChange();
-        await removeTrigger(force);
+        await removeTrigger();
       };
     } catch (error) {
       try {

packages/node/tests/trigger.test.ts

@@ -616,12 +616,24 @@ describe('Triggers', () => {
   databaseTest('persistDestination: should not drop destination table on dispose', async ({ database }) => {
     const table = 'persist_dest_dispose_test';
 
+    // Manually create the destination table (simulates a table that persisted from a prior session)
+    await database.execute(`
+        CREATE TABLE ${table} (
+          operation_id INTEGER PRIMARY KEY AUTOINCREMENT,
+          id TEXT,
+          operation TEXT,
+          timestamp TEXT,
+          value TEXT,
+          previous_value TEXT
+        )
+      `);
+
     const dispose = await database.triggers.createDiffTrigger({
       source: 'todos',
       destination: table,
       when: { [DiffTriggerOperation.INSERT]: 'TRUE' },
       useStorage: true, // persistent table so we can verify via sqlite_master
-      persistDestination: true
+      manageDestinationExternally: true
     });
 
     // Table must exist before dispose
@@ -634,23 +646,20 @@ describe('Triggers', () => {
     await dispose();
 
     // Table must STILL exist — currently FAILS (impl drops the table unconditionally)
-    rows = await database.getAll<{ name: string }>(
-      `SELECT name FROM sqlite_master WHERE type='table' AND name = ?`,
-      [table]
-    );
+    rows = await database.getAll<{ name: string }>(`SELECT name FROM sqlite_master WHERE type='table' AND name = ?`, [
+      table
+    ]);
     expect(rows.length).toEqual(1);
 
     // Manual cleanup so the test doesn't leak
     await database.execute(`DROP TABLE IF EXISTS ${table}`);
   });
 
-  databaseTest(
-    'persistDestination: should allow reusing an existing destination table',
-    async ({ database }) => {
-      const table = 'persist_dest_reuse_test';
+  databaseTest('persistDestination: should allow reusing an existing destination table', async ({ database }) => {
+    const table = 'persist_dest_reuse_test';
 
-      // Manually create the destination table (simulates a table that persisted from a prior session)
-      await database.execute(`
+    // Manually create the destination table (simulates a table that persisted from a prior session)
+    await database.execute(`
         CREATE TABLE ${table} (
           operation_id INTEGER PRIMARY KEY AUTOINCREMENT,
           id TEXT,
@@ -661,22 +670,21 @@ describe('Triggers', () => {
         )
       `);
 
-      // Must NOT throw even though the table already exists.
-      // Currently FAILS — impl runs bare CREATE TABLE which SQLite rejects with "table already exists".
-      const dispose = await database.triggers.createDiffTrigger({
-        source: 'todos',
-        destination: table,
-        when: { [DiffTriggerOperation.INSERT]: 'TRUE' },
-        useStorage: true,
-        persistDestination: true
-      });
+    // Must NOT throw even though the table already exists.
+    // Currently FAILS — impl runs bare CREATE TABLE which SQLite rejects with "table already exists".
+    const dispose = await database.triggers.createDiffTrigger({
+      source: 'todos',
+      destination: table,
+      when: { [DiffTriggerOperation.INSERT]: 'TRUE' },
+      useStorage: true,
+      manageDestinationExternally: true
+    });
 
-      await dispose();
+    await dispose();
 
-      // Manual cleanup
-      await database.execute(`DROP TABLE IF EXISTS ${table}`);
-    }
-  );
+    // Manual cleanup
+    await database.execute(`DROP TABLE IF EXISTS ${table}`);
+  });
 
   databaseTest('Should cast operation_id as string with withDiff option', async ({ database }) => {
     const results: TriggerDiffRecord<string>[] = [];

packages/web/tests/triggers.test.ts

@@ -55,7 +55,7 @@ describe('Triggers', () => {
       }
     });
 
-    onTestFinished(() => disposeTrigger());
+    onTestFinished(disposeTrigger);
 
     await db.execute("INSERT INTO customers (id, name) VALUES (uuid(), 'test')");
 
@@ -103,7 +103,7 @@ describe('Triggers', () => {
       }
     });
 
-    onTestFinished(() => disposeTrigger());
+    onTestFinished(disposeTrigger);
 
     await db.execute("INSERT INTO customers (id, name) VALUES (uuid(), 'test')");
 
@@ -221,7 +221,7 @@ describe('Triggers', () => {
       }
     });
 
-    onTestFinished(() => disposeTrigger());
+    onTestFinished(disposeTrigger);
 
     // Perform an insert from client B
     await dbB.execute("INSERT INTO customers (id, name) VALUES (uuid(), 'from-client-b')");