Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
bf3f75a toe71d5dbComparee71d5db to7667365Comparechalasr commentedSep 25, 2022
What if one gets a |
HeahDude commentedSep 25, 2022 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
@chalasr I thought about adding some logic in the
|
chalasr commentedSep 25, 2022
@HeahDude Makes sense, thanks. That could be done later if needed anyways |
nicolas-grekas left a comment• edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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 forget('~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 :)
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
…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 | TODOWhile 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 inhttps://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
HeahDude commentedSep 26, 2022
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 :). |
lyrixx commentedSep 26, 2022
I like this idea too 👍🏼 I proposed that8 years ago 👼🏼 |
7484b91 tof19ff62Comparesrc/Symfony/Component/DependencyInjection/Compiler/PassConfig.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
nicolas-grekas commentedNov 24, 2022
We're wondering about how to achieve built-time parameterssince years and this is the best and only actionable idea we've had. I like it a lot personnaly: simple and effective. |
a8110a0 to80561dcComparenicolas-grekas commentedDec 5, 2022
Any other vote @symfony/mergers ? I'd like to merge this before#48469 to prevent merge conflicts. |
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
yceruto left a comment
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 toc75dbcaComparechalasr commentedDec 5, 2022
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 | MITIntroduced in#47680Not 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 prefixUse the new compile-time only prefix for build parameters.Seesymfony/symfony#47680 for the original feature implementation.Commits-------cf36166 [Performance] Update parameter to compile-time prefix
Uh oh!
There was an error while loading.Please reload this page.
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.