refactor: replace usage of Bluebird Promise.tap with native Promise.t…

…hen (#3287)
knex · Jun 17, 2019 · 9d8e900 · 9d8e900
1 parent 1e950b9
commit 9d8e900
Show file tree

Hide file tree

Showing 12 changed files with 42 additions and 25 deletions.
diff --git a/src/client.js b/src/client.js
@@ -251,12 +251,13 @@ assign(Client.prototype, {
 
     return Object.assign(poolConfig, {
       create: () => {
-        return this.acquireRawConnection().tap((connection) => {
+        return this.acquireRawConnection().then(async (connection) => {
           connection.__knexUid = uniqueId('__knexUid');
 
           if (poolConfig.afterCreate) {
-            return Bluebird.promisify(poolConfig.afterCreate)(connection);
+            await Bluebird.promisify(poolConfig.afterCreate)(connection);
           }
+          return connection;
         });
       },
 
@@ -306,8 +307,9 @@ assign(Client.prototype, {
     }
     try {
       return Bluebird.try(() => this.pool.acquire().promise)
-        .tap((connection) => {
+        .then((connection) => {
           debug('acquired connection from pool: %s', connection.__knexUid);
+          return connection;
         })
         .catch(TimeoutError, () => {
           throw new Bluebird.TimeoutError(

diff --git a/src/dialects/mssql/transaction.js b/src/dialects/mssql/transaction.js
@@ -70,12 +70,13 @@ module.exports = class Transaction_MSSQL extends Transaction {
         reject(e);
       }
     })
-      .tap(function(conn) {
+      .then(function(conn) {
         conn.__knexTxId = t.txid;
         if (!t.outerTx) {
           t.conn = conn;
           conn.tx_ = conn.transaction();
         }
+        return conn;
       })
       .disposer(function(conn) {
         if (t.outerTx) return;

diff --git a/src/dialects/mysql/schema/tablecompiler.js b/src/dialects/mysql/schema/tablecompiler.js
@@ -95,9 +95,7 @@ assign(TableCompiler_MySQL.prototype, {
               }
             })
               .then(function() {
-                let sql = `alter table ${table} change ${wrapped} ${
-                  column.Type
-                }`;
+                let sql = `alter table ${table} change ${wrapped} ${column.Type}`;
 
                 if (String(column.Null).toUpperCase() !== 'YES') {
                   sql += ` NOT NULL`;

diff --git a/src/dialects/mysql/transaction.js b/src/dialects/mysql/transaction.js
@@ -26,14 +26,15 @@ assign(Transaction_MySQL.prototype, {
         t._completed = true;
         debug('%s error running transaction query', t.txid);
       })
-      .tap(function() {
+      .then(function(res) {
         if (status === 1) t._resolver(value);
         if (status === 2) {
           if (isUndefined(value)) {
             value = new Error(`Transaction rejected with non-error: ${value}`);
           }
           t._rejecter(value);
         }
+        return res;
       });
     if (status === 1 || status === 2) {
       t._completed = true;

diff --git a/src/dialects/mysql2/transaction.js b/src/dialects/mysql2/transaction.js
@@ -25,13 +25,14 @@ assign(Transaction_MySQL2.prototype, {
         t._completed = true;
         debug('%s error running transaction query', t.txid);
       })
-      .tap(function() {
+      .then(function(res) {
         if (status === 1) t._resolver(value);
         if (status === 2) {
           if (isUndefined(value)) {
             value = new Error(`Transaction rejected with non-error: ${value}`);
           }
           t._rejecter(value);
+          return res;
         }
       });
     if (status === 1 || status === 2) {

diff --git a/src/dialects/oracle/transaction.js b/src/dialects/oracle/transaction.js
@@ -50,10 +50,11 @@ module.exports = class Oracle_Transaction extends Transaction {
 
         return connection;
       })
-      .tap((connection) => {
+      .then((connection) => {
         if (!t.outerTx) {
           connection.setAutoCommit(false);
         }
+        return connection;
       })
       .disposer((connection) => {
         debugTx('%s: releasing connection', t.txid);

diff --git a/src/dialects/postgres/index.js b/src/dialects/postgres/index.js
@@ -125,8 +125,9 @@ assign(Client_PG.prototype, {
         }
         resolver(connection);
       });
-    }).tap(function setSearchPath(connection) {
-      return client.setSchemaSearchPath(connection);
+    }).then(function setSearchPath(connection) {
+      client.setSchemaSearchPath(connection);
+      return connection;
     });
   },
 

diff --git a/src/interface.js b/src/interface.js
@@ -91,7 +91,6 @@ module.exports = function(Target) {
       'spread',
       'map',
       'reduce',
-      'tap',
       'thenReturn',
       'return',
       'yield',

diff --git a/src/migrate/Migrator.js b/src/migrate/Migrator.js
@@ -64,7 +64,10 @@ class Migrator {
 
     return migrationListResolver
       .listAllAndCompleted(this.config, this.knex)
-      .tap((value) => validateMigrationList(this.config.migrationSource, value))
+      .then((value) => {
+        validateMigrationList(this.config.migrationSource, value);
+        return value;
+      })
       .then(([all, completed]) => {
         const migrations = getNewMigrations(
           this.config.migrationSource,
@@ -100,7 +103,10 @@ class Migrator {
 
     return migrationListResolver
       .listAllAndCompleted(this.config, this.knex)
-      .tap((value) => validateMigrationList(this.config.migrationSource, value))
+      .then((value) => {
+        validateMigrationList(this.config.migrationSource, value);
+        return value;
+      })
       .then(([all, completed]) => {
         const migrationToRun = getNewMigrations(
           this.config.migrationSource,
@@ -141,9 +147,10 @@ class Migrator {
       }
       migrationListResolver
         .listAllAndCompleted(this.config, this.knex)
-        .tap((value) =>
-          validateMigrationList(this.config.migrationSource, value)
-        )
+        .then((value) => {
+          validateMigrationList(this.config.migrationSource, value);
+          return value;
+        })
         .then((val) => {
           const [allMigrations, completedMigrations] = val;
 
@@ -168,8 +175,9 @@ class Migrator {
 
     return migrationListResolver
       .listAllAndCompleted(this.config, this.knex)
-      .tap((value) => {
-        return validateMigrationList(this.config.migrationSource, value);
+      .then((value) => {
+        validateMigrationList(this.config.migrationSource, value);
+        return value;
       })
       .then(([all, completed]) => {
         const migrationToRun = all
@@ -306,7 +314,10 @@ class Migrator {
         .then((batchNo) => {
           return this._waterfallBatch(batchNo, migrations, direction, trx);
         })
-        .tap(() => this._freeLock(trx))
+        .then(async (res) => {
+          await this._freeLock(trx);
+          return res;
+        })
         .catch(async (error) => {
           let cleanupReady = Promise.resolve();
 

diff --git a/src/runner.js b/src/runner.js
@@ -52,8 +52,9 @@ assign(Runner.prototype, {
 
         // Fire a single "end" event on the builder when
         // all queries have successfully completed.
-        .tap(function() {
+        .then(function(res) {
           runner.builder.emit('end');
+          return res;
         })
     );
   },

diff --git a/src/transaction.js b/src/transaction.js
@@ -156,7 +156,7 @@ class Transaction extends EventEmitter {
         this._completed = true;
         debug('%s error running transaction query', this.txid);
       })
-      .tap(() => {
+      .then((res) => {
         if (status === 1) {
           this._resolver(value);
         }
@@ -166,6 +166,7 @@ class Transaction extends EventEmitter {
           }
           this._rejecter(value);
         }
+        return res;
       });
     if (status === 1 || status === 2) {
       this._completed = true;
@@ -322,7 +323,6 @@ const promiseInterface = [
   'spread',
   'map',
   'reduce',
-  'tap',
   'thenReturn',
   'return',
   'yield',

diff --git a/test/integration/logger.js b/test/integration/logger.js
@@ -66,12 +66,13 @@ module.exports = function(knex) {
         const oldThen = qb.then;
         qb.then = function() {
           let promise = oldThen.apply(this, []);
-          promise = promise.tap(function(resp) {
+          promise = promise.then(function(resp) {
             if (typeof returnval === 'function') {
               expect(!!returnval(resp)).to.equal(true);
             } else {
               expect(stripDates(resp)).to.eql(returnval);
             }
+            return resp;
           });
           return promise.then.apply(promise, arguments);
         };