Possible syntax improvements

[samdark]

Proposed new feature or change

Why

It should be as easy as possible to write and read migrations. Current syntax could be optimized.

Current syntax

<?php

declare(strict_types=1);

namespace App\Migration;

use Yiisoft\Db\Migration\MigrationBuilder;
use Yiisoft\Db\Migration\RevertibleMigrationInterface;

final class M251225221906CreateNewsTable implements RevertibleMigrationInterface
{
    public function up(MigrationBuilder $b): void
    {
        $cb = $b->columnBuilder();

        $b->createTable('news', [
            'id' => $cb::uuidPrimaryKey(),
            'title' => $cb::string()->notNull(),
            'content' => $cb::text(),
        ]);
    }

    public function down(MigrationBuilder $b): void
    {
        $b->dropTable('news');
    }
}

Proposal

1. Pass column builder as argument so there's no need to call $cb = $b->columnBuilder();
2. Better naming.

<?php

declare(strict_types=1);

namespace App\Migration;

use Yiisoft\Db\Migration\MigrationBuilder;
use Yiisoft\Db\Migration\RevertibleMigrationInterface;

final class M251225221906CreateNewsTable implements RevertibleMigrationInterface
{
    public function up(MigrationBuilder $build, string $type): void
    {
        $build->createTable('news', [
            'id' => $type::uuidPrimaryKey(),
            'title' => $type::string()->notNull(),
            'content' => $type::text(),
        ]);
    }

    public function down(MigrationBuilder $build): void
    {
        $build->dropTable('news');
    }
}

[vjik]

Another suggestion — duplicate columnBuilder() to column() and mark columnBuilder() as deprecated. In this case we can keep BC and use one parameter.

<?php

declare(strict_types=1);

namespace App\Migration;

use Yiisoft\Db\Migration\MigrationBuilder;
use Yiisoft\Db\Migration\RevertibleMigrationInterface;

final class M251225221906CreateNewsTable implements RevertibleMigrationInterface
{
    public function up(MigrationBuilder $build): void
    {
        $build->createTable('news', [
            'id' => $build->column()::uuidPrimaryKey(),
            'title' => $build->column()::string()->notNull(),
            'content' => $build->column()::text(),
        ]);
    }

    public function down(MigrationBuilder $build): void
    {
        $build->dropTable('news');
    }
}

[samdark]

Potentially we don't need $b->columnBuilder() if we make this change.

[vjik]

Potentially we don't need $b->columnBuilder() if we make this change.

1. One parameter is simpler than two.
2. $type is too common name, it's not obvious that this is a column builder.

[samdark]

@vjik $build->column() will be repeated many times.

[vjik]

final class M251225221906CreateNewsTable implements RevertibleMigrationInterface
{
    public function up(MigrationBuilder $build): void
    {
        $build->createTable('news', [
            'id' => $build->column()::uuidPrimaryKey(),
            'title' => $build->column()::string()->notNull(),
            'content' => $build->column()::text(),
        ]);
    }

    public function down(MigrationBuilder $build): void
    {
        $build->dropTable('news');
    }
}

vs

final class M251225221906CreateNewsTable implements RevertibleMigrationInterface
{
    public function up(MigrationBuilder $build, string $column): void
    {
        $build->createTable('news', [
            'id' => $column::uuidPrimaryKey(),
            'title' => $column::string()->notNull(),
            'content' => $column::text(),
        ]);
    }

    public function down(MigrationBuilder $build, string $column): void
    {
        $build->dropTable('news');
    }
}

Maybe you are right, and second option is better...
But I prefer call it $column instead of $type.

string type looks strangely here. @Tigrov Is there reason for static methods in column builder?

[samdark]

Good question about static methods. These look a bit alien.

[Tigrov]

string type looks strangely here. @Tigrov Is there reason for static methods in column builder?

The only reason for static methods is to be able to call them without creating an object ColumnBuilder::primaryKey()
But nothing prevents it from creating an object and calling them on the object new ColumnBuilder->primaryKey()

And I agree that object looks better than string here
public function up(MigrationBuilder $build, ColumnBuilder $column): void

[samdark]

Yes. I'd go for object with non-static methods if we have an instance already.

[vjik]

Yii DB don't provide a class with dynamic methods. It requires create such class before and add method that returns it. Seems, it is BC breaking for Yii DB.

[Tigrov]

What about to use new ColumnBuilder()?

[vjik]

What about to use new ColumnBuilder()?

A final constructor should be added to ensure that ColumnBuilder can be instantiated using new. This is also BC breaking.

And it will still be static methods:

final class M251225221906CreateNewsTable implements RevertibleMigrationInterface
{
    public function up(MigrationBuilder $build, ColumnBuilder $column): void
    {
        $build->createTable('news', [
            'id' => $column::uuidPrimaryKey(),
            'title' => $column::string()->notNull(),
            'content' => $column::text(),
        ]);
    }

    public function down(MigrationBuilder $build, string $column): void
    {
        $build->dropTable('news');
    }
}

[Tigrov]

Is this also BC?
public function up(MigrationBuilder $build, ?ColumnBuilder $column = null): void

You can use both static and dynamic methods.

[vjik]

BC breaking in yiisoft/db-migration will be anyway.

This is BC breaking in Yii DB:

A final constructor should be added to ensure that ColumnBuilder can be instantiated using new.

[Tigrov]

This is BC breaking in Yii DB:

A final constructor should be added to ensure that ColumnBuilder can be instantiated using new.

We can add a constructor to the driver implementation where final is used.