refactor: extract connection configurations to root, deprecate old ones. (#1388)

also flip reset connection on release default.

...

...

...

Author: igalklebanov

src/dialect/mssql/mssql-dialect-config.ts

@@ -1,4 +1,14 @@
+import { KyselyTypeError } from '../../util/type-error.js'
+
 export interface MssqlDialectConfig {
+  /**
+   * When `true`, connections are reset to their initial states when released
+   * back to the pool, resulting in additional requests to the database.
+   *
+   * Defaults to `false`.
+   */
+  resetConnectionsOnRelease?: boolean
+
   /**
    * This dialect uses the `tarn` package to manage the connection pool to your
    * database. To use it as a peer dependency and not bundle it with Kysely's code,
@@ -55,20 +65,26 @@ export interface MssqlDialectConfig {
    * ```
    */
   tedious: Tedious
+
+  /**
+   * When `true`, connections are validated before being acquired from the pool,
+   * resulting in additional requests to the database.
+   *
+   * Defaults to `true`.
+   */
+  validateConnections?: boolean
 }
 
 export interface Tedious {
   connectionFactory: () => TediousConnection | Promise<TediousConnection>
   ISOLATION_LEVEL: TediousIsolationLevel
   Request: TediousRequestClass
-  TYPES: TediousTypes
+  // TODO: remove in v0.29.0
   /**
-   * Controls whether connections are reset to their initial states when released back to the pool. Resetting a connection performs additional requests to the database.
-   * See {@link https://tediousjs.github.io/tedious/api-connection.html#function_reset | connection.reset}.
-   *
-   * Defaults to `true`.
+   * @deprecated use {@link MssqlDialectConfig.resetConnectionsOnRelease} instead.
    */
-  resetConnectionOnRelease?: boolean
+  resetConnectionOnRelease?: KyselyTypeError<'deprecated: use `MssqlDialectConfig.resetConnectionsOnRelease` instead'>
+  TYPES: TediousTypes
 }
 
 export interface TediousConnection {
@@ -165,12 +181,11 @@ export interface Tarn {
    * which must be implemented by this dialect.
    */
   options: Omit<TarnPoolOptions<any>, 'create' | 'destroy' | 'validate'> & {
+    // TODO: remove in v0.29.0
     /**
-     * Controls whether connections are validated before being acquired from the pool. Connection validation performs additional requests to the database.
-     *
-     * Defaults to `true`.
+     * @deprecated use {@link MssqlDialectConfig.validateConnections} instead.
      */
-    validateConnections?: boolean
+    validateConnections?: KyselyTypeError<'deprecated: use `MssqlDialectConfig.validateConnections` instead'>
   }
 
   /**

src/dialect/mssql/mssql-driver.ts

@@ -31,7 +31,7 @@ import { extendStackTrace } from '../../util/stack-trace-utils.js'
 import { randomString } from '../../util/random-string.js'
 import { Deferred } from '../../util/deferred.js'
 
-const PRIVATE_RELEASE_METHOD = Symbol()
+const PRIVATE_RESET_METHOD = Symbol()
 const PRIVATE_DESTROY_METHOD = Symbol()
 
 export class MssqlDriver implements Driver {
@@ -41,8 +41,11 @@ export class MssqlDriver implements Driver {
   constructor(config: MssqlDialectConfig) {
     this.#config = freeze({ ...config })
 
-    const { tarn, tedious } = this.#config
-    const { validateConnections, ...poolOptions } = tarn.options
+    const { tarn, tedious, validateConnections } = this.#config
+    const {
+      validateConnections: deprecatedValidateConnections,
+      ...poolOptions
+    } = tarn.options
 
     this.#pool = new tarn.Pool({
       ...poolOptions,
@@ -57,7 +60,8 @@ export class MssqlDriver implements Driver {
       // @ts-ignore `tarn` accepts a function that returns a promise here, but
       // the types are not aligned and it type errors.
       validate:
-        validateConnections === false
+        validateConnections === false ||
+        (deprecatedValidateConnections as any) === false
           ? undefined
           : (connection) => connection.validate(),
     })
@@ -101,7 +105,13 @@ export class MssqlDriver implements Driver {
   }
 
   async releaseConnection(connection: MssqlConnection): Promise<void> {
-    await connection[PRIVATE_RELEASE_METHOD]()
+    if (
+      this.#config.resetConnectionsOnRelease ||
+      this.#config.tedious.resetConnectionOnRelease
+    ) {
+      await connection[PRIVATE_RESET_METHOD]()
+    }
+
     this.#pool.release(connection)
   }
 
@@ -297,16 +307,13 @@ class MssqlConnection implements DatabaseConnection {
     })
   }
 
-  async [PRIVATE_RELEASE_METHOD](): Promise<void> {
-    // TODO: flip this to `if (!this.#tedious.resetConnectionOnRelease) {}` in a future PR.
-    if (this.#tedious.resetConnectionOnRelease !== false) {
-      await new Promise((resolve, reject) => {
-        this.#connection.reset((error) => {
-          if (error) reject(error)
-          else resolve(undefined)
-        })
+  async [PRIVATE_RESET_METHOD](): Promise<void> {
+    await new Promise((resolve, reject) => {
+      this.#connection.reset((error) => {
+        if (error) reject(error)
+        else resolve(undefined)
       })
-    }
+    })
   }
 
   [PRIVATE_DESTROY_METHOD](): Promise<void> {

test/node/src/test-setup.ts

@@ -35,7 +35,6 @@ import {
   InsertObject,
   MssqlDialect,
   SelectQueryBuilder,
-  QueryId,
 } from '../../../'
 import {
   OrderByDirection,
@@ -189,19 +188,23 @@ export const DB_CONFIGS: PerDialect<KyselyConfig> = {
 
   mssql: {
     dialect: new MssqlDialect({
+      resetConnectionsOnRelease: false,
       tarn: {
         options: {
           max: POOL_SIZE,
           min: 0,
-          validateConnections: false,
+          // @ts-expect-error making sure people see the deprecation warning
+          validateConnections: true,
         },
         ...Tarn,
       },
       tedious: {
         ...Tedious,
         connectionFactory: () => new Tedious.Connection(DIALECT_CONFIGS.mssql),
-        resetConnectionOnRelease: false,
+        // @ts-expect-error making sure people see the deprecation warning
+        resetConnectionOnRelease: true,
       },
+      validateConnections: false,
     }),
     plugins: PLUGINS,
   },