LaravelPackage.com icon indicating copy to clipboard operation
LaravelPackage.com copied to clipboard

Publishing migrations - are stubs really supported?

Open judgej opened this issue 3 years ago • 6 comments

This documentation describes two ways of publishing migrations: as a complete directory of full migration scripts, or as named migration stubs.

The main Laravel documentation, right up to v8, does not describe the stubs method at all, which suggests that method may be discouraged.

Now the problem comes with checking if a stub migration has already been published. I am finding that, with L8 at least, the /database/migrations directory is not autoloaded by the framework. That means the method of determining if migrations are already loaded by looking for the class (e.g. class_exists('CreatePostsTable');) does not work - it always returns false with the result that the same migrations can get published multiple times.

I am using a small workaround in my package service provider to load the migrations of interest if they exist, so the class_exists() check can be used as documented here:

MyPackageServiceProvider:

    use CreatePostsTable;

    public function boot()
    {
        if ($this->app->runningInConsole()) {
            collect([
                'migrations/*_create_posts_table.php',
                'migrations/*_any_other_migrations_of_interest.php',
                'migrations/*.php', // or everything if you like playing dangerously
            ])
            ->flatMap(function ($item) {
                return glob(database_path($item));
            })
            ->each(function ($item) {
                include_once $item;
            });

            if (! class_exists(CreatePostsTable::class)) {
                // ...
            }

However, that feels like a dirty workaround that should not be necessary.

So what's the best approach? This workaround or similar, documented in this project? A fix in the core Laravel framework to load all current migration classes when running a package migration? Or deprecating stubs for package migrations completely (they are PHP, so are dynamic and can set object names from config anyway, without a stub)?

Using stubs is handy for having a timestamp that shows when it was published, but is that really a benefit? I switched a package to using migration stubs, thinking it was the right way and where the framework was headed. But was that a mistake?

judgej avatar Apr 02 '21 12:04 judgej

This package uses wildcards to see if the migration files have been created, without actually loading them:

https://github.com/spatie/laravel-permission/blob/master/src/PermissionServiceProvider.php#L172

However, their stubs create table namess from the config data, so they do not appear to be making use of the stub concept anyway. So again - feels like there are dirty hacks to get this working, but should the migration stubs even be a thing needing workarounds?

judgej avatar Apr 02 '21 12:04 judgej

Dear @judgej

Thanks for raising this question 👍 You are right, when you publish the migrations it will always publish a new migration for the reasons you mentioned. I don't think Laravel would like to include an autoload of the database/migrations directory by default. This happens for the example package but I see that also is the case for the Spatie laravel-medialibrary for example and also your mentioned examples.

I'll look into possible solutions to get the stub method working and not republishing if a published migration already exists.

The reason I don't want to just focus on the loadMigrationsFrom() method is that you can't easily modify a package's migrations while you often might want your end users to be able to do so.

I'd like to come back to your question somewhere next week.

Jhnbrn90 avatar Apr 10 '21 18:04 Jhnbrn90

I just saw this PR to the Laravel framework in which migration classes no longer have a name but anonymously extend the base migration class.

Therefore, the check if the classname already exists will no longer work anyway.

I'll keep an eye on the development of this feature and will update the documentation accordingly.

Jhnbrn90 avatar Apr 18 '21 05:04 Jhnbrn90

That's an interesting read. Looks like duplicate migration publishing has been an issue for some time.

I have, for my package, abandoned using stubs for the migrations. Even with workarounds, there were still many cases where it gets more complex. For example, I have one project where migrations have been grouped into sub-directories for a multi-tenanted application. The tenant package may know where it has put the published migrations for tenants, but no other package is expected to know where they are.

judgej avatar Apr 18 '21 08:04 judgej

I just run into the same issue trying to setup the testing environment and saw that laravel changes the migrations to anonymous classes which is not considered yet in the doc.

@judgej did you find a way to run the migrations?

lordisp avatar Apr 18 '22 22:04 lordisp

No @lordisp , I have stayed away from stubs for now. I haven't explored using them since anonymous migrations were introduced, and it's probably worthwhile doing that. There won't be the duplicate migration class names now, but duplcate migrations could still be created. I think migrations need to protect themselves from being run multiple times, if they are created from stubs (e.g. check if the table/columns etc exist before running) so that they don't fail when run. What is improved is that the whole migration process won't fail withe the duplicate class errors.

judgej avatar Apr 19 '22 09:04 judgej