CodeIgniter4
CodeIgniter4 copied to clipboard
codeigniter4
fix: PHP 8.2 - Allow Dynamic Properties in Database Config
Description
Dynamic properties needs to be allowed since PHP8.2 so that additional database conections can be added using registrars.
This is useful in situation where module needs to provide it’s own database connection group.
Checklist:
- [x] Securely signed commits
- [x] Component(s) with PHPDoc blocks, only if necessary or adds value
- [ ] Unit testing, with >80% coverage
- [ ] User guide updated
- [x] Conforms to style guide
@paulbalandan I've updated the PR to go on 4.5 branch but I would not call it a feature as we already have same behavior in older PHP versions.
When there is a module that provides it’s own database connection group, how do we configure the group (user/password, database name)?
@kenjis The complete database group with all parameters can be set from registrar.
.env file is processed after the registrar so it is still possible to override the parameters.
@najdanovicivan Okay, you are right. If you know the all parameters, you can set from registrar. But you don't know my database parameters.
Why do we need to add AllowDynamicProperties by default?
We can add it to app/Config/Database.php by ourselves if we want to use Dynamic Properties.
@kenjis The reason why I want this by default is to be able to easily update the framework. I try to keep mostly the original config files from the framework and my code relies a lot on the Modules.
It will be best if framework is able to work without having those config files at all and relying on default config parameters of the framework. That way when framework is updated the is no need to touch config files.
Not being able to have the dynamic properties in db config is limitation on that since I cannot just use the default parameters because in such case I wont be able to push additional db groups with registrars.
I understand that you do not want to change the default configuration files.
However, in this case you only need to add #[AllowDynamicProperties] once in the database configuration file.
Also, I don't see any problem with upgrading the framework, as tatter/patches does it quite well.
I'm not comfortable with adding #[AllowDynamicProperties] as the framework default, since PHP will forbid dynamic properties by default. Plus, using dynamic properties slows it down.
Instead of adding the property dynamically at runtime with Registrars, I think a possible solution would be to add the property to app/Config/Database.php with Publisher when you install the module.
@kenjis I did not know about the performance issues. In such case it does not make sense adding the #[AllowDynamicProperties]
I'm going to close this one.