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] Always preload services classes tagged with container.preload#36742
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
| $dump =str_replace('\\\\Fixtures\\\\includes\\\\foo.php','/Fixtures/includes/foo.php',$dump); | ||
| } | ||
| $this->assertStringMatchesFormatFile(self::$fixturesPath.'/php/services9_as_files.txt',$dump); | ||
| $this->assertStringEqualsFile(self::$fixturesPath.'/php/services9_as_files.txt',$dump); |
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.
We can't use a regex anymore because the file to compare to have become too big (preg_match(): Compilation failed: regular expression is too large at offset 35752). I guess this strategy is useful when merging from lower branches? Do someone have a better idea than strict comparison or creating a new file? 😕
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.
Tests are failing: I need to recheck the strategy.
| ->addTag('container.preload', ['class' =>'Some\Sidekick1']) | ||
| ->addTag('container.preload', ['class' =>'Some\Sidekick2']); | ||
| $container->register('no_preload','TestNoPreload') |
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 also added a test withcontainer.no_preload as I think it was not tested.
nicolas-grekas commentedMay 7, 2020
I feel like we might not need this PR after#36749 At least the case we built this solution for ( Agreed? |
fancyweb commentedMay 9, 2020
I agree, it's better. |
This PR aims to solve the case where you want to force the preload of a service class even if the service definition has the
container.no_preloadtag.For example, in
RegisterListenersPass, we add thecontainer.no_preloadtag on definitions that listen to some particular events. What if I want to force the preload of those services classes anyway?Adding the
container.preloadtag does not work because the "no preload" behavior has the priority. I propose to change that so one can simply add the tag when he wants to override Symfony's default behavior.Moreover, to not repeat the service class on the
container.preloadclassattribute, let's make this attribute optional. When it's not there, it means I want to preload the service class.