-
-
Couldn't load subscription status.
- Fork 5.3k
Replace the fluent PHP config format by the array-shape one #21511
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
base: 8.0
Are you sure you want to change the base?
Conversation
dc1e644 to
730f41f
Compare
|
PR ready. |
097460b to
776698b
Compare
|
Thanks for this! We should merge this soon to aovid merge conflicts with other PRs. However, GitHub shows errors on these 5 files, so we should fix those:
|
5372241 to
106f154
Compare
106f154 to
f86a9b6
Compare
|
PR updated after symfony/symfony#62129 |
f86a9b6 to
9fd693b
Compare
… to assist in writing and discovering app's configuration (nicolas-grekas) This PR was squashed before being merged into the 7.4 branch. Discussion ---------- [FrameworkBundle] Auto-generate `config/reference.php` to assist in writing and discovering app's configuration | Q | A | ------------- | --- | Branch? | 7.4 | Bug fix? | no | New feature? | yes | Deprecations? | no | Doc PR | symfony/symfony-docs#21511 | License | MIT This PR reverts #61490 and #61885, and builds on #61894. These reverts explain a big chunk of the attached patch. This adds a compiler pass that generates a `config/reference.php` file. This file contains two classes that define array-shapes for app's and routing configuration. Part of these shapes are auto-generated from the list of bundles found in `config/bundles.php`. The `config/reference.php` file should be loaded by a new line in the "autoload" entry of composer.json files: `"classmap": ["config/"]` - recipe update pending. This means that the file should be committed. This is on purpose: as the name suggests, this file is also a config reference for human readers. Having to commit the changes is also a nice way to convey config improvements to the community - at least for ppl that review their commits ;). It also solves a discovery problem that happens with phpstan/etc having a hard time to find the classes currently generated for config builders in the cache directory. With this PR, `config/services.php` could start as such: ```php <?php namespace Symfony\Component\DependencyInjection\Loader\Configurator; return App::config([ 'services' => [ 'App\\' => [ 'resource' => '../src/' ], ], ]); ``` and `config/routes.php` would start as: ```php <?php namespace Symfony\Component\Routing\Loader\Configurator; return Routes::config([ 'controllers' => [ 'resource' => 'attributes', 'type' => 'tagged_services', ] ]); ``` The generated shapes use advanced features that are not fully supported by phpstan / phpstorm. But the gap should be closed soon. PS: https://symfony.com/blog/new-in-symfony-7-4-deprecated-xml-configuration will need an update. Commits ------- 22349f5 [FrameworkBundle] Auto-generate `config/reference.php` to assist in writing and discovering app's configuration
2ae8fe4 to
a042e8c
Compare
|
I replaced examples that used |
a042e8c to
93757d2
Compare
| twig: | ||
| strict_variables: true | ||
| # config/packages/twig.yaml | ||
| when@test: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice update 👍🏻
Consistent with the Twig recipe.
controller/error_pages.rst
Outdated
| return App::config([ | ||
| 'content' => 'This is my custom problem normalizer.', | ||
| 'exception'=> [ | ||
| 'message' => $exception->getMessage(), | ||
| 'code' => $exception->getStatusCode(), | ||
| ], | ||
| ]; | ||
| ]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not a config file.
| return App::config([ | |
| 'content' => 'This is my custom problem normalizer.', | |
| 'exception'=> [ | |
| 'message' => $exception->getMessage(), | |
| 'code' => $exception->getStatusCode(), | |
| ], | |
| ]; | |
| ]); | |
| return [ | |
| 'content' => 'This is my custom problem normalizer.', | |
| 'exception'=> [ | |
| 'message' => $exception->getMessage(), | |
| 'code' => $exception->getStatusCode(), | |
| ], | |
| ]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I stopped at "security/".
| 'cache' => [ | ||
| 'pools' => [ | ||
| 'cache.mycache' => [ | ||
| 'adapter' => 'cache.adapter.redis', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a service id, but we could update the config to accept a Reference: symfony/symfony#62142
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not convinced we should diverge from what's possible in yaml.
| 'services' => [ | ||
| AvatarPackage::class => [ | ||
| 'tags' => [ | ||
| ['assets.package' => ['package' => 'avatars']], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Both syntaxes work. which one should be documented?
| ['assets.package' => ['package' => 'avatars']], | |
| ['name' => 'assets.package', 'package' => 'avatars'], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I documented only the first in the shape for two reasons:
array{name: string, ...<string, mixed>}isn't supported so this would break the type- only the first allows attributes with key
name.
93757d2 to
aadb453
Compare
|
Thanks @GromNaN, comments addressed. |
|
Just asking: the "code blocks" check is reporting many issues (https://github.com/symfony/symfony-docs/actions/runs/18777842138/job/53576516241?pr=21511) but I don't know if they are legit or just false positives because the review tooling is not ready to review this new format properly? |
Related to
symfony/symfony#62092symfony/symfony#62129 and https://symfony.com/blog/new-in-symfony-7-4-deprecated-xml-configurationThe diff itself isn't really interesting. What's interesting is comparing yaml to php arrays.
This can give a nice overview of the new format.