-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[DependencyInjection][HttpKernel] Introduce build parameters #47680
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
Conversation
bf3f75a
to
e71d5db
Compare
e71d5db
to
7667365
Compare
What if one gets a |
@chalasr I thought about adding some logic in the
|
@HeahDude Makes sense, thanks. That could be done later if needed anyways |
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 like that ;)
Here are some notes about the implementation.
The drawback is that parameters are going to be "public" by default, and everybody will need to add a leading dot almost all the time. On the other hand, it's not a big deal if they don't, parameters are cheap, so it's fine to me.
The next step will likely be to find a way to help the community move from public params to private ones. Here is an idea: when both 'foo' and '.foo' exist, we could consider that this means that using 'foo' at runtime is deprecated. This might help bundles adopt private params.
And another step could be to find a way to completely deprecate a parameter. An idea: params starting-and-ending with a ~
would mean that: get('foo')
would lookup for get('~foo~')
and trigger a deprecation. That'd work with private and public params of course.
Either for this PR, or another one, as you want, if you want :)
…s (HeahDude) This PR was merged into the 6.2 branch. Discussion ---------- [DependencyInjection] Deprecate numeric parameter names | Q | A | ------------- | --- | Branch? | 6.2 | Bug fix? | no | New feature? | no | Deprecations? | yes | Tickets | ~ | License | MIT | Doc PR | TODO While trying to use `'.' !== $key[0]` instead of `str_starts_with($key, '.')` in #47680, I noticed some tests were failing due to the usage of numeric parameter names in the fixtures. This leads to inconsistent behavior since the following code: `$parameterBag->set(10, 10)`, will first cast the name `10` to string because of the method signature, but will then cast back to integer when using it as an array key in https://github.com/symfony/symfony/blob/6.2/src/Symfony/Component/DependencyInjection/ParameterBag/ParameterBag.php#L89. Because Symfony does not use strict types, it can be really tricky. This PR propose to deprecate using such names to be able to properly throw in 7.0. Commits ------- 3e0050a [DependencyInjection] Deprecate numeric parameter names
Before working on this, my intention was to work on deprecating parameters (https://symfony-devs.slack.com/archives/C8WHX21K7/p1663341791619939), so I definitely agree that this should be the next step for another PR :). |
I like this idea too 👍🏼 I proposed that 8 years ago 👼🏼 |
7484b91
to
f19ff62
Compare
src/Symfony/Component/DependencyInjection/Compiler/PassConfig.php
Outdated
Show resolved
Hide resolved
We're wondering about how to achieve built-time parameters since years and this is the best and only actionable idea we've had. I like it a lot personnaly: simple and effective. |
a8110a0
to
80561dc
Compare
Any other vote @symfony/mergers ? I'd like to merge this before #48469 to prevent merge conflicts. |
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 addition!
c32f55b
to
c75dbca
Compare
Thank you @HeahDude. |
This PR was merged into the 6.3 branch. Discussion ---------- [HttpKernel] fix wrong deprecation message | Q | A | ------------- | --- | Branch? | 6.3 | Bug fix? | yes | New feature? | no | License | MIT Introduced in #47680 Not sure about how to catch the deprecation. Commits ------- 6bfc18d [HttpKernel] fix wrong deprecation message
This PR was merged into the 6.3 branch. Discussion ---------- Update parameter to compile-time prefix Use the new compile-time only prefix for build parameters. See symfony/symfony#47680 for the original feature implementation. Commits ------- cf36166 [Performance] Update parameter to compile-time prefix
Alternative implementation of #47608.
Add a convention for parameters named with a starting dot to be available at compile-time only:
Calling
$container->getParameter('.bar')
on a compiled container will throw.