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

Feature request: Support Symfony Forms #2947

Open
michnovka opened this issue Feb 13, 2024 · 4 comments
Open

Feature request: Support Symfony Forms #2947

michnovka opened this issue Feb 13, 2024 · 4 comments

Comments

@michnovka
Copy link

Hello,

I encountered an issue with the following code - when using Carbon with Symfony, setting up Doctrine types is one thing and works fine. But using Carbon inside forms results in errors.

$builder ->add(
    'time',
    \Symfony\Component\Form\Extension\Core\Type\DateTimeType::class,
    [
        'widget' => 'single_text',
        'label' => 'Time *',
        'required' => false,
        'html5' => false,
     ]
);

Carbon version: 3.0.2

PHP version: 8.2.1

I expected to get a working form on submission that populates by Entity's Carbon type field, but instead I get:

image

Since Carbon claims Symfony support, I would expect it to provide CarbonType Form field for Symfony, just like it provides one for Doctrine. Otherwise, imo the support of this package is for Doctrine only, not for Symfony, given that basic functionality like forms does not exist.

The solution seems simple - we might just extend the DateTimeType form type and make a custom DataTransformer. The DateTimeType has input field and we could just make a simple CarbonImmutableToDateTimeTransformer and CarbonToDateTimeTransformer and add them as model transformers into the chain. Same trick is used by Symfony when you want a datetime_immutable type - internally it works with DateTime all the time, but at the very end, it converts to DateTime to DateTimeImmutable using DateTimeImmutableToDateTimeTransformer

Thanks!

@michnovka
Copy link
Author

FYI I managed to make it work even without adding a custom form type, just extending datetime:

<?php

declare(strict_types=1);

namespace App\Form\Extension;

use App\Form\DataTransformer\CarbonImmutableToDateTimeTransformer;
use App\Form\DataTransformer\CarbonToDateTimeTransformer;
use Symfony\Component\Form\AbstractTypeExtension;
use Symfony\Component\Form\Extension\Core\Type\DateTimeType;
use Symfony\Component\Form\Extension\Core\Type\DateType;
use Symfony\Component\Form\Extension\Core\Type\TimeType;
use Symfony\Component\Form\FormBuilderInterface;
use Symfony\Component\OptionsResolver\OptionsResolver;

class CarbonExtension extends AbstractTypeExtension
{
    /**
     * {@inheritdoc}
     */
    public function configureOptions(OptionsResolver $resolver): void
    {
        $resolver->addAllowedValues('input', ['carbon', 'carbon_immutable']);
        $resolver->setDefault('input', 'carbon');
    }

    /**
     * {@inheritdoc}
     */
    public function buildForm(FormBuilderInterface $builder, array $options): void
    {
        if ('carbon_immutable' === $options['input']) {
            $builder->addModelTransformer(new CarbonImmutableToDateTimeTransformer());
        } elseif ('carbon' === $options['input']) {
            $builder->addModelTransformer(new CarbonToDateTimeTransformer());
        }
    }

    /**
     * {@inheritdoc}
     */
    public static function getExtendedTypes(): iterable
    {
        return [
            DateType::class,
            DateTimeType::class,
            TimeType::class,
        ];
    }
}
<?php

declare(strict_types=1);

namespace App\Form\DataTransformer;

use Carbon\CarbonImmutable;
use DateTime;
use Symfony\Component\Form\DataTransformerInterface;
use Symfony\Component\Form\Exception\TransformationFailedException;

/**
 * @implements DataTransformerInterface<CarbonImmutable, DateTime>
 */
final class CarbonImmutableToDateTimeTransformer implements DataTransformerInterface
{
    /**
     * Transforms a CarbonImmutable into a DateTime object.
     *
     * @param CarbonImmutable|null $value A CarbonImmutable object
     *
     * @throws TransformationFailedException If the given value is not a \DateTimeImmutable
     */
    public function transform(mixed $value): ?DateTime
    {
        if (null === $value) {
            return null;
        }

        if (!$value instanceof CarbonImmutable) {
            throw new TransformationFailedException('Expected a CarbonImmutable.');
        }

        return $value->toDateTime();
    }

    /**
     * Transforms a DateTime object into a CarbonImmutable object.
     *
     * @param DateTime|null $value A DateTime object
     *
     * @throws TransformationFailedException If the given value is not a \DateTime
     */
    public function reverseTransform(mixed $value): ?CarbonImmutable
    {
        if (null === $value) {
            return null;
        }

        if (!$value instanceof DateTime) {
            throw new TransformationFailedException('Expected a \DateTime.');
        }

        return CarbonImmutable::createFromMutable($value);
    }
}
<?php

declare(strict_types=1);

namespace App\Form\DataTransformer;

use Carbon\Carbon;
use DateTime;
use Symfony\Component\Form\DataTransformerInterface;
use Symfony\Component\Form\Exception\TransformationFailedException;

/**
 * @implements DataTransformerInterface<Carbon, DateTime>
 */
final class CarbonToDateTimeTransformer implements DataTransformerInterface
{
    /**
     * Transforms a Carbon into a DateTime object.
     *
     * @param Carbon|null $value A Carbon object
     *
     * @throws TransformationFailedException If the given value is not a \DateTimeImmutable
     */
    public function transform(mixed $value): ?DateTime
    {
        if (null === $value) {
            return null;
        }

        if (!$value instanceof Carbon) {
            throw new TransformationFailedException('Expected a Carbon.');
        }

        return $value->toDateTime();
    }

    /**
     * Transforms a DateTime object into a Carbon object.
     *
     * @param DateTime|null $value A DateTime object
     *
     * @throws TransformationFailedException If the given value is not a \DateTime
     */
    public function reverseTransform(mixed $value): ?Carbon
    {
        if (null === $value) {
            return null;
        }

        if (!$value instanceof DateTime) {
            throw new TransformationFailedException('Expected a \DateTime.');
        }

        return Carbon::instance($value);
    }
}

This will add the option to use carbon as model data for symfony forms. I think it should be implemented so that others can use Carbon in Symfony forms easily.

@kylekatarnls
Copy link
Collaborator

Hello,

I would have to dig deeper in symfony/form but as Carbon extends DateTime and CarbonImmutable extends DateTimeImmutable I would expect that you can pass:

  • any Carbon object where DateTime is expected by Symfony
  • any CarbonImmutable object where DateTimeImmutable is expected by Symfony

Without needing any override/custom extension.

I would also expect that Symfony would provide types that just are compatible with DateTimeInterface and so they would be compatible with all of those classes.

However converting CarbonImmutable to DateTime (or Carbon to DateTimeImmutable) feels wrong, and I hope it could be avoidable.

@kylekatarnls
Copy link
Collaborator

Side note, it's still very early for 4.x plans, but I think we'll rather move to less things embedded by default (so to releases constraints for those who don't need it), and for instance carbon would be installable without the doctrine type.

So I think it might also be better to have symfony-form-carbon-type as a separated bridge package.

BTW if this mutable vs. immutable question could be investigated and understood and that you would be up to provide the code above as a package and publish it under your own name, I would definitely promote it in the documentation under Symfony section.

@michnovka
Copy link
Author

@kylekatarnls

I would have to dig deeper in symfony/form but as Carbon extends DateTime and CarbonImmutable extends DateTimeImmutable I would expect that you can pass:

  • any Carbon object where DateTime is expected by Symfony
  • any CarbonImmutable object where DateTimeImmutable is expected by Symfony

no, this is not true, you should read more in the docs here. It explains nicely the concept of Model/Norm/View data in Symfony forms.

The issue stems from the Model Data being of DateTime type (not DateTimeInterface). Note that the Norm data for all DateTimeType related types (including TimeType and DateType) is DateTime. There is a form option for all these form types called input, which specified the Model data type and by default is datetime (meaning DateTime is used), but can be set to various other options, including datetime_immutable (as expected, then DateTimeImmutable is used) <- but this affects ONLY the Model data, not the Norm data.

This is fine however, if you look at my code, I mimic exactly what Symfony does natively for datetime_immutable type, as can be seen here.

However converting CarbonImmutable to DateTime (or Carbon to DateTimeImmutable) feels wrong, and I hope it could be avoidable.

So, since this is only about the model data (as we dont want to change symfony internals that do work with DateTime anyways in the norm layer), I think my solution is the best, as it allows you to use the default Symfony types with all their rich features. Recoding a totally custom CarbonType would result in tons of code repetition and there would be 0 benefit added.

I do agree this should be part of a symfony bundle package and not core carbon package. Similarly, I dont understand why current carbon composer.json requires carbonphp/carbon-doctrine-types, and its not an optional dependency that one would use only if Doctrine is in the project.

BTW if this mutable vs. immutable question could be investigated and understood and that you would be up to provide the code above as a package and publish it under your own name, I would definitely promote it in the documentation under Symfony section.

I am happy to help with the development, however I think the code above really does provide 100% of the functionality needed, all we need to do is wrap it in the bundle. I would also prefer if this were under carbonphp package and repos, and not under my name.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants