diff --git a/UPGRADING.md b/UPGRADING.md index 938a1d91f..cc4d709c7 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -3,7 +3,8 @@ ### Upgrading to version 0.16.0+ * MSSQL: DB versions older than 2008 are no longer supported, make sure to update your DB; -* PostgreSQL|MySQL: it is recommended to use options object for `table.datetime` and `table.timestamp` methods instead of argument options. See documentation for these methods for more details. +* PostgreSQL|MySQL: it is recommended to use options object for `table.datetime` and `table.timestamp` methods instead of argument options. See documentation for these methods for more details; +* Node 6: There are known issues with duplicate event listeners when using knex.js with Node 6 (resulting in MaxListenersExceededWarning under certain use-cases (such as reusing single knex instance to run migrations or seeds multiple times)). Please upgrade to Node 8+ as soon as possible (knex 0.17.0 will be dropping Node 6 support altogether). ### Upgrading to version 0.15.0+ diff --git a/bin/cli.js b/bin/cli.js index fa4d85b7d..3b455a728 100755 --- a/bin/cli.js +++ b/bin/cli.js @@ -44,7 +44,10 @@ function initKnex(env, opts) { checkLocalModule(env); if (process.cwd() !== env.cwd) { process.chdir(env.cwd); - console.log('Working directory changed to', color.magenta(tildify(env.cwd))); + console.log( + 'Working directory changed to', + color.magenta(tildify(env.cwd)) + ); } if (!opts.knexfile) { @@ -102,7 +105,10 @@ function invoke(env) { .version( color.blue('Knex CLI version: ', color.green(cliPkg.version)) + '\n' + - color.blue('Local Knex version: ', color.green(env.modulePackage.version)) + + color.blue( + 'Local Knex version: ', + color.green(env.modulePackage.version) + ) + '\n' ) .option('--debug', 'Run with debugging.') diff --git a/knex.js b/knex.js index 9a64dc594..6b0c29691 100644 --- a/knex.js +++ b/knex.js @@ -1,3 +1,5 @@ +const { isNode6 } = require('./lib/util/version-helper'); + // Knex.js // -------------- // (c) 2013-present Tim Griesser @@ -6,12 +8,7 @@ // http://knexjs.org // Should be safe to remove after support for Node.js 6 is dropped -if ( - process && - process.versions && - process.versions.node && - process.versions.node.startsWith('6.') -) { +if (isNode6()) { const oldPromise = global.Promise; require('@babel/polyfill'); diff --git a/package.json b/package.json index 08418e0bc..348c53246 100644 --- a/package.json +++ b/package.json @@ -35,7 +35,7 @@ "devDependencies": { "@babel/cli": "^7.2.3", "@babel/core": "^7.2.2", - "@babel/preset-env": "^7.2.3", + "@babel/preset-env": "^7.3.1", "@types/node": "*", "JSONStream": "^1.3.5", "async": "^2.6.1", @@ -45,29 +45,29 @@ "chai-subset-in-order": "^2.1.2", "coveralls": "^3.0.2", "cross-env": "^5.2.0", - "eslint": "5.10.0", - "eslint-config-prettier": "^3.3.0", - "eslint-plugin-import": "^2.14.0", - "husky": "^1.2.1", + "eslint": "5.12.1", + "eslint-config-prettier": "^3.6.0", + "eslint-plugin-import": "^2.16.0", + "husky": "^1.3.1", "jake": "^8.0.19", "json-loader": "^0.5.7", - "lint-staged": "^8.1.0", + "lint-staged": "^8.1.1", "mocha": "^5.2.0", "mock-fs": "^4.7.0", "mssql": "^5.0.0-alpha.1", "mysql": "^2.16.0", "mysql2": "^1.6.4", "nyc": "^13.1.0", - "pg": "^7.7.1", + "pg": "^7.8.0", "pg-query-stream": "^1.1.2", - "prettier": "^1.15.3", - "rimraf": "^2.6.2", - "sinon": "^7.2.2", + "prettier": "^1.16.2", + "rimraf": "^2.6.3", + "sinon": "^7.2.3", "sinon-chai": "^3.3.0", - "source-map-support": "^0.5.9", - "sqlite3": "^4.0.4", + "source-map-support": "^0.5.10", + "sqlite3": "^4.0.6", "tap-spec": "^5.0.0", - "tape": "^4.9.1", + "tape": "^4.9.2", "through": "^2.3.8", "toxiproxy-node-client": "^2.0.6" }, diff --git a/src/util/make-knex.js b/src/util/make-knex.js index d52d982a6..e8f9476bf 100644 --- a/src/util/make-knex.js +++ b/src/util/make-knex.js @@ -4,15 +4,17 @@ import Migrator from '../migrate/Migrator'; import Seeder from '../seed/Seeder'; import FunctionHelper from '../functionhelper'; import QueryInterface from '../query/methods'; -import { assign } from 'lodash'; +import { assign, merge } from 'lodash'; import batchInsert from './batchInsert'; import * as bluebird from 'bluebird'; +import { isNode6 } from './version-helper'; export default function makeKnex(client) { // The object we're potentially using to kick off an initial chain. function knex(tableName, options) { return createQueryBuilder(knex.context, tableName, options); } + redefineProperties(knex, client); return knex; } @@ -81,15 +83,16 @@ function initContext(knexFn) { withUserParams(params) { const knexClone = shallowCloneFunction(knexFn); // We need to include getters in our clone if (this.client) { - knexClone.client = Object.assign({}, this.client); // Clone client to avoid leaking listeners that are set on it + knexClone.client = Object.create(this.client.constructor.prototype); // Clone client to avoid leaking listeners that are set on it + merge(knexClone.client, this.client); knexClone.client.config = Object.assign({}, this.client.config); // Clone client config to make sure they can be modified independently - const parentPrototype = Object.getPrototypeOf(this.client); - if (parentPrototype) { - Object.setPrototypeOf(knexClone.client, parentPrototype); - } } redefineProperties(knexClone, knexClone.client); + _copyEventListeners('query', knexFn, knexClone); + _copyEventListeners('query-error', knexFn, knexClone); + _copyEventListeners('query-response', knexFn, knexClone); + _copyEventListeners('start', knexFn, knexClone); knexClone.userParams = params; return knexClone; }, @@ -99,6 +102,12 @@ function initContext(knexFn) { knexFn.context = knexContext; } } +function _copyEventListeners(eventName, sourceKnex, targetKnex) { + const listeners = sourceKnex.listeners(eventName); + listeners.forEach((listener) => { + targetKnex.on(eventName, listener); + }); +} function redefineProperties(knex, client) { // Allow chaining methods from the root object, before @@ -194,21 +203,38 @@ function redefineProperties(knex, client) { knex[key] = ee[key]; } + // Unfortunately, something seems to be broken in Node 6 and removing events from a clone also mutates original Knex, + // which is highly undesireable + if (knex._internalListeners && !isNode6()) { + knex._internalListeners.forEach(({ eventName, listener }) => { + knex.client.removeListener(eventName, listener); // Remove duplicates for copies + }); + } + knex._internalListeners = []; + // Passthrough all "start" and "query" events to the knex object. - knex.client.on('start', function(obj) { + _addInternalListener(knex, 'start', (obj) => { knex.emit('start', obj); }); - knex.client.on('query', function(obj) { + _addInternalListener(knex, 'query', (obj) => { knex.emit('query', obj); }); - knex.client.on('query-error', function(err, obj) { + _addInternalListener(knex, 'query-error', (err, obj) => { knex.emit('query-error', err, obj); }); - knex.client.on('query-response', function(response, obj, builder) { + _addInternalListener(knex, 'query-response', (response, obj, builder) => { knex.emit('query-response', response, obj, builder); }); } +function _addInternalListener(knex, eventName, listener) { + knex.client.on(eventName, listener); + knex._internalListeners.push({ + eventName, + listener, + }); +} + function createQueryBuilder(knexContext, tableName, options) { const qb = knexContext.queryBuilder(); if (!tableName) diff --git a/src/util/version-helper.js b/src/util/version-helper.js new file mode 100644 index 000000000..01c0e05f5 --- /dev/null +++ b/src/util/version-helper.js @@ -0,0 +1,12 @@ +function isNode6() { + return ( + process && + process.versions && + process.versions.node && + process.versions.node.startsWith('6.') + ); +} + +module.exports = { + isNode6, +}; diff --git a/test/index.js b/test/index.js index e9f53868c..4557b82d1 100644 --- a/test/index.js +++ b/test/index.js @@ -31,6 +31,8 @@ describe('Query Building Tests', function() { require('./unit/schema/oracledb'); require('./unit/migrate/migration-list-resolver'); require('./unit/seed/seeder'); + // require('./unit/interface'); ToDo Uncomment after fixed + require('./unit/knex'); }); describe('Integration Tests', function() { diff --git a/test/integration/builder/additional.js b/test/integration/builder/additional.js index b9a3d900e..303f0b03d 100644 --- a/test/integration/builder/additional.js +++ b/test/integration/builder/additional.js @@ -5,6 +5,7 @@ const Knex = require('../../../knex'); const _ = require('lodash'); const Promise = require('bluebird'); +const { isNode6 } = require('../../../lib/util/version-helper'); module.exports = function(knex) { describe('Additional', function() { @@ -45,6 +46,9 @@ module.exports = function(knex) { }); it('should pass query context for raw responses', () => { + if (isNode6()) { + return; + } return knex .raw('select * from ??', ['accounts']) .queryContext('the context') @@ -104,6 +108,9 @@ module.exports = function(knex) { }); it('should work using camelCased table name', () => { + if (isNode6()) { + return; + } return knex('testTableTwo') .columnInfo() .then((res) => { @@ -118,6 +125,9 @@ module.exports = function(knex) { }); it('should work using snake_cased table name', () => { + if (isNode6()) { + return; + } return knex('test_table_two') .columnInfo() .then((res) => { @@ -958,6 +968,9 @@ module.exports = function(knex) { }); it('Event: query-response', function() { + if (isNode6()) { + return; + } let queryCount = 0; const onQueryResponse = function(response, obj, builder) { @@ -986,7 +999,10 @@ module.exports = function(knex) { }); }); - it('Event: does not duplicate listeners on a copy with user params', function() { + it('Event: preserves listeners on a copy with user params', function() { + if (isNode6()) { + return; + } let queryCount = 0; const onQueryResponse = function(response, obj, builder) { @@ -1012,7 +1028,7 @@ module.exports = function(knex) { }) .then(function() { expect(Object.keys(knex._events).length).to.equal(1); - expect(Object.keys(knexCopy._events).length).to.equal(0); + expect(Object.keys(knexCopy._events).length).to.equal(1); knex.removeListener('query-response', onQueryResponse); expect(Object.keys(knex._events).length).to.equal(0); expect(queryCount).to.equal(4); @@ -1020,6 +1036,9 @@ module.exports = function(knex) { }); it('Event: query-error', function() { + if (isNode6()) { + return; + } let queryCountKnex = 0; let queryCountBuilder = 0; const onQueryErrorKnex = function(error, obj) { @@ -1055,6 +1074,9 @@ module.exports = function(knex) { }); it('Event: start', function() { + if (isNode6()) { + return; + } return knex('accounts') .insert({ last_name: 'Start event test' }) .then(function() { diff --git a/test/integration/builder/transaction.js b/test/integration/builder/transaction.js index 6f6b945ba..783f9c7b1 100644 --- a/test/integration/builder/transaction.js +++ b/test/integration/builder/transaction.js @@ -6,6 +6,7 @@ const Promise = testPromise; const Knex = require('../../../knex'); const _ = require('lodash'); const sinon = require('sinon'); +const { isNode6 } = require('../../../lib/util/version-helper'); module.exports = function(knex) { // Certain dialects do not have proper insert with returning, so if this is true @@ -362,6 +363,10 @@ module.exports = function(knex) { }); it('#855 - Query Event should trigger on Transaction Client AND main Client', function() { + if (isNode6()) { + return; + } + let queryEventTriggered = false; knex.once('query', function(queryData) { diff --git a/test/integration/helpers/knex-builder.js b/test/integration/helpers/knex-builder.js new file mode 100644 index 000000000..028320acc --- /dev/null +++ b/test/integration/helpers/knex-builder.js @@ -0,0 +1,15 @@ +const knex = require('../../../knex'); +const config = require('../../knexfile'); + +/* +Please do not remove this file even though it is not referenced anywhere. +This helper is meant for local debugging of specific tests. + */ + +function getSqliteKnex() { + return knex(config.sqlite3); +} + +module.exports = { + getSqliteKnex, +}; diff --git a/test/unit/knex.js b/test/unit/knex.js index b55c8d4a8..061c65cd5 100644 --- a/test/unit/knex.js +++ b/test/unit/knex.js @@ -4,6 +4,7 @@ const bluebird = require('bluebird'); const sqliteConfig = require('../knexfile').sqlite3; const sqlite3 = require('sqlite3'); const { noop } = require('lodash'); +const { isNode6 } = require('../../lib/util/version-helper'); describe('knex', () => { it('preserves global Bluebird Promise', () => { @@ -98,10 +99,76 @@ describe('knex', () => { const knexWithParams = knex.withUserParams({ userParam: '451' }); expect(knexWithParams.migrate.knex.userParams).to.deep.equal({ + isProcessingDisabled: true, + postProcessResponse: undefined, userParam: '451', + wrapIdentifier: undefined, }); }); + it('copying does not result in duplicate listeners', () => { + if (isNode6()) { + return; + } + + const knex = Knex({ + client: 'sqlite', + }); + const knexWithParams = knex.withUserParams(); + + expect(knex.client.listeners('start').length).to.equal(1); + expect(knex.client.listeners('query').length).to.equal(1); + expect(knex.client.listeners('query-error').length).to.equal(1); + expect(knex.client.listeners('query-response').length).to.equal(1); + + expect(knexWithParams.client.listeners('start').length).to.equal(1); + expect(knexWithParams.client.listeners('query').length).to.equal(1); + expect(knexWithParams.client.listeners('query-error').length).to.equal(1); + expect(knexWithParams.client.listeners('query-response').length).to.equal( + 1 + ); + }); + + it('listeners added to knex directly get copied correctly', () => { + const knex = Knex({ + client: 'sqlite', + }); + const onQueryResponse = function(response, obj, builder) {}; + expect(knex.listeners('query-response').length).to.equal(0); + knex.on('query-response', onQueryResponse); + + const knexWithParams = knex.withUserParams(); + + expect(knex.listeners('query-response').length).to.equal(1); + expect(knexWithParams.listeners('query-response').length).to.equal(1); + }); + + it('adding listener to copy does not affect base knex', () => { + if (isNode6()) { + return; + } + + const knex = Knex({ + client: 'sqlite', + }); + + expect(knex.client.listeners('start').length).to.equal(1); + expect(knex.client.listeners('query').length).to.equal(1); + expect(knex.client.listeners('query-error').length).to.equal(1); + expect(knex.client.listeners('query-response').length).to.equal(1); + + const knexWithParams = knex.withUserParams(); + knexWithParams.client.on('query', (obj) => { + knexWithParams.emit('query', obj); + }); + + expect(knex.client.listeners('start').length).to.equal(1); + expect(knex.client.listeners('query').length).to.equal(1); + expect(knex.client.listeners('query-error').length).to.equal(1); + expect(knex.client.listeners('query-response').length).to.equal(1); + expect(knexWithParams.client.listeners('query').length).to.equal(2); + }); + it('sets correct postProcessResponse for builders instantiated from clone', () => { const knex = Knex({ client: 'sqlite', @@ -171,17 +238,18 @@ describe('knex', () => { }); it('throws if client module has not been installed', () => { - expect(Knex({ client: 'oracle' })).to.throw( - /Knex: run\n$ npm install oracle/ + expect(() => { + Knex({ client: 'oracledb', connection: {} }); + }).to.throw( + "Knex: run\n$ npm install oracledb --save\nCannot find module 'oracledb'" ); }); describe('async stack traces', () => { it('should capture stack trace on query builder instantiation', () => { - const knex = Knex({ - ...sqliteConfig, - asyncStackTraces: true, - }); + const knex = Knex( + Object.assign({}, sqliteConfig, { asyncStackTraces: true }) + ); return knex('some_nonexisten_table') .select()