Opt-in ES6 support via babel-register

[benjie]

Fixes #289
Possibly removes the need for #340?

I was trying to find the easiest way to support the new import/export syntax via Babel from within the migrations so that they would pass my ESLint rules and be consistent with the rest of my app. I remembered that you've had coffee-script support for ages, so I figured it might be possible to add Babel support the same way; and it worked!

To use it, the user would opt-in by doing npm install babel-register - that's all.

I don't think this breaks backwards compatibility since babel runs vanilla ES3/ES5 javascript code just fine; so whether or not the project has babel-register installed already I don't see any negative consequences (except an almost negligible performance cost?)

I couldn't add tests for this because that would require modifying the package.json to require babel-register and thus all tests would then use babel's parser.

If you think this is a good idea I'd be happy to update the relevant docs with a couple sentences about it - just point me to the relevant places.

[wzrdtales]

Hey @benjie,

I thought about babel already once, especially to also give es7 capabilities. But, if we're going to support es6/7, then probably someone else wants typescript. Also it is questionable if typescript would have any value in a migration, but this shouldn't be a matter in this dicussion.

What I have had in mind the last days was that I wanted to introduce a bit more extensibility of db-migrate, by adding a plugin system. This would solve also some of the other issues, that want things, that won't be included in db-migrate. For example I have been asked once to introduce yaml support for configurations, while I definitely do not want to support this, as I don't want huge bloated project just to support some fancy edge cases, and creating a module that is so awkwardy slow like npm is (at least the bootstrap time of npm is horrible, which is half a second), I still would like to give the ability to like you said OptIn into such features, so possibly plugins would be a good fit for this.

Let me review this later again and give you a more detailed answer this evening.

[benjie]

@wzrdtales Cheers for giving this a look.

Another option, potentially, is to give db-migrate a --require command line switch that can be used to pull in these dependencies (e.g. typescript-register / coffee-script/register / babel-register / etc). You'd want to allow it to be specified multiple times for long-running projects that switch from one language to another.

Of course if you do that you'd need to re-think the filenames regexp, though you could make that a command line flag such as: db-migrate --require babel-register --require typescript-register --add-file-ext .ts up; but that gets a bit wordy. Perhaps a db-migrate configuration file for storing common command line switches to use throughout the project (like mocha.opts)?

Anyway, This PR is so simple you could potentially add typescript-register in the same way and just say the feature's in alpha.

[wzrdtales]

@benjie I wont merge this, but we talked about the plugins already, those are about to be introduced. In fact basic plugin support does exist already on the master branch (not yet published), so you will be able to publish this as a plugin instead. I will keep this item open and let you know how to publish this as a plugin as soon as this is official.

Basically those plugins will be really simple, just see https://github.com/db-migrate/plugin-yaml/blob/master/index.js

[benjie]

Cool 👍

[wzrdtales]

Hook for registering a precompiler together with adding a file extension to be included is now added.

See this for an example:
https://github.com/db-migrate/plugin-coffee

In your case, you can also just return null instead as you don't register any new file extensions.

[wzrdtales]

To help developers there is now also a module called db-migrate-pkgkit which needs to be installed globally and delivers a cli to the developer accessable via the command db-migrate-pkgkit. This cli is aimed to make driver and plugin creation easier. Yet only build for plugins though. This plugin you're going to write for this addition is quite small, so you might not need it, but just wanted to let current plugin devs know.

[benjie]

Okay; finally got around to creating it:

https://github.com/benjie/db-migrate-plugin-babel

You're welcome to take ownership of it if you want.

[wzrdtales]

Cool, @benjie. Thanks for your contribution!

You are also free to keep maintaining it, but I am open to take ownership if you want to transfer it, as this plugin is really small I wouldn't mind it though.

Just one thing, why didn't you follow the standard and used loadPlugin like it is intended?
This plugin shows how it should be used:
https://github.com/db-migrate/plugin-coffee/blob/master/index.js

Was there any reason why? Just asking no offense, in this case it wouldn't make that big of a difference though. Maybe I explain the thoughts behind this method:

The loadPlugin method is there to ensure that a plugin ca be registered without loading its logic already. This was designed like it is, to avoid having some heavy or badly written modules slowing db-migrate down, before it actually executes this action, as db-migrate does register the plugins as the very first thing though. We've had for example through a contribution which added the very slow tunnel-ssh module, which slowed down db-migrate from 0.05 seconds to 0.22 seconds, this module gets now loaded at the point where it is used and we're back to 0.05 seconds again. That is a huge difference, especially for CLI applications. And 0.22 seconds is something an user already notice for a cli application though. And this is done to ensure that an user can and should delay load all modules. While this is still ensured in your plugin, I would suggest to use this method properly.

Also to note, every plugin removes the loadPlugin method after being called. In your case at this hook, this does not gets called multiple times, but for many other hooks, that would mean that every call would spend the CPU Cycles for the function call of loadPlugin again, if it was not removed.

So what is suggested is actually: Either completely omit loadPlugin so it is not going to be called, which would be no problem for your plugin, or use it, which I would always recommend as a best practice, for the reasons given above.

[benjie]

Hi Tobias,

At first I was a little confused, you seemed to be advocating for efficiency whilst also suggesting that the plugin should be implemented less efficiently (more object allocations, more CPU cycles, more memory usage). However, I guess what you're saying is that it'd be wise to add the slightly less efficient boilerplate just in case others use this plugin as a template for their own (less efficient) plugins - that seems like a fair trade-off so I've made those changes. I've also removed the unnecessary overwriting of module.exports.

Cheers,

Benjie.

[wzrdtales]

Thanks again for your effort @benjie .

[benjie]

You're still welcome to take ownership of this plugin; I'm still using it but have never needed to change it 👍

[tneumueller]

Could you guys add a minimal tutorial on how to "install" this plugin? I'm having a hard time figuring out how to actually get it to run

[tneumueller]

Ok so i figured out that it is enough to just install the corresponding npm package if its name begins with db-migrate-plugin. Now the question is, how should I add @benjie's plugin, if there is no npm package to it?

[benjie]

I think you can npm install benjie/db-migrate-plugin-babel; but I'll publish it later if it isn't already.

[tneumueller]

Wow, I never realized you can just install repos as npm packages. It works that way, thank you!

Edit:
Nevertheless this packages lacks some documentation, you should add an explanation on how to add plugins

[benjie]

I've pushed it to npm now too 👍