RFC #3 - Should be able to just have SQL files

[djMax]

Right now we support having a js file with up/down SQL files that use a boilerplate JS. It would be nice to just skip the js file and support SQL up/down files in a directory with a date based name. I don't love the cruft of lots of boilerplate JS.

---

Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

[wzrdtales]

Any suggestion on how a safe standard may looks like that is not getting in the way of real migrations?
First thought of mine would be to use the scoping config for this, I would not go for generally searching for .sql files, this would end up colliding with the current default sql behavior.

[wzrdtales]

But generally agreeing, the current way is really lots of boilerplate that is actually avoidable and would only be really useful if one wants to combine a migration with a bigger chunk of raw sql.

[wzrdtales]

It is hard to not collide against the current default behavior, if this is going to be introduced this is probably a candidate for the migration version assistant that was planned after #400 occured. This way it would be possible to reconfigure the scope configs to match the original behavior, for anyone that is currently transitioning from an older standard.

[djMax]

How about just a config setting that enabled it?

[wzrdtales]

Yeah, I've thought about this, but I asked my self the question:

If .coffee file gets loaded seemlessly as soon as the coffee-script module is available, while .sql files exclusively are executed as long as the config is stating to do this. Is this still a clean behavior, or does this ends up in confusion for the user?

Also with just the config param the problem is not solved though. If there are sql files used in the default behavior, those would end up being executed twice when the scope definition would be all.

[djMax]

I think a config (defaulting to off) does solve it in that we're not trying to protect people from themselves. If you adopt this mechanism, you should not do silly things like mix directories with "up.sql" and "down.sql" into JS based migrations.

[wzrdtales]

While that might be true, it would be also very common, that people that are migrating from the original default behavior are stumbling over this.

Also to note the problem is not the mixing of sql and js migrations in the same directory, but to mix behaviors. And in case of sql most people are going to mix sql and js, as sql is not so much of a migration anymore, as this could be anything, but in many cases can be quite useful.

I do also not think that db-migrate should protect people from themselves, that is a task that is probably never accomplished without transforming an application in a bloated and not so useful one anymore. But on the other side creating confusion is also not the way to go, I will give this some thoughts later again.

[fantapop]

We just picked this library up. It would be great to get rid of this boilerplate when using pure sql. fwiw, I would also like the option to choose either sql or js for a specific migration.

[stepankuzmin]

Hi there! I agree, that moving boilerplate code, that loads SQL, can be moved into the library. Any plans on this issue?

[Hurtak]

For anyone new looking for options how to avoid having all .js migration files full of generated boilerplate, you can easily abstract the boilerplate into shared file. Eg like this

// migrations/20190920142456-add-users-table.js
module.exports = require("./utils/boilerplate.js")("20190920142456-add-users-table");
// migrations/sqls/20190920142456-add-users-table-up.sql -> 20190920142456-add-users-table-up

And then have the shared boilerplate in function on the side

// migrations/utils/boilerplate.js
"use strict";

const fs = require("fs");
const path = require("path");

async function executeSql(db, sqlFileName, upOrDown) {
  const filePath = path.join(__dirname, "../sqls/", `${sqlFileName}-${upOrDown}.sql`);

  const data = await 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);
    });
  });

  return db.runSql(data);
}

module.exports = function(sqlFileName) {
  const setup = function(options, seedLink) {
    // const dbm = options.dbmigrate;
    // const type = dbm.dataType;
    // const seed = seedLink;
  };

  const up = function(db) {
    return executeSql(db, sqlFileName, "up");
  };

  const down = function(db) {
    return executeSql(db, sqlFileName, "down");
  };

  return {
    setup,
    up,
    down,
    _meta: {
      version: 1,
    },
  };
};

[vincentcr]

@wzrdtales we just picked up this library as well, planning to use mostly plain SQL. I second the other comments that the .js boilerplate is quite unfortunate. Reading your comments, I'm a bit confused as to what the actual stumbling block is.

It seems to me js loader code is pure, repetitive boilerplate, that could easily be moved into an internal module of this package. If I don't want a pure sql migration, I can mix and match in a .js migration.

Also note that the generated code conflicts with many linter rulesets, requiring additional configuration to avoid linter errors.

I agree this might confuse existing users at first, but this change should be easy to explain and understand, no? I don't think there's any major behavior change that can be done with absolutely zero friction. Making it a major version change would prevent it from propagating automatically to everyone.

If you don't have the time to do it and you're willing to accept such a change, then I'd be happy to volunteer to do the PR.

[wzrdtales]

there will be some work around that, but it is a task for a plugin rather than the core. feel free to put something together already I am happy to review it and help with getting it into the right structure.

[vincentcr]

sounds good @wzrdtales, are there examples of plugins I should follow?

[nmccready]

While I agree that it's almost 90% boiler plate the option needs to be there to utilize a js loader file on a per migration basis. Where it defaults to not needed one by default. I say this cause there are different migrations that sometimes require massaging on a different degree or basis than others. Think of inject environment variables into the SQL as a simple use case.

This is a highly desirable feature.

Also where does one look to be able to set the JS boilerplate files location. Currently it defaults to:

-/migrations/*.js - can't collapse easily to look at sql files only

• /migraitons/sql/*.sql - easy to read and collapsable

Much more desirable to have ./migrations/js/*.js .