mariusbalcytis / webpack-bundle

Bundle to Integrate Webpack into Symfony
MIT License
122 stars 36 forks source link

Bundle inheritance not working #36

Closed mreuter closed 7 years ago

mreuter commented 7 years ago

I am using some bundle inheritance in my project. Unfortunately the bundle I am inheriting from is ignored completly.

mariusbalcytis commented 7 years ago

Where do you expect it to work? If inside webpack_asset function, this would be possible, but I don't know if it would be a good idea, as it would suggest that this would work inside assets, too. Which wouldn't.

Everything inside assets is left for webpack to manage. Webpack has alias functionality inside of it - bundle just provides @AcmeApplicationBundle and similar aliases for it. It does not support multiple paths for single aliases for inheritance to work, as does Symfony.

What's your use-case exactly? Do you overwrite entry point itself of internally required assets?

mreuter commented 7 years ago

In my application bundle I have a base twig-layout that all other layouts inherit from. That base layout includes the application entrypoint using <script src="{{ webpack_asset('@AppBundle/Resources/assets/main.js') }}"></script> The templates consist of lots of small 'snippets' and each of the 'snippets' is stored in its own twig-file. To customize the application, I've got an AppThemeBundle that overrides some (or all) of these snippet-files and therefore inherits from AppBundle. That leads to the described problem. The webpack_asset in my base AppBundle is completly ignored and no javascript code is generated by webpack.

mariusbalcytis commented 7 years ago

That's not the issue I've imagined - as I understand, you have only one entry-point and there are no issues with require statements inside JavaScript code.

Are all the bundles enabled? Could you provide your maba_webpack node in config.yml? Is your base bundle enabled in AppKernel?

Asset provider just takes bundle path by it's name (all bundles are taken by default), adds configured prefix (/Resources/views) and looks for all templates (*.twig) inside these directories.

maryo commented 7 years ago

@mariusbalcytis

maba_webpack:
    enabled_bundles: [WebBundle, AdminBundle, VanioAdminBundle]

AdminBundle has VanioAdminBundle set as it's parent.

But vendor/maba/webpack-bundle/src/AssetProvider/DirectoryProvider/BundlesDirectoryProvider.php

method getDirectories()

When it calls $this->kernel->getBundle($bundleName, true) it returns AdminBundle instance for both AdminBundle and VanioAdminBundle because of the true argument (Whether to return the first bundle only or together with its descendants). So templates inside VanioAdminBundle are not parsed.

maryo commented 7 years ago
$bundles = $this->kernel->getBundle($bundleName, false);
$directory = end($bundles)->getPath() . $this->relativeDirectory;

Passing false and getting only the last $bundle from the array fixed it for me (assuming that's the one, not sure it's a rule, foreaching and asking for it's name is maybe safer, I don't know).

OFC as a workarround I use twig.additional_directories instead.

BTW I am using Symfony 3.3.

mariusbalcytis commented 7 years ago

Thanks for helping to track this down! Should be fixed, will release as 0.6.4 when travis build passes.

maryo commented 7 years ago

@mariusbalcytis That's great 👍 ... But I am thinking about it again and... Let's say there is FooBundle and BarBundle. BarBundle has FooBundle set as it's parent and both have layout.html.twig... Maybe only the layout.html.twig from BarBundle should be parsed then. And there is also Twig's namespace feature... In this case @Foo/layout.html.twig loads template from BarBundle which is the same as using FooBundle::layout.html.twig but it's possible to register more directories under the @Foo alias.

mariusbalcytis commented 7 years ago

The thing is that some templates can be overriden and some left to be at parent bundle. When including, extending or embedding in twig, templates can be relative and not necessarily use bundle aliases. This would mean that it's really difficult to find all those which should be really analysed as can possibly be used. Such cases should be rare, but would be considered as a bug.

The only downside in current approach, as I see it, is possible additional entry points, which would result in slower compilation times.

Do you have any further ideas - maybe this could be handled better?

maryo commented 7 years ago

@mariusbalcytis "The only downside in current approach, as I see it, is possible additional entry points, which would result in slower compilation times.". Yes, maybe it's not such an issue. I am not sure. At least it's better with the patch :-)

"templates can be relative" What do you mean by relative paths? Paths like ../? I didn't even know it is possible... But looks like it is.

"Do you have any further ideas - maybe this could be handled better?" Maybe remembering the whole bundle hierarchy and then traversing the directories and parsing only those templates which are not overriden inside the child bundles. But yes, there are probably some edge cases. Yeah, this issue is not simple :-/