Stop Making EasyAdmin Fields Final
Short description of what this feature will allow to do:
When we think of integrating a new custom feature into an existing field, it's impossible because the field is final. This means that we'd have to reconstruct the field again (copy and paste) to extended and achieve the intended functionality.
Example of how to use this feature
class MyCustomChoiceField extends ChoiceField
{
// ...everything in choice exists
public function myCustomChoiceMethod()
{
// ...some complex thing I intend to do with choice field
}
}
For the moment you can use https://github.com/EasyCorp/easyadmin-no-final-plugin
@ucscode This was already discussed here:
https://github.com/EasyCorp/EasyAdminBundle/issues/4386 https://github.com/EasyCorp/EasyAdminBundle/issues/5299 https://github.com/EasyCorp/EasyAdminBundle/issues/5752 https://github.com/EasyCorp/EasyAdminBundle/issues/5830 https://github.com/EasyCorp/EasyAdminBundle/issues/5890
I think without the final keyword it is harder to change the bundles code in the future without breaking changes.
@ucscode This was already discussed here:
I think without the
finalkeyword it is harder to change the bundles code in the future without breaking changes.
I cannot stop to wonder how the future code will become hard to manage by removing final. Final prevents the code from being extended. Removing it allows extension. How that makes it harder to maintain is not clear to me.
I inspected the no-final-plugin and noticed it rewrite the vendor/ code using file_put_contents which removes the final keyword, and that doesn't make any difference from altering the original code.
Nevertheless, I cannot be certain nor be Judgemental because I don't fully know the internal workings or if there are any dependencies or logic that forcefully requires the use of final. So, I'll just accept it as it is because easy admin has undoubtedly been great and the contributors know better about the bundle internal management.
@ucscode I'm no expert on this either but one example: if final is removed the maintainer is not able to delete a protected method or change its signature without breaking backwards compatibility. Because your app would be allowed to rely on the fact that the method is there or gets called.
(Another thing is that if apps extend a class and something in the parent changes it easily can produce strange errors in the child class after upgrading the bundle. Then people create GitHub issues posting their error, and maintainers have to reproduce all the errors just to be able to close the issue which arises from inheriting and not from the bundle itself.)
@ucscode I'm no expert on this either but one example: if
finalis removed the maintainer is not able to delete aprotectedmethod or change its signature without breaking backwards compatibility. Because your app would be allowed to rely on the fact that the method is there or gets called.(Another thing is that if apps extend a class and something in the parent changes it easily can produce strange errors in the child class after upgrading the bundle. Then people create GitHub issues posting their error, and maintainers have to reproduce all the errors just to be able to close the issue which arises from inheriting and not from the bundle itself.)
Very valid points!
Honestly, EasyAdmin could simply use the @internal PHPDoc tag on all its methods, and everyone would be fine with that. If a developer decides to use such a method, it's their own responsibility. If they encounter issues later, those should just be ignored. Meanwhile, others would still be able to extend fields easily.
Another, and from a programming perspective, better approach would be to move all such protected methods into traits and make them private. Mark these traits as @internal, so that any class inheriting them has access to the methods internally but cannot expose them publicly. The parent class can still call them, everything works, and everyone stays happy.