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] [POC] Introduce build parameters#47608
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
nicolas-grekas 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.
I didn't finish my review but I'm wondering: instead of adding new methods, wouldn't it be better and simpler to have a naming convention for build-time parameters? My proposal would be to consider that any parameter who's name starts with a dot will removed from the compiled container.
parameters:.abc:def
| * | ||
| * @var array<string, false> | ||
| */ | ||
| publicarray$buildParameters = []; |
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.
better have a getter, no need to make it internal
| publicfunctionremoveParameter(string$parameter):void | ||
| { | ||
| $this->parameterBag->remove($parameter); |
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.
no need for the removeParamter method I suppose
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.
You're right, for DX purpose this could be a dedicated PR if needed.
| $c .=''.$this->doExport($id)." => true,\n"; | ||
| } | ||
| $files['removed-ids.php'] =$c."];\n"; | ||
| $files['removed-service-ids.php'] =$c."];\n"; |
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'd keet the current name and name the new oneremoved-parameters.php
HeahDude commentedSep 18, 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.
I considered it, but I'm worried about potential BC breaks this could cause. |
chalasr commentedSep 18, 2022
Might be worth adding some global config option or DI parameter as we did for the autowiring' strict mode? |
wouterj commentedSep 18, 2022
I'm not sure if I prefer convention over an explicit option. And needing an explicit option to turn on a convention in order to use this feature seems even less than ideal to me :) |
nicolas-grekas commentedSep 18, 2022
The BC break seems theoretical to me. I think that's still my preferred solution and how I'd like this topic be solved. |
HeahDude commentedSep 24, 2022
See#47680 for the suggested implementation. |
chalasr commentedSep 25, 2022
Thanks. Closing in favor of#47680 |
…meters (HeahDude)This PR was squashed before being merged into the 6.3 branch.Discussion----------[DependencyInjection][HttpKernel] Introduce build parameters| Q | A| ------------- | ---| Branch? | 6.3| Bug fix? | no| New feature? | yes| Deprecations? | yes| Tickets | ~| License | MIT| Doc PR | TODOAlternative implementation of#47608.Add a convention for parameters named with a starting dot to be available at compile-time only:```php$containerBuilder->setParameter('foo'); // nothing changes$containerBuilder->setParameter('.bar'); // will not be dumped```Calling `$container->getParameter('.bar')` on a compiled container will throw.Commits-------c75dbca [DependencyInjection][HttpKernel] Introduce build parameters
Uh oh!
There was an error while loading.Please reload this page.
We miss a way to optimise dumped container with compile-time only parameters, or ability to remove them in a straightforward/easy to discover way: