Skip to content

Conversation

antonkomarev
Copy link
Contributor

@antonkomarev antonkomarev commented Apr 24, 2020

In some cases you might need access to the container to ensure the proper update of your data structure. This could be necessary to update relations with some specific logic or to create new entities.

class CreateDemoTable extends Migration
{
    public function __construct(TenantManagerInterface $tenantManager)
    {
        $this->tenantManager = $tenantManager;
    }

    public function up(): void
    {
    }
}

This question was raised many times before:

@antonkomarev
Copy link
Contributor Author

antonkomarev commented Apr 24, 2020

If you are rejecting this approach - I could propose another one.
It's not so neat, but we could introduce special contract which will be used to set container via mutator method.

use Illuminate\Contracts\Container\Container;

class CreateDemoTable extends Migration implements ContainerAwareInterface
{
    private $container;

    public function setContainer(Container $container)
    {
        $this->container = $container;
    }

    public function up(): void
    {
    }
}

Personally I don't like it, but leaving it for consideration of all possible alternatives.

@antonkomarev
Copy link
Contributor Author

Container is nullable by default as @GrahamCampbell proposed. @taylorotwell if you still don't want to force all migrations to use DI - I could revert changes in service provider and keep them for Migrator class only. Then to enable DI in migrations people will only need to overwrite Migrator binding without extending and overriding class itself.

@taylorotwell
Copy link
Member

$this->tenantManager = app(TenantManagerInterface::class);

@antonkomarev antonkomarev deleted the add-migrations-constructor-di branch April 27, 2020 15:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants