From 21333a2669538a8f8aa7d52162fe95c3596fb916 Mon Sep 17 00:00:00 2001 From: Christiaan Landman Date: Wed, 11 Mar 2026 10:03:07 +0200 Subject: [PATCH 1/2] Added `setupContext` option to CreateDiffTriggerOptions and a lock context to the cleanup function returned by createDiffTrigger, this allows you to dispose and recreate a diffTrigger inside a single writeLock. --- .changeset/quick-needles-cheat.md | 5 +++ .../src/client/triggers/TriggerManager.ts | 5 ++- .../src/client/triggers/TriggerManagerImpl.ts | 20 ++++++--- packages/node/tests/trigger.test.ts | 45 +++++++++++++++++++ packages/web/tests/triggers.test.ts | 6 +-- 5 files changed, 71 insertions(+), 10 deletions(-) create mode 100644 .changeset/quick-needles-cheat.md diff --git a/.changeset/quick-needles-cheat.md b/.changeset/quick-needles-cheat.md new file mode 100644 index 000000000..b46d0c3b7 --- /dev/null +++ b/.changeset/quick-needles-cheat.md @@ -0,0 +1,5 @@ +--- +'@powersync/common': minor +--- + +Added `setupContext` option to CreateDiffTriggerOptions and a lock context to the cleanup function returned by createDiffTrigger, this allows you to dispose and recreate a diffTrigger inside a single writeLock. diff --git a/packages/common/src/client/triggers/TriggerManager.ts b/packages/common/src/client/triggers/TriggerManager.ts index 6b73ce9c6..daa3b334a 100644 --- a/packages/common/src/client/triggers/TriggerManager.ts +++ b/packages/common/src/client/triggers/TriggerManager.ts @@ -223,14 +223,15 @@ export interface CreateDiffTriggerOptions extends BaseCreateDiffTriggerOptions { * This table will be dropped once the trigger is removed. */ destination: string; + + setupContext?: LockContext; } /** * @experimental * Callback to drop a trigger after it has been created. */ -export type TriggerRemoveCallback = () => Promise; - +export type TriggerRemoveCallback = (context?: LockContext) => Promise; /** * @experimental * Options for {@link TriggerDiffHandlerContext#withDiff}. diff --git a/packages/common/src/client/triggers/TriggerManagerImpl.ts b/packages/common/src/client/triggers/TriggerManagerImpl.ts index 6a6d7d00d..13d797af3 100644 --- a/packages/common/src/client/triggers/TriggerManagerImpl.ts +++ b/packages/common/src/client/triggers/TriggerManagerImpl.ts @@ -201,6 +201,7 @@ export class TriggerManagerImpl implements TriggerManager { columns, when, hooks, + setupContext, // Fall back to the provided default if not given on this level useStorage = this.defaultConfig.useStorageByDefault } = options; @@ -268,13 +269,18 @@ 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 () => { + const cleanup = async (context?: LockContext) => { disposeWarningListener(); - return this.db.writeLock(async (tx) => { + const doCleanup = async (tx: LockContext) => { await this.removeTriggers(tx, triggerIds); - await tx.execute(/* sql */ `DROP TABLE IF EXISTS ${destination};`); + await tx.execute(`DROP TABLE IF EXISTS ${destination};`); await releaseStorageClaim?.(); - }); + }; + if (context) { + await doCleanup(context); + } else { + await this.db.writeLock(doCleanup); + } }; const setup = async (tx: LockContext) => { @@ -360,7 +366,11 @@ export class TriggerManagerImpl implements TriggerManager { }; try { - await this.db.writeLock(setup); + if (setupContext) { + await setup(setupContext); + } else { + await this.db.writeLock(setup); + } return cleanup; } catch (error) { try { diff --git a/packages/node/tests/trigger.test.ts b/packages/node/tests/trigger.test.ts index eca9c6c9e..86456bd76 100644 --- a/packages/node/tests/trigger.test.ts +++ b/packages/node/tests/trigger.test.ts @@ -705,6 +705,51 @@ describe('Triggers', () => { ); }); + databaseTest('Recreating trigger with setupContext should not lose data', async ({ database }) => { + const destination = 'temp_recreate'; + const columns: Array = ['content']; + const when = { [DiffTriggerOperation.INSERT]: 'TRUE' }; + + let dispose = await database.triggers.createDiffTrigger({ + source: 'todos', + destination, + columns, + when + }); + + let gapTodos: Database['todos'][] = []; + + // Dispose the trigger (drops destination table + triggers) + await database.writeLock(async (tx) => { + await dispose(tx); + + // Create new trigger in the same lock context — no nested lock needed + dispose = await database.triggers.createDiffTrigger({ + source: 'todos', + destination, + columns, + when, + setupContext: tx + }); + }); + + // Verify new trigger works after locked context recreation + await database.execute("INSERT INTO todos (id, content) VALUES (uuid(), 'after recreate');"); + + await vi.waitFor( + async () => { + const rows = await database.writeLock(async (tx) => + tx.getAll(`SELECT * FROM ${destination}`) + ); + expect(rows.length).toBe(1); + expect(JSON.parse(rows[0].value).content).toBe('after recreate'); + }, + { timeout: 1000 } + ); + + await dispose(); + }); + databaseTest('Should use persisted tables and triggers', async ({ database }) => { const table = 'temp_remote_lists'; diff --git a/packages/web/tests/triggers.test.ts b/packages/web/tests/triggers.test.ts index 7a356d6a8..585879f5b 100644 --- a/packages/web/tests/triggers.test.ts +++ b/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')"); From 1e5eae66973387459f0bc0a694621e7367d1ef05 Mon Sep 17 00:00:00 2001 From: Christiaan Landman Date: Wed, 11 Mar 2026 12:32:14 +0200 Subject: [PATCH 2/2] Moved cleanup context to an options object. --- .changeset/quick-needles-cheat.md | 2 +- .../common/src/client/triggers/TriggerManager.ts | 14 +++++++++++++- .../src/client/triggers/TriggerManagerImpl.ts | 6 ++++-- packages/node/tests/trigger.test.ts | 6 ++---- 4 files changed, 20 insertions(+), 8 deletions(-) diff --git a/.changeset/quick-needles-cheat.md b/.changeset/quick-needles-cheat.md index b46d0c3b7..e12d30edf 100644 --- a/.changeset/quick-needles-cheat.md +++ b/.changeset/quick-needles-cheat.md @@ -2,4 +2,4 @@ '@powersync/common': minor --- -Added `setupContext` option to CreateDiffTriggerOptions and a lock context to the cleanup function returned by createDiffTrigger, this allows you to dispose and recreate a diffTrigger inside a single writeLock. +Added `setupContext` option to `CreateDiffTriggerOptions` and a lock context option to the cleanup function returned by `createDiffTrigger`, this allows you to create and dispose of a trigger inside of a lock context. diff --git a/packages/common/src/client/triggers/TriggerManager.ts b/packages/common/src/client/triggers/TriggerManager.ts index daa3b334a..b5eaa671d 100644 --- a/packages/common/src/client/triggers/TriggerManager.ts +++ b/packages/common/src/client/triggers/TriggerManager.ts @@ -224,14 +224,26 @@ export interface CreateDiffTriggerOptions extends BaseCreateDiffTriggerOptions { */ destination: string; + /** + * Context to use for the setup operation. + * This is useful for when the setup operation needs to be executed in a specific context. + */ setupContext?: LockContext; } +/** + * @experimental + * Options for {@link TriggerRemoveCallback}. + */ +export interface TriggerRemoveCallbackOptions { + context?: LockContext; +} + /** * @experimental * Callback to drop a trigger after it has been created. */ -export type TriggerRemoveCallback = (context?: LockContext) => Promise; +export type TriggerRemoveCallback = (options?: TriggerRemoveCallbackOptions) => Promise; /** * @experimental * Options for {@link TriggerDiffHandlerContext#withDiff}. diff --git a/packages/common/src/client/triggers/TriggerManagerImpl.ts b/packages/common/src/client/triggers/TriggerManagerImpl.ts index 13d797af3..379fdd1fd 100644 --- a/packages/common/src/client/triggers/TriggerManagerImpl.ts +++ b/packages/common/src/client/triggers/TriggerManagerImpl.ts @@ -9,6 +9,7 @@ import { TriggerManager, TriggerManagerConfig, TriggerRemoveCallback, + TriggerRemoveCallbackOptions, WithDiffOptions } from './TriggerManager.js'; @@ -269,7 +270,8 @@ 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 (context?: LockContext) => { + const cleanup = async (options?: TriggerRemoveCallbackOptions) => { + const { context } = options ?? {}; disposeWarningListener(); const doCleanup = async (tx: LockContext) => { await this.removeTriggers(tx, triggerIds); @@ -374,7 +376,7 @@ export class TriggerManagerImpl implements TriggerManager { return cleanup; } catch (error) { try { - await cleanup(); + await cleanup(setupContext ? { context: setupContext } : undefined); } catch (cleanupError) { throw new AggregateError([error, cleanupError], 'Error during operation and cleanup'); } diff --git a/packages/node/tests/trigger.test.ts b/packages/node/tests/trigger.test.ts index 86456bd76..5c677cefc 100644 --- a/packages/node/tests/trigger.test.ts +++ b/packages/node/tests/trigger.test.ts @@ -705,7 +705,7 @@ describe('Triggers', () => { ); }); - databaseTest('Recreating trigger with setupContext should not lose data', async ({ database }) => { + databaseTest('Recreating trigger with write lock on dispose and re-creation', async ({ database }) => { const destination = 'temp_recreate'; const columns: Array = ['content']; const when = { [DiffTriggerOperation.INSERT]: 'TRUE' }; @@ -717,11 +717,9 @@ describe('Triggers', () => { when }); - let gapTodos: Database['todos'][] = []; - // Dispose the trigger (drops destination table + triggers) await database.writeLock(async (tx) => { - await dispose(tx); + await dispose({ context: tx }); // Create new trigger in the same lock context — no nested lock needed dispose = await database.triggers.createDiffTrigger({