Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Laravel 11: Remove Doctrine DBAL #11429

Merged

Conversation

hafezdivandari
Copy link
Contributor

@hafezdivandari hafezdivandari commented Feb 15, 2024

Description

Doctrine DBAL has been removed from Laravel 11.x:

This PR removes doctrine/dbal from filament and uses following Laravel's native schema methods, that are stable since Laravel 10.38, instead:

  • Schema::getColumns($table)
  • Schema::getColumnListing($table)
  • Schema::getIndexes($table)

PS 1: It's my first time contributing to filament, please let me know if I've missed anything, and sorry about that!
PS 2: This PR targets laravel-11 branch (#10972), please let me know if you prefer anything else.
PS 3: I tried to keep the changes as minimum as possible, but this PR changes the methods' signatures on packages/support/src/Commands/Concerns/CanReadModelSchemas.php, IDK if you consider this as a breaking change or not.

Code style

  • composer cs command has been run.

Testing

  • Changes have been tested.

Documentation

  • Documentation is up-to-date.

Comment on lines 126 to 137
// 'binary', 'varbinary', 'bytea', 'image', 'blob', 'tinyblob', 'mediumblob', 'longblob' => 'binary',
// 'uuid', 'uniqueidentifier' => 'uuid',
// 'enum' => 'enum',
// 'set' => 'set',
// 'inet', 'cidr', 'macaddr', 'macaddr8' => 'string',
// 'bit', 'varbit' => 'bit',
// 'xml' => 'xml',
// 'year' => 'year',
// 'interval' => 'interval',
// 'geometry', 'geometrycollection', 'linestring', 'multilinestring', 'multipoint', 'multipolygon', 'point', 'polygon', 'box', 'circle', 'line', 'lseg', 'path' => 'geometry',
// 'geography' => 'geography',
// 'tsvector', 'tsquery' => 'text',
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These column types are also possible, I put them here for you to do the mapping as you want. For example you may want to have a select input for enum column type.

Comment on lines 147 to 150
// 'enum', 'set' => ['values' => $values],
// 'float', 'decimal', 'double' => ['precision' => (int) $values[0], 'scale' => isset($values[1]) ? (int) $values[1] : null],
// 'datetime', 'timestamp', 'time', 'interval' => ['precision' => (int) $values[0]],
// 'geometry', 'geography' => ['subtype' => $values[0] ?? $column['type_name'] ?? null, 'srid' => isset($values[1]) ? (int) $values[1] : null],
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Related to https://github.com/filamentphp/filament/pull/11429/files#r1491482327

You don't have any usage for these, but I put them here in case you wanted to add something in the future.

@zepfietje zepfietje added this to the v3 milestone Feb 16, 2024
@zepfietje zepfietje added the enhancement New feature or request label Feb 16, 2024
@danharrin danharrin changed the base branch from laravel-11 to 3.x February 23, 2024 13:13
@danharrin danharrin changed the base branch from 3.x to laravel-11 February 23, 2024 13:17
@danharrin danharrin merged commit cd2403d into filamentphp:laravel-11 Feb 23, 2024
6 of 10 checks passed
@danharrin
Copy link
Member

Thank you

@hafezdivandari hafezdivandari deleted the laravel-11-remove-doctrine-dbal branch February 23, 2024 13:29
@sandersjj
Copy link
Contributor

Hello, I am having an issue with this:

When I run this migration:

      Schema::table('products', function (Blueprint $table) {
            $table->integer('stock')->nullable()->change();
        });

I get the error that it needs Doctrine/DBAL

I am on Laravel 10.46

@danharrin
Copy link
Member

You can install DBAL in your project as per the Laravel documentation, ideally you should not be relying on the fact that Filament had it installed by default since it is also in the Laravel instructions

@sandersjj
Copy link
Contributor

I understand and I will install the package in my dev. But I find it weird. Shouldn't this PR break many more projects? Since this migration is quite up to date and is still be used in Laravel.

@danharrin
Copy link
Member

If you want to submit a PR to add it back you can, but this does feel like mostly user error

@danharrin
Copy link
Member

I'll just do it in the next release

@hafezdivandari
Copy link
Contributor Author

hafezdivandari commented Feb 29, 2024

@sandersjj on Laravel 10, you may call Schema::useNativeSchemaOperationsIfPossible() method within the boot method of your App\Providers\AppServiceProvider class. See laravel/framework#45487

@sandersjj
Copy link
Contributor

@hafezdivandari thanks this works!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants