New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor(bluebird): remove Bluebird.bind #3477
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,16 +5,7 @@ const fs = require('fs'); | |
const path = require('path'); | ||
const { promisify } = require('util'); | ||
const mkdirp = require('mkdirp'); | ||
const Bluebird = require('bluebird'); | ||
const { | ||
filter, | ||
includes, | ||
map, | ||
bind, | ||
template, | ||
each, | ||
extend, | ||
} = require('lodash'); | ||
const { filter, includes, extend } = require('lodash'); | ||
const { writeJsFileUsingTemplate } = require('../util/template'); | ||
|
||
// The new seeds we're performing, typically called from the `knex.seed` | ||
|
@@ -29,7 +20,7 @@ class Seeder { | |
// Runs seed files for the given environment. | ||
async run(config) { | ||
this.config = this.setConfig(config); | ||
const [all] = await this._seedData(); | ||
const all = await this._listAll(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will types changed in https://github.com/knex/knex/pull/3438/files still be correct after this change? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. _seedData is just _listAll, but wrapping result inside promise into [] |
||
const files = | ||
config && config.specific | ||
? all.filter((file) => file === config.specific) | ||
|
@@ -62,11 +53,6 @@ class Seeder { | |
); | ||
} | ||
|
||
// Gets the seed file list from the specified seed directory. | ||
_seedData() { | ||
return Bluebird.join(this._listAll()); | ||
} | ||
|
||
// Ensures a folder for the seeds exist, dependent on the | ||
// seed config settings. | ||
async _ensureFolder() { | ||
|
@@ -80,13 +66,8 @@ class Seeder { | |
|
||
// Run seed files, in sequence. | ||
_runSeeds(seeds) { | ||
return Bluebird.all(map(seeds, bind(this._validateSeedStructure, this))) | ||
.bind(this) | ||
.then(function(seeds) { | ||
return Bluebird.bind(this).then(function() { | ||
return this._waterfallBatch(seeds); | ||
}); | ||
}); | ||
seeds.forEach((seed) => this._validateSeedStructure(seed)); | ||
return this._waterfallBatch(seeds); | ||
} | ||
|
||
// Validates seed files by requiring and checking for a `seed` function. | ||
|
@@ -105,14 +86,6 @@ class Seeder { | |
); | ||
} | ||
|
||
// Generates the stub template for the current seed file, returning a compiled template. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @lorefnon I assume we stopped using it after your refactoring? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, i moved the lodash template usage to a shared utility which we can potentially use from other places as well. |
||
_generateStubTemplate() { | ||
const stubPath = this._getStubPath(); | ||
return promisify(fs.readFile)(stubPath).then( | ||
(stub) => template(stub.toString(), { variable: 'd' }) | ||
); | ||
} | ||
|
||
_getNewStubFileName(name) { | ||
if (name[0] === '-') name = name.slice(1); | ||
return name + '.' + this.config.extension; | ||
|
@@ -136,39 +109,32 @@ class Seeder { | |
} | ||
|
||
// Runs a batch of seed files. | ||
_waterfallBatch(seeds) { | ||
async _waterfallBatch(seeds) { | ||
const { knex } = this; | ||
const seedDirectory = this._absoluteConfigDir(); | ||
let current = Bluebird.bind({ failed: false, failedOn: 0 }); | ||
const log = []; | ||
each(seeds, (seed) => { | ||
const name = path.join(seedDirectory, seed); | ||
seed = require(name); | ||
|
||
// Run each seed file. | ||
current = current.then(() => | ||
// Nesting promise to prevent bubbling up of error on catch | ||
Promise.resolve() | ||
.then(() => seed.seed(knex)) | ||
.then(() => log.push(name)) | ||
.catch((originalError) => { | ||
const error = new Error( | ||
`Error while executing "${name}" seed: ${originalError.message}` | ||
); | ||
error.original = originalError; | ||
error.stack = | ||
error.stack | ||
.split('\n') | ||
.slice(0, 2) | ||
.join('\n') + | ||
'\n' + | ||
originalError.stack; | ||
throw error; | ||
}) | ||
); | ||
}); | ||
|
||
return current.then(() => [log]); | ||
for (const seedName of seeds) { | ||
const seedPath = path.join(seedDirectory, seedName); | ||
const seed = require(seedPath); | ||
try { | ||
await seed.seed(knex); | ||
log.push(seedPath); | ||
} catch (originalError) { | ||
const error = new Error( | ||
`Error while executing "${seedPath}" seed: ${originalError.message}` | ||
); | ||
error.original = originalError; | ||
error.stack = | ||
error.stack | ||
.split('\n') | ||
.slice(0, 2) | ||
.join('\n') + | ||
'\n' + | ||
originalError.stack; | ||
throw error; | ||
} | ||
} | ||
return [log]; | ||
} | ||
|
||
_absoluteConfigDir() { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are those fields not used anywhere?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't found any usages in project
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
7a6937f first revision where failedOn exists, i think it just partially implemented feature
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair enough, guess we can drop it.