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

[11.x] Adds Model::casts() method and named static methods for built-in casters #47237

Merged
merged 17 commits into from
Jun 28, 2023

Conversation

nunomaduro
Copy link
Member

@nunomaduro nunomaduro commented May 26, 2023

Depends on: #47235.

This pull request is originally created from ideas of #46649, does not have any breaking change, and it enables the usage of an new Model::casts() method and named static methods for built-in casters.

class User extends Model
{
    /**
     * The attributes that should be cast.
     */
    protected function casts(): array
    {
        return [
            'options' => AsCollection::using(OptionCollection::class),
                      // AsEncryptedCollection::using(OptionCollection::class),
                      // AsEnumArrayObject::using(OptionEnum::class),
                      // AsEnumCollection::using(OptionEnum::class),
        ];
    }
}

Note that, by enabling the casts() method, developers can easily use named static methods on the built-in casters. In addition, they may add their own named static methods to their own casters if they want. As syntax, just like AsCollection::using(OptionCollection::class) is not supported when using properties.

For convenience, support for "array" syntax, on the property and method, was also added on this pull request, so developers can also use this syntax:

    protected $casts = [
        'carOptions' => [AsCollection::class, OptionCollection::class],
    ];

    protected function casts(): array
    {
        return [
            'bookOptions' => [AsCollection::class, OptionCollection::class],
        ];
    }

Finally, some technical details: as already mentioned, this pull request does not introduce any breaking changes. Also, there are no performance drawbacks as the merging of the method and property occurs during class instantiation.

In addition, when an attribute is defined both on the property and method, the method one takes priority.

@innocenzi
Copy link
Contributor

innocenzi commented May 26, 2023

unless there is an Model has a "casts" relation, witch is very unlikely

I'm not so sure about that, it totally sounds like a possible relation name. In the previous PR, that was my main concern with the idea of having a method for allowing named constructors.

Maybe we can check the return type of the casts method, and use it for the casts feature if it's array, and otherwise ignore it? Though that might hurt performance.

That point put apart, LGTM. I'm really looking forward to the array syntax. 👍

@nunomaduro nunomaduro marked this pull request as ready for review May 26, 2023 14:26
@nunomaduro nunomaduro requested a review from taylorotwell May 26, 2023 14:27
@taylorotwell
Copy link
Member

taylorotwell commented Jun 5, 2023

@innocenzi I'm not sure if a breaking change on casts method needs to be worried about. For example, imagine the following scenario on current 10.x without even merging this PR:

CleanShot 2023-06-05 at 14 32 28@2x

The someOtherMethod method would throw an error, because you would be referencing the casts property and not the casts relationship. So, defining a casts relationship on a model is already semi-broken on 10.x and can lead to unreliable behavior and errors. Any reference to the relationship $this->casts such as $this->casts->map would break and throw an error.

@innocenzi
Copy link
Contributor

@taylorotwell I see, interesting. Seems like we can go forward with casts() then. 👍

@taylorotwell taylorotwell marked this pull request as draft June 21, 2023 23:21
@nunomaduro nunomaduro force-pushed the feat/model-casts-method branch from 7d227a8 to e812be5 Compare June 22, 2023 09:25
@nunomaduro nunomaduro changed the base branch from 10.x to master June 22, 2023 09:25
@nunomaduro nunomaduro changed the title [10.x] Adds Model::casts() method and named static methods for built-in casters [11.x] Adds Model::casts() method and named static methods for built-in casters Jun 22, 2023
@nunomaduro
Copy link
Member Author

@taylorotwell Rebased to Laravel 11.x.

@nunomaduro nunomaduro marked this pull request as ready for review June 22, 2023 09:46
@oleynikd
Copy link

Using laravel/framework v10.48.9
If I use new casts() method to cast attributes in my Model:

protected function casts(): array
{
    return [
        'starts_at' => 'datetime',
    ];
}

Then my starts_at is still a string:

php artisan tinker
MyModel::first()->starts_at
= "2024-04-24 21:19:09"

But if I use $casts property to cast attributes in my model:

protected $casts = [
    'starts_at' => 'datetime',
];

Then everythig works as expected:

php artisan tinker
MyModel::first()->starts_at
= Illuminate\Support\Carbon @1713993549 {#5208
    date: 2024-04-24 21:19:09.0 UTC (+00:00),
  }

Am I doing something wrong?

@driesvints
Copy link
Member

@oleynikd this was merged into 11.x not 10.x

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.

6 participants