feat (client): renamed disableFKs flag and modified default to disabl…

…e FKs on SQLite (#1376)

Decided to go for an enumeration as this is more explicit than the
`boolean | undefined`:
```ts
enum ForeignKeyChecks {
  enabled = 'enabled',
  disabled = 'disabled',
  inherit = 'inherit',
}
```

.changeset/fast-fishes-hunt.md

@@ -0,0 +1,5 @@
+---
+"electric-sql": patch
+---
+
+Modify FK flag option to default to disabling FK checks on SQLite.

clients/typescript/src/config/index.ts

@@ -43,10 +43,17 @@ export interface ElectricConfig {
    */
   connectionBackOffOptions?: ConnectionBackOffOptions
   /**
-   * Whether to disable FK checks when applying downstream (i.e. incoming) transactions to the local SQLite database.
-   * When using Postgres, this is the default behavior and can't be changed.
+   * Whether to check foreign keys when applying downstream (i.e. incoming) transactions to the local SQLite database.
+   * Defaults to `disabled`, meaning that FKs are not checked.
+   * When using Postgres, this option cannot be changed.
    */
-  disableForeignKeysDownstream?: boolean
+  foreignKeyChecksDownstream?: ForeignKeyChecks
+}
+
+export enum ForeignKeyChecks {
+  enabled = 'enabled',
+  disabled = 'disabled',
+  inherit = 'inherit',
 }
 
 export type ElectricConfigWithDialect = ElectricConfig & {
@@ -66,7 +73,7 @@ export type HydratedConfig = {
   debug: boolean
   connectionBackOffOptions: ConnectionBackOffOptions
   namespace: string
-  disableFKs: boolean | undefined
+  fkChecks: ForeignKeyChecks
 }
 
 export type InternalElectricConfig = {
@@ -79,7 +86,7 @@ export type InternalElectricConfig = {
   }
   debug?: boolean
   connectionBackOffOptions?: ConnectionBackOffOptions
-  disableFKs?: boolean
+  fkChecks: ForeignKeyChecks
 }
 
 export const hydrateConfig = (
@@ -99,6 +106,9 @@ export const hydrateConfig = (
 
   const defaultNamespace = config.dialect === 'Postgres' ? 'public' : 'main'
 
+  const fkChecks =
+    config.foreignKeyChecksDownstream ?? ForeignKeyChecks.disabled
+
   const replication = {
     host: url.hostname,
     port: port,
@@ -133,6 +143,6 @@ export const hydrateConfig = (
     debug,
     connectionBackOffOptions,
     namespace: defaultNamespace,
-    disableFKs: config.disableForeignKeysDownstream,
+    fkChecks,
   }
 }

clients/typescript/src/migrators/bundle.ts

@@ -13,6 +13,7 @@ import { _electric_migrations } from '../satellite/config'
 import { pgBuilder, QueryBuilder, sqliteBuilder } from './query-builder'
 import { dedent } from 'ts-dedent'
 import { runInTransaction } from '../util/transactions'
+import { ForeignKeyChecks } from '../config'
 
 export const SCHEMA_VSN_ERROR_MSG = `Local schema doesn't match server's. Clear local state through developer tools and retry connection manually. If error persists, re-generate the client. Check documentation (https://electric-sql.com/docs/reference/roadmap) to learn more.`
 
@@ -133,15 +134,15 @@ export abstract class BundleMigratorBase implements Migrator {
 
   async apply(
     { statements, version }: StmtMigration,
-    disableFKs?: boolean
+    fkChecks: ForeignKeyChecks = ForeignKeyChecks.inherit
   ): Promise<void> {
     if (!VALID_VERSION_EXP.test(version)) {
       throw new Error(
         `Invalid migration version, must match ${VALID_VERSION_EXP}`
       )
     }
 
-    await runInTransaction(this.adapter, disableFKs, ...statements, {
+    await runInTransaction(this.adapter, fkChecks, ...statements, {
       sql: dedent`
         INSERT INTO ${this.migrationsTable} (version, applied_at)
         VALUES (${this.queryBuilder.makePositionalParam(
@@ -160,7 +161,7 @@ export abstract class BundleMigratorBase implements Migrator {
    */
   async applyIfNotAlready(
     migration: StmtMigration,
-    disableFKs: boolean | undefined
+    fkChecks: ForeignKeyChecks = ForeignKeyChecks.inherit
   ): Promise<boolean> {
     const rows = await this.adapter.query({
       sql: dedent`
@@ -175,7 +176,7 @@ export abstract class BundleMigratorBase implements Migrator {
     if (shouldApply) {
       // This is a new migration because its version number
       // is not in our migrations table.
-      await this.apply(migration, disableFKs)
+      await this.apply(migration, fkChecks)
     }
 
     return shouldApply

clients/typescript/src/migrators/index.ts

@@ -1,3 +1,4 @@
+import { ForeignKeyChecks } from '../config'
 import { Statement } from '../util'
 import { QueryBuilder } from './query-builder'
 
@@ -29,10 +30,10 @@ export function makeStmtMigration(migration: Migration): StmtMigration {
 
 export interface Migrator {
   up(): Promise<number>
-  apply(migration: StmtMigration, disableFKs?: boolean): Promise<void>
+  apply(migration: StmtMigration, fkChecks: ForeignKeyChecks): Promise<void>
   applyIfNotAlready(
     migration: StmtMigration,
-    disableFKs: boolean | undefined
+    fkChecks: ForeignKeyChecks
   ): Promise<boolean>
   querySchemaVersion(): Promise<string | undefined>
   queryBuilder: QueryBuilder

clients/typescript/src/migrators/mock.ts

@@ -1,3 +1,4 @@
+import { ForeignKeyChecks } from '../config'
 import { Migrator, StmtMigration } from './index'
 import { QueryBuilder } from './query-builder'
 
@@ -8,13 +9,13 @@ export class MockMigrator implements Migrator {
     return 1
   }
 
-  async apply(_: StmtMigration, _disableFks?: boolean): Promise<void> {
+  async apply(_: StmtMigration, _fkChecks: ForeignKeyChecks): Promise<void> {
     return
   }
 
   async applyIfNotAlready(
     _: StmtMigration,
-    _disableFks: boolean | undefined
+    _fkChecks: ForeignKeyChecks
   ): Promise<boolean> {
     return true
   }

clients/typescript/src/satellite/config.ts

@@ -1,5 +1,6 @@
 import { IBackOffOptions } from 'exponential-backoff'
 import { QualifiedTablename } from '../util/tablename'
+import { ForeignKeyChecks } from '../config'
 
 export type ConnectionBackoffOptions = Omit<IBackOffOptions, 'retry'>
 export interface SatelliteOpts {
@@ -25,11 +26,12 @@ export interface SatelliteOpts {
   /** Backoff options for connecting with Electric*/
   connectionBackOffOptions: ConnectionBackoffOptions
   /**
-   * Whether to disable FK checks when applying incoming (i.e. remote) transactions to the local SQLite database.
-   * When using Postgres, this is the default behavior and can't be changed.
-   * If this flag is undefined and we're running on SQLite, then the FK pragma is left untouched.
+   * Whether to enable or disable FK checks when applying incoming (i.e. remote) transactions to the local SQLite database.
+   * When set to `inherit` the FK pragma is left untouched.
+   * This option defaults to `disable` which disables FK checks on incoming transactions.
+   * This option only affects FK checks on SQLite databases and should not be modified when using Postgres.
    */
-  disableFKs: boolean | undefined
+  fkChecks: ForeignKeyChecks
   /** With debug mode enabled, Satellite can show additional logs. */
   debug: boolean
 }
@@ -76,7 +78,7 @@ export const satelliteDefaults: (namespace: string) => SatelliteOpts = (
       numOfAttempts: 50,
       timeMultiple: 2,
     },
-    disableFKs: undefined,
+    fkChecks: ForeignKeyChecks.disabled,
     debug: false,
   }
 }

clients/typescript/src/satellite/process.ts

@@ -79,6 +79,7 @@ import groupBy from 'lodash.groupby'
 import { ShapeManager } from './shapes/shapeManager'
 import { SyncStatus } from '../client/model/shapes'
 import { runInTransaction } from '../util/transactions'
+import { ForeignKeyChecks } from '../config'
 
 type ChangeAccumulator = {
   [key: string]: Change
@@ -125,7 +126,7 @@ export class SatelliteProcess implements Satellite {
   builder: QueryBuilder
 
   opts: SatelliteOpts
-  disableFKs: boolean | undefined
+  fkChecks: ForeignKeyChecks
 
   _authState?: AuthState
   _unsubscribeFromAuthState?: UnsubscribeFunction
@@ -175,8 +176,10 @@ export class SatelliteProcess implements Satellite {
     this.builder = this.migrator.queryBuilder
 
     this.opts = opts
-    this.disableFKs =
-      this.builder.dialect === 'SQLite' ? this.opts.disableFKs : undefined
+    this.fkChecks =
+      this.builder.dialect === 'SQLite'
+        ? this.opts.fkChecks
+        : ForeignKeyChecks.inherit
     this.relations = {}
 
     this.previousShapeSubscriptions = []
@@ -695,11 +698,11 @@ export class SatelliteProcess implements Satellite {
   }
 
   /**
-   * Runs the provided statements in a transaction and disables FK checks if `disableFKs` is true.
-   * `disableFKs` should only be set to true when using SQLite as we already disable FK checks for incoming TXs when using Postgres
+   * Runs the provided statements in a transaction and disables FK checks if `this.fkChecks` is set to `disabled`.
+   * `this.fkChecks` should only be set to true when using SQLite as we already disable FK checks for incoming TXs when using Postgres
    */
   async runInTransaction(...stmts: Statement[]) {
-    return runInTransaction(this.adapter, this.disableFKs, ...stmts)
+    return runInTransaction(this.adapter, this.fkChecks, ...stmts)
   }
 
   _resetClientState(opts?: { keepSubscribedShapes: boolean }): Promise<void> {
@@ -1515,7 +1518,7 @@ export class SatelliteProcess implements Satellite {
           statements: allStatements,
           version: transaction.migrationVersion,
         },
-        this.disableFKs
+        this.fkChecks
       )
     } else {
       await this.runInTransaction(...allStatements)

clients/typescript/src/satellite/registry.ts

@@ -220,7 +220,7 @@ export class GlobalRegistry extends BaseRegistry {
     const satelliteOpts: SatelliteOpts = {
       ...satelliteDefaults(config.namespace),
       connectionBackOffOptions: config.connectionBackOffOptions,
-      disableFKs: config.disableFKs,
+      fkChecks: config.fkChecks,
       debug: config.debug,
     }
 

clients/typescript/src/util/transactions.ts

@@ -1,24 +1,26 @@
 import { Statement } from '.'
+import { ForeignKeyChecks } from '../config'
 import { DatabaseAdapter, RunResult } from '../electric'
 
 /**
- * Runs the provided statements in a transaction and disables FK checks if `disableFKs` is true.
- * FK checks are enabled if `disableFKs` is false.
- * FK checks are left untouched if `disableFKs` is undefined.
- * `disableFKs` should only be set to true when using SQLite as we already disable FK checks for incoming TXs when using Postgres,
+ * Runs the provided statements in a transaction and sets the `foreign_keys` pragma based on the `fkChecks` flag.
+ * FK checks are enabled if `fkChecks` is `ForeignKeyChecks.enabled`.
+ * FK checks are disabled if `fkChecks` is `ForeignKeyChecks.disabled`.
+ * FK checks are left untouched if `fkChecks` is `ForeignKeyChecks.inherit`.
+ * `fkChecks` should only be set to `ForeignKeyChecks.disabled` when using SQLite as we already disable FK checks for incoming TXs when using Postgres,
  * so the executed SQL code to disable FKs is for SQLite dialect only.
  */
 export async function runInTransaction(
   adapter: DatabaseAdapter,
-  disableFKs: boolean | undefined,
+  fkChecks: ForeignKeyChecks,
   ...stmts: Statement[]
 ): Promise<RunResult> {
-  if (disableFKs === undefined) {
+  if (fkChecks === ForeignKeyChecks.inherit) {
     // don't touch the FK pragma
     return adapter.runInTransaction(...stmts)
   }
 
-  const desiredPragma = disableFKs ? 0 : 1
+  const desiredPragma = fkChecks === ForeignKeyChecks.disabled ? 0 : 1
 
   return adapter.runExclusively(async (uncoordinatedAdapter) => {
     const [{ foreign_keys: originalPragma }] = await uncoordinatedAdapter.query(

clients/typescript/test/satellite/process.ts

@@ -53,6 +53,7 @@ import {
 } from '../../src/notifiers'
 import { QueryBuilder } from '../../src/migrators/query-builder'
 import { SatelliteOpts } from '../../src/satellite/config'
+import { ForeignKeyChecks } from '../../src/config'
 
 export type ContextType = CommonContextType & {
   builder: QueryBuilder
@@ -1164,6 +1165,7 @@ export const processTests = (test: TestFn<ContextType>) => {
     await runMigrations()
 
     if (builder.dialect === 'SQLite') {
+      satellite.fkChecks = ForeignKeyChecks.inherit // set FK checks to inherit because by default they are disabled
       await adapter.run({ sql: `PRAGMA foreign_keys = ON` })
     }
     await satellite._setMeta('compensations', 0)
@@ -1297,6 +1299,7 @@ export const processTests = (test: TestFn<ContextType>) => {
     await runMigrations()
 
     if (builder.dialect === 'SQLite') {
+      satellite.fkChecks = ForeignKeyChecks.inherit // set FK checks to inherit because by default they are disabled
       await adapter.run({ sql: `PRAGMA foreign_keys = ON` })
     }
     await satellite._setMeta('compensations', 0)
@@ -1966,6 +1969,11 @@ export const processTests = (test: TestFn<ContextType>) => {
 
     await runMigrations()
 
+    if (builder.dialect === 'SQLite') {
+      satellite.fkChecks = ForeignKeyChecks.inherit // set FK checks to inherit because by default they are disabled
+      await adapter.run({ sql: `PRAGMA foreign_keys = ON` })
+    }
+
     const tablename = 'child'
 
     // relations must be present at subscription delivery

clients/typescript/test/satellite/registry.test.ts

@@ -1,6 +1,9 @@
 import test from 'ava'
 
-import { InternalElectricConfig } from '../../src/config/index'
+import {
+  ForeignKeyChecks,
+  InternalElectricConfig,
+} from '../../src/config/index'
 import { DatabaseAdapter } from '../../src/electric/adapter'
 import { Migrator } from '../../src/migrators/index'
 import { Notifier } from '../../src/notifiers/index'
@@ -18,6 +21,7 @@ const notifier = {} as Notifier
 const socketFactory = {} as SocketFactory
 const config: InternalElectricConfig = {
   auth: {},
+  fkChecks: ForeignKeyChecks.inherit,
 }
 const args = [
   dbName,