Skip to content

Commit

Permalink
warn when migration did not take a callback and did not return a promise
Browse files Browse the repository at this point in the history
  • Loading branch information
wesleytodd committed Nov 5, 2017
1 parent 2df3fb2 commit 39fe87f
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 1 deletion.
4 changes: 4 additions & 0 deletions lib/migrate.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ function migrate (set, direction, migrationName, fn) {

// Run the migration function
set.emit('migration', migration, direction)
var arity = migration[direction].length
var returnValue = migration[direction](function (err) {
if (isPromise) return set.emit('warning', 'if your migration returns a promise, do not call the done callback')
completeMigration(err)
Expand All @@ -38,6 +39,9 @@ function migrate (set, direction, migrationName, fn) {
// Is it a promise?
isPromise = typeof Promise !== 'undefined' && returnValue instanceof Promise

// If not a promise and arity is not 1, warn
if (!isPromise && arity < 1) set.emit('warning', 'it looks like your migration did not take or callback or return a Promise, this might be an error')

// Handle the promises
if (isPromise) {
returnValue
Expand Down
9 changes: 9 additions & 0 deletions test/fixtures/promises/4-neither-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
'use strict'

module.exports.up = function () {
arguments[0]()
}

module.exports.down = function () {
arguments[0]()
}
File renamed without changes.
16 changes: 15 additions & 1 deletion test/promises.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,22 @@ describe('Promise migrations', function () {
})
})

it('should warn with no promise or callback', function (done) {
set.up('3-callback-promise-test.js', function () {
var warned = false
set.on('warning', function (msg) {
assert(msg)
warned = true
})
set.up('4-neither-test.js', function () {
assert(warned)
done()
})
})
})

it('should error with rejected promises', function (done) {
set.up('4-failure-test.js', function (err) {
set.up('99-failure-test.js', function (err) {
assert(err)
assert.equal(err.message, 'foo')
done()
Expand Down

0 comments on commit 39fe87f

Please sign in to comment.