cakephp / twig-view

Twig View for CakePHP
MIT License
13 stars 6 forks source link

Twig returns error when doing a date filter on a Cake\I18n\Date #97

Closed mcube27 closed 1 month ago

mcube27 commented 2 months ago

I have a customer created through a CustomerFactory in a test.

It has a birthdate typed Cake\I18n\Date.

When I use the Twig file parses it, it returns an error : error: An exception has been thrown during the rendering of a template ("Failed to parse time string (23/06/2024) at position 0 (2): Unexpected character").

In the Twig file, it's written like this : {{ customer.birthdate|date("d.m.Y") }}

I am using CakePhp 5.0.8, using php 8.1, Twig-view 2.0.1

It works in Cakephp 4.5.3, Twig-view 1.0.3

markstory commented 1 month ago

What is the type of birthdate? I wasn't able to reproduce your problem using Date objects, see d98f882181a23f8626f60d2b081cc03076e06a8a

mcube27 commented 1 month ago

I finally found what makes the test fail with my error : If you add at the top of the test I18n::setLocale('fr') it fails. I do not know why yet.

othercorey commented 1 month ago

This is most likely changing the locale-aware parsing and m/d/y is not supported in that locale.

You would need to confirm the date string to a date object first instead of using the implicit conversion in twig using strtotime. I don't see any documentation around locale-aware parsing in that function, but something is.

See https://twig.symfony.com/doc/3.x/filters/date.html#date

mcube27 commented 1 month ago

All worked nicely before moving to cakephp 5

mcube27 commented 1 month ago

With a little research, if instead of \Cake\I18n\Date we use \Cake\I18n\DateTime it works. I suppose in cake 4, it worked because Date was a DateTime or something like that.

So the new Date is not parsed because it doesn't fit in the \Twig\Extension\CoreExtension::convertDate but DateTime does.

mcube27 commented 1 month ago

I found a temporary solution changing the default \Cake\I18n\Date::_toStringFormat to Y-MM-dd instead of IntlDateFormatter::SHORT.

This allow the $date = new \DateTime($date, $this->getTimezone()); in \Twig\Extension\CoreExtension::convertDate line 518 to work in all locale.

markstory commented 1 month ago

I found a temporary solution changing the default \Cake\I18n\Date::_toStringFormat to Y-MM-dd instead of IntlDateFormatter::SHORT.

This isn't something we can do in the framework though. It would cause existing applications to behave differently.

othercorey commented 1 month ago

So it was converting the I18n\Date to a string then parsing with strtotime()?

Believe you can do what you want with I18n\Date::setToStringFormat()

mcube27 commented 1 month ago

With the change from cake 4 to cake 5, something in Date changed that make the \Datetime conversion fail in other locale.

So, something could be done in Cake Date, but I could not really understand what could.

othercorey commented 1 month ago

Twig might have special logic for DateTimeInterface objects that skips our string conversion.

@markstory Looking through the Twig docs, I think that's what's happening. If it's a DateTimeInterface, |date() calls DateTimeInterface::format().

Because Chronos\Date no longer implements DateTimeInterface in Chronos 3, it's using the string conversion.

Cake\I18n\Date defaults to a locale-aware SHORT format for string conversion which gets out of sync with what strototime() can parse naturally.

othercorey commented 1 month ago

@mcube27 This is definitely an unfortunate behavior change. I am not sure how to fix this so it works implicitly. You'd have to convert the Date object to a DateTimeImmutable for passing to twig or set the default to string format as suggested.

I know that's not a great solution if you end up needing a different to string format for other places.

markstory commented 1 month ago

We could try overloading the |date filter and handle dates there 🤷