-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
Prepend Child Bundle paths before the parent #9112
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
Imo the application level Resources should be loaded first. |
You are right. I will update shortly. |
@@ -78,6 +78,16 @@ public function load(array $configs, ContainerBuilder $container) | |||
if (is_dir($dir = dirname($reflection->getFilename()).'/Resources/views')) { | |||
$this->addTwigPath($twigFilesystemLoaderDefinition, $dir, $bundle); | |||
} | |||
|
|||
if ($reflection->hasMethod('getParent')) { |
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.
The getParent()
method is always there since it's a part of Symfony\Component\HttpKernel\Bundle\BundleInterface
. There's no need to verify its existence.
And finally, would be good to cover this with tests. |
…ding. app Resources should always override child bundle resources.
@jakzal app Resources will now take priority. I had to sort it by Child bundles (and removed prepending) to ensure that the priority is correct. |
@jakzal I am not too sure how to write tests for this use case as there will be no parent/child bundles to access from the tests? |
$reflection = new \ReflectionClass($class); | ||
|
||
$bundleInstance = $reflection->newInstance(); | ||
if (null === $parentBundle = $bundleInstance->getParent()) { |
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.
can't see the need of $parentBundle
here
…o parent bundle namespace for consitency
This PR was submitted for the 2.3-dev branch but it was merged into the 2.3 branch instead (closes #9112). Discussion ---------- Prepend Child Bundle paths before the parent This fixes #9085 Say you have AcmeDemoBundle and AppDemoBundle. AcmeDemoBundle is the parent of AppDemoBundle. If you load templates using @AcmeDemoBundle/ControllerDir/template.html.twig it means that you cannot override the template in AppDemoBundle. The patch below prepends the AppDemoBundle Resources directory to the AcmeDemo namespace. The namespace directories would not result in: ``` [AcmeDemo] => Array( [0] => [absolute-dir-here]/src/App/DemoBundle/Resources/views [1] => [absolute-dir-here]/app/Resources/AcmeDemoBundle/views [2] => [absolute-dir-here]/src/Acme/DemoBundle/Resources/views ) ``` Commits ------- 19fad88 Prepend Child Bundle paths before the parent
@fabpot I suggest reverting this for now as it is not BC. It forbids using constructor arguments in bundle classes |
👍 for reverting. I'm not sure if that patch works correctly anyway (no test case was added). |
can't |
reverted |
* 2.3: Revert "bug #9112 Prepend Child Bundle paths before the parent (trsteel88)"
* 2.4: Revert "bug #9112 Prepend Child Bundle paths before the parent (trsteel88)"
This fixes #9085
Say you have AcmeDemoBundle and AppDemoBundle. AcmeDemoBundle is the parent of AppDemoBundle.
If you load templates using @AcmeDemoBundle/ControllerDir/template.html.twig it means that you cannot override the template in AppDemoBundle. The patch below prepends the AppDemoBundle Resources directory to the AcmeDemo namespace.
The namespace directories would not result in: