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

Cast value to string for createFromFormat #707

Merged
merged 1 commit into from
Apr 4, 2024

Conversation

kylekatarnls
Copy link
Contributor

@kylekatarnls kylekatarnls commented Mar 15, 2024

Cast $value to string explictly because DateTime::createFromFormat() expects string since PHP 8

Fix #708

Cast `$value` to `string` explictly because `DateTime::createFromFormat()` expects `string` since PHP 8
@kylekatarnls
Copy link
Contributor Author

kylekatarnls commented Mar 15, 2024

Hello 👋

I know it's not spatie/laravel-data responsibility to explicit this cast since since you don't use declare(strict_types=1) in this file and the cast happens implictly. But I have some tricky constraints with Carbon upgrade from 2 to 3. And it fails trying to do:

Carbon::createFromFormat('U', $timestamp)

With $timestamp is int.

@Elnadrion
Copy link

Is this PR going to be merged any soon? My team has exact same problem after bumping laravel-data, carbon and laravel to the next major versions

@kylekatarnls
Copy link
Contributor Author

@rubenvanassche 🙏 I'm sorry this strict type change affect your library. I will evaluate if I can do smarter validation such as accepting and casting int if the format is a single letter that expect a numeric value.

But meanwhile a cast would be the safer fix.

@Elnadrion
Copy link

@kylekatarnls as a temp solution for the issue we created custom caster:

<?php

namespace App\Casters;

use DateTimeInterface;
use Spatie\LaravelData\Casts\DateTimeInterfaceCast;
use Spatie\LaravelData\Casts\Uncastable;
use Spatie\LaravelData\Support\Creation\CreationContext;
use Spatie\LaravelData\Support\DataProperty;

class CustomDateTimeInterfaceCast extends DateTimeInterfaceCast
{
    public function cast(DataProperty $property, mixed $value, array $properties, CreationContext $context): DateTimeInterface|Uncastable
    {
        return parent::cast($property, (string)$value, $properties, $context);
    }

    public function castIterableItem(DataProperty $property, mixed $value, array $properties, CreationContext $context): DateTimeInterface|Uncastable
    {
        return parent::castIterableItem($property, (string)$value, $properties, $context);
    }
}

@rubenvanassche
Copy link
Member

Thanks!

@rubenvanassche rubenvanassche merged commit f01f389 into spatie:main Apr 4, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Can no longer use parse integer timestamp into Carbon object with Laravel 11
3 participants