laravel / ideas

Issues board used for Laravel internals discussions.
938 stars 28 forks source link

Get rid of migrations order magic and consistency with seeders #1296

Closed autaut03 closed 5 years ago

autaut03 commented 6 years ago

Hello. There have been issues related to next major Laravel release (so called 7.0), and this is one of them.

Description

Currently, Laravel sorts migrations by date, prepended to migration's filename. For a small project that's an easy solution, allowing us to use artisan make:migration without any hassle. However, when third-party packages come in place, it all leads to naming/renaming files manually as 2000_01_01_000001_whatever_name_here.php to keep the order correct while not loosing the readability. As the project grows, some may choose to use a modular structure. To keep things looking good, you would have to develop own naming scheme to not mess up when creating migrations in yet another module. Unfortunately, people rarely follow such things as they may seem unimportant, so in best case scenario you'll have a bunch of randomly named migrations that somehow work.

Possible solution

What I propose is to make migrations work exactly the same way as seeders. There would be a main "Migrator" coming in laravel/laravel repo that can register migrations in any order developer wants. Migrations classes would no longer be under global namespace and files would be named accordingly.

Pros

Cons

I'd like to hear your feedback on this. I may have missed something important, but I'm open for a discussion.

Thanks regardless.

sisve commented 6 years ago

Could you show us an example implementation of this?

Your list of pros includes autoloading and namespaces; this sounds like two points for the same thing. One could argue that the modular projects is also included into the namespaces point. You also mention better control of migrations flow, and filenames no longer responsible for order. That also sounds like two points describing the same thing.

We could extend the list of cons in a similar fashion. Not only is this a breaking change, it requires changes to existing laravel packages, changes to existing laravel projects, changes to userland code, changes to existing tutorials. That's a lot of cons! (And it's not only harder for beginners, it's also harder for newcomers, for novices, for rookies, ...)

On a more serious note;

  1. If you do not scan the migrations directory, how would you show the developer a list of all migrations that hasn't been applied? (The migrate:status command.)
  2. Are we expected to manually maintain a list of all migrations? If so, how is this different from having them as files in a directory, as it is today?

You can modify the mapping between filename and classname by overriding the Migrator::resolve method. You can also change the order of migrations in Migrator::getMigrationFiles to use whatever logic you need. This can be done today without any breaking changes (assuming you keep backward compatibility).

I agree that namespacing would be nice to have. It's slightly annoying to have two migrations called AlterUsersTable, one for my application and one for the forum component I just installed. We could use namespaces to solve this, but it will still mean we can never reuse that migration name in the future. And it's a crappy name anyway. Why not just call it AlterUsersTable20180829094112 so we know it's unique? It would be easier to sort those class names in order if the timestamp is first (Laravel style) or they were all named Version+timestamp (Laravel Doctrine).

I have never built packages that provides migrations, so I don't know much about the problems those developers experience.

autaut03 commented 6 years ago

@sisve,

Example of implementing:


use App\Modules\Users\Migrations\CreateUsersTableMigration;
use App\Modules\Chat\Migrations\CreateChatMessagesTableMigration;
use App\Modules\Chat\Migrations\AlterUsersTableMigration as ChatAlterUsersTableMigration;

class DatabaseMigrator extends Migrator {
    public function register() {
        $this->register([
            CreateUsersTableMigration::class,
            CreateChatMessagesTableMigration::class,
            ChatAlterUsersTableMigration::class
        ]);
    }
}

In Migrator, we get rid of getMigrationFiles. Instead, we put a simple array of registered migrations and use it.

Autoloading-namespacing - yes, you are right.

What about modular projects, yes and no. Let me explain what I mean: modular projects also have their migrations spreaded accross the entire project. As it grows, it becomes hard to maintain order consistency as they are all in different folders and you can't clearly see which migrations come first. This is similar to namespacing, but under namespacing I actually mean the fact that "new" migrations are classes with valid filenames and namespaces, giving us an ability to do whatever we want with them.

About filenames order and better control of the flow: they are different. If we just removed dates from filenames, and, let's say, made an autoloading based on some complex dependencies system, we'd still not get that control. Under that I mean that you can easily "toggle off" a specific migration just by commenting a line, include a migration class from third-party package, make any of them conditional etc.

I understand that this is a breaking change. I'm not expecting this to appear any time soon. Just my two cents in improving the framework. Something like 5.7 or 5.8 is, of course, not appropriate for such change, but in 6.0/7.0 it may fit.

Question answers:

  1. Ehm.. The same way? For Migrator, the difference is little and will not change any of current behaviour.
  2. Yes, exactly. Main differences:
    • method registers them, not magic glob(), giving us more control
    • migrations are now complete classes, not magic that is somehow resolved
    • getting rid of dates in files, letting the method have control over migrations order

Actually, I haven't thought it could be done without breaking changes, that's a good idea. This would solve all of the problems, except for publishing third-party migrations, so I'd still like that to be the only way of defining migrations in the future (maybe in couple of years?). But for now, non-breaking change is a lot better.

What about packages, take a look at this: https://github.com/JosephSilber/bouncer/blob/master/migrations/create_bouncer_tables.php