Closed liqueurdetoile closed 7 months ago
We're in the middle of the river with current setup 😬 Either, the default bootstrap should be updated (and my unit tests would have quick failed), either, default phpstan config should be updated :
Updating the phpstan configuration seems like the lowest friction change here. Removing functions.php
from application startup is another viable option. I don't prefer that approach because we still have a lot of documentation and examples that are using global functions and I'd prefer to not create confusion for new folks.
For migration ease, they can be split in an optional compatibility plugin so it won't lure IDE with unrequired globals if typing __( too quickly if plugin is not installed.
This is a good option as well. Moving global functions out into a package seems like a good change to make for cake 6. We can't really move it for 5.x as that would break compatibility outside of a major release which is something we try to avoid doing.
We should update the phpstan config.
Even going forth I would prefer the global functions be opt-out (as they are now) instead of opt-in. The primary reason I namespaced all the functions was to avoid potential conflicts with other frameworks/libs (when using split packages for e.g), not to remove the convenience for CakePHP users.
@ADmad You're definitely right if keeping globals in core in order to be consistent with autocompletion and I also agree with @markstory that moving them to a package should require a major release.
@liqueurdetoile Can you please make a PR updating the phpstan config?
@ADmad Sure. Should I also propose a PR for docs ?
This is a (multiple allowed):
[x] bug
[ ] enhancement
[ ] feature-discussion (RFC)
CakePHP Application Skeleton Version: 5.0.1
Platform and Target: PHP 8.2.11
Hey there,
Just starting with a fresh 5x app install, I was facing a phpstan error with the global translation function
__
:Curiously my unit tests were working fine and not complaining about global
__
use. Wandering why, I've found out that my default app'sconfig/bootstrap.php
still hasrequire CAKE . 'functions.php';
.I've read once (and forgot till now) in the migration guide that
We're in the middle of the river with current setup 😬 Either, the default bootstrap should be updated (and my unit tests would have quick failed), either, default phpstan config should be updated :
Finally, it's also possible to get rid of these globals as they are now defined in namespace. For migration ease, they can be split in an optional compatibility plugin so it won't lure IDE with unrequired globals if typing
__(
too quickly if plugin is not installed.It's a really quick and naive approach and I'm certainly missing many elements to say what is the best option. If using an IDE, last one seems to be the best though.
By the way, you've made a hard work with CakePHP 5.x. Thanks !