Type: Issue
Severity: Critical

❌ Bug: Invalid class constant usage

The original XML uses type="datetime" (lowercase string), but the PHP version uses \datetime::class. This will fail because:

1. \datetime should be \DateTime (capital D) if it were a class reference
2. But it shouldn't be a class reference at all - it should be the string 'datetime'

Current code:

->tag('sylius.grid_field', ['type' => \datetime::class]);

Should be:

->tag('sylius.grid_field', ['type' => 'datetime']);

yes, I agree, that's not good.

Type: Issue
Severity: Medium

⚠️ Type Safety Issue: Driver doesn't accept null

After reviewing the Sylius\Bundle\GridBundle\Doctrine\ORM\Driver class, there's a type mismatch. The Driver's constructor requires a non-nullable ManagerRegistry parameter:

public function __construct(ManagerRegistry $managerRegistry)

However, the service configuration uses ->nullOnInvalid(), which can pass null when the doctrine service is not available. This will cause a type error at runtime if Doctrine is not installed.

Note: The PHPCR driver correctly handles this by accepting a nullable parameter: public function __construct(private ?DocumentManagerInterface $documentManager = null) and checking for null in the getDataSource() method.

Recommendation: Either remove ->nullOnInvalid() (since Doctrine is a required dependency), or update the Driver class to accept nullable ManagerRegistry and add proper null checks.

Fixed.

Type: Nitpick

💡 Suggestion: Consider using ::class constant for better IDE support and refactoring safety.

Current:

$services->set('sylius.grid.data_extractor.property_access', 'Sylius\Component\Grid\DataExtractor\PropertyAccessDataExtractor')

Suggested:

use Sylius\Component\Grid\DataExtractor\PropertyAccessDataExtractor;

$services->set('sylius.grid.data_extractor.property_access', PropertyAccessDataExtractor::class)

This would make the code more maintainable and catch typos at parse time rather than runtime.

Yes, done with AI

Type: Suggestion

💡 Code Quality: Consider adding use statements at the top of the file for all the classes being referenced. This would make the file more readable and easier to maintain.

Example:

use Sylius\Component\Grid\FieldTypes\CallableFieldType;
use Sylius\Component\Grid\FieldTypes\DatetimeFieldType;
use Sylius\Component\Grid\FieldTypes\StringFieldType;
use Sylius\Component\Grid\FieldTypes\EnumFieldType;

Then you could use CallableFieldType::class instead of the full string.

Done (with AI 👍)

Type: Suggestion

💡 Code Quality: There's a lot of repetition in the filter definitions (lines 35-110). Each filter follows the same pattern:

1. Define the filter service with a tag
2. Create an alias
3. Define the form type
4. Create another alias

Consider extracting this into a helper function or loop to reduce duplication:

$filters = [
    'string' => ['filter' => StringFilter::class, 'form' => StringFilterType::class],
    'boolean' => ['filter' => BooleanFilter::class, 'form' => BooleanFilterType::class],
    // ...
];

foreach ($filters as $type => $classes) {
    // Register filter and form type
}

This would make the code more maintainable and less error-prone.

Interesting, but it should be done in another PR to keep it simple to review.
We should open an issue instead.

#441