[11.x] PrimaryKey attribute for Eloquent Models

[WendellAdriel]

Changes

This PR adds a new attribute that can be used on Eloquent Models to allow defining the Primary Key for the Model in a really simple and intuitive way. This attribute is inspired by a package I created to play around with attributes and to know more about the Eloquent Models.

Why

This makes it really easy and intuitive to customize the primary key for the models.

Instead of this:

class MyModel extends Model
{
    public $incrementing = false;

    protected $primaryKey = 'uuid';

    protected $keyType = 'string';

    // Model code here
}

We can do this:

#[PrimaryKey(name: 'uuid', type: 'string', incrementing: false)]
class MyModel extends Model
{
    // Model code here
}

If we need to just update the name for example we can do this:

#[PrimaryKey(name: 'custom_id')]
class MyModel extends Model
{
    // Model code here
}

Additional Info

The package I mentioned has some other things on it and if this PR is something that the core team think could be useful I'll be happy to bring more ideas that I implemented on the package to be discussed to be added to the framework.

[Wulfheart]

I am a huge fan of this! I think it will be easier to remember and auto-completable than the current approach.

[taylorotwell]

This is one of those PRs that potential opens the door to many more PRs. For example, defining the table for the model... the database connection... thoughts?

[WendellAdriel]

@taylorotwell
Would you accept if I made in this same PR something like this: https://wendell-adriel.gitbook.io/laravel-lift/attributes/db

A DB or Database attribute to set the table, connection and timestamps.

[Wulfheart]

This is one of those PRs that potential opens the door to many more PRs. For example, defining the table for the model... the database connection... thoughts?

@taylorotwell I feel like this would be a great opportunity to use modern PHP features and gain autocompletion on the way.

[pb30]

This feels like something that should be intentionally done to all properties, at least on Model, if not the entire framework (Job coming first to mind as another class that heavily uses properties).

If I was a newbie, it would seem weird I could use attributes for PK, but not table name, or timestamp names, or on other classes. Then I'd wonder which is the "right" way, and if I can't consistently use Attributes for everything, I'd probably stick with the properties just for code consistency.

(In general, I support switching to Attributes, but likely wouldn't switch over unless I could do it for just about everything)

[WendellAdriel]

@pb30 If support for attributes is something that @taylorotwell is thinking about, I’d be happy to help with PRs adding that to other places in the framework

[marius-mcp]

This is one of those PRs that potential opens the door to many more PRs. For example, defining the table for the model... the database connection... thoughts?

Hi. How about inheritance...?
Why is this need of changing working things just because attributes are available?
Why change something that is working fine as it is, just to be able to say that it is using latest features?
Are attributes ctrl-clickable on IDEs?
Are they having inheritance hints in IDEs?

I would vote no to this but I am sure it will get implemented just because... this is life:)

[Wulfheart]

How about inheritance...?

I am not sure what your use-case would be here. As far as I know Laravel the old way would still be available.

Why is this need of changing working things just because attributes are available? Why change something that is working fine as it is, just to be able to say that it is using latest features?

Why not write all your code in assembly? We don't need that new language called C. ;-)

Are attributes ctrl-clickable on IDEs?

Yes.

Are they having inheritance hints in IDEs?

Not sure what hints you are referring to.

[marius-mcp]

@Wulfheart
"Not sure what hints you are referring to."

UPDATE
"Why not write all your code in assembly? We don't need that new language called C. ;-)"

Attributes should be used only where they are the only solution in my opinion, not where there are better solutions.

"I am not sure what your use-case would be here. As far as I know Laravel the old way would still be available."

If you extend the model with a base model for example in which you put $incrementing to false via attributes, you need extra code to inherit that $incrementing false in the models that extend the base model if I am no mistaken.

Update 2.
Not to mention if you use both properties and attributes to define the same thing...
Sad thing is that if this gets into laravel, it will be just an overhead for the projects that will not use this feature. That remembers me of @taylorotwell 's words: "make a package for it" and in that way only who wants to use it will use it.

[WendellAdriel]

@taylorotwell following the concern about having a lot of other PRs to add support for other Attributes, I'm thinking on releasing this and other attributes as a package, so we can check the adoption by the community and revisit this later on to see if it could be integrated to the core, what do you think?

[Wulfheart]

@WendellAdriel I can only speak for myself but I am a bit wary about using a package for this as it won't be maintained by the Laravel core team. Therefore, I think it would get a lot of adoption if there was a consistent first-party solution.

[WendellAdriel]

@Wulfheart I understand, but a lot of super popular packages are also not maintained by the Core Team and used across thousands of projects.

If this is something that the Core Team don't want to include and maintain in the core right now, using a package could be a good way to see the adoption and later on if the Core Team wants to add this to the Framework I'd be happy to help to bring this to the framework or even passing to them the ownership of the package if they want to make it a first-party package.

cc: @taylorotwell

[WendellAdriel]

Here's the package: https://github.com/WendellAdriel/laravel-virtue if someone wants to take a look or give it a try.
@taylorotwell if you feel that these things can be added to the framework I'll be happy to update this PR with the attributes you want me to add.

[devajmeireles]

The model has thousands of properties, so why would we just choose the primary key for this? An example of this is the fillable property. It is one of the most declared properties, perhaps even more than the primary key. IMO, the best thing to do is keep it in the packaging, @WendellAdriel . Btw, this is a very useful package, I use it a lot. Unless Laravel accepts adding attributes for all commonly used properties.

[WendellAdriel]

@devajmeireles When I first created the PR I thought that would be a great idea.
But then when @taylorotwell brought up that this would open a door for other PRs and I gave some thought about it, that's true. People can start adding attributes everywhere and this would add an overload for the Core Team to maintain.
If this is something that they are thinking on already doing, I'm all in to help implement that.

But if that's something that they don't want to add/maintain in the framework, it's totally understandable.
That's why I wrapped up this PrimaryKey attribute alongside others in this package: https://wendell-adriel.gitbook.io/laravel-virtue

[taylorotwell]

I do think this needs to be part of a more all-encompassing approach to defining this type of stuff (properties vs. attributes).