db-migrate / node-db-migrate

Database migration framework for node
Other
2.32k stars 360 forks source link

Reduce boilerplate code when running migrations from SQL files #452

Closed maxsummers closed 6 years ago

maxsummers commented 7 years ago

When sql-file option is used it creates the following migration file:

'use strict';

var dbm;
var type;
var seed;
var fs = require('fs');
var path = require('path');
var Promise;

/**
  * We receive the dbmigrate dependency from dbmigrate initially.
  * This enables us to not have to rely on NODE_PATH.
  */
exports.setup = function(options, seedLink) {
  dbm = options.dbmigrate;
  type = dbm.dataType;
  seed = seedLink;
  Promise = options.Promise;
};

exports.up = function(db) {
  var filePath = path.join(__dirname, 'sqls', '20161226111110-test-up.sql');
  return new Promise( function( resolve, reject ) {
    fs.readFile(filePath, {encoding: 'utf-8'}, function(err,data){
      if (err) return reject(err);
      console.log('received data: ' + data);

      resolve(data);
    });
  })
  .then(function(data) {
    return db.runSql(data);
  });
};

exports.down = function(db) {
  var filePath = path.join(__dirname, 'sqls', '20161226111110-test-down.sql');
  return new Promise( function( resolve, reject ) {
    fs.readFile(filePath, {encoding: 'utf-8'}, function(err,data){
      if (err) return reject(err);
      console.log('received data: ' + data);

      resolve(data);
    });
  })
  .then(function(data) {
    return db.runSql(data);
  });
};

exports._meta = {
  "version": 1
};

This is a huge amount of non-optimized boilerplate code.

It's pretty common to have hundreds of migrations in actively developed project. Now, consider this boilerplate multiplied by one hundred. This will increase code base size significantly and will take additional time for IDE to analyze the code and build it's indices.

Please, consider to reduce the size of such migration files. I think the only relevant part of it is: __dirname, 'sqls', '20161226111110-test-down.sql', so everything else could be moved to the library code.

--- Want to back this issue? **[Post a bounty on it!](https://www.bountysource.com/issues/40399864-reduce-boilerplate-code-when-running-migrations-from-sql-files?utm_campaign=plugin&utm_content=tracker%2F73887&utm_medium=issues&utm_source=github)** We accept bounties via [Bountysource](https://www.bountysource.com/?utm_campaign=plugin&utm_content=tracker%2F73887&utm_medium=issues&utm_source=github).
wzrdtales commented 7 years ago

Duplicate of #401

wzrdtales commented 7 years ago

@maxsummers Please have a look in the other issue I linked above and read my comments over there, I have already stated about some things that need to be in place to actually do what you ask for.

Fact is: SQL migrations currently do not exist, there is just a template and you basically still use javascript migrations. So yes this is true, but this is a migration framework that started with just javascript, so this is nothing that would be unexpected. So far there are plans to support raw SQL in a more native way, just like this is true now for es6 or coffeescript, for SQL as it does not use the abstractions and is not getting transpiled, there needs to be a few more things to be done and thought about though. As said above take a look over there in the other issue.

troygoode commented 7 years ago

@maxsummers this was driving me nuts so I pulled the boilerplate out into a separate package that you can use:

https://www.npmjs.com/package/db-migrate-boilerplate

This simplifies what you're committing into your repo to:

'use strict'

const path = require('path')
const boilerplate = require('db-migrate-boilerplate')

module.exports = boilerplate({
  upPath: path.join(__dirname, 'sqls', '20161226111110-test-up.sql'),
  downPath: path.join(__dirname, 'sqls', '20161226111110-test-down.sql')
})
wzrdtales commented 7 years ago

@troygoode :) +1

I've made some thoughts about that again, and it might be quite a good option to handle the word sqls as a keyword that can not be used to name a scope, just like all. That way the problem described in the other issue wouldn't exist anymore and a name schema like the already existing ones would work to directly execute sqls.

stale[bot] commented 6 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.