sensiolabs / StorybookBundle

MIT License
41 stars 5 forks source link

Can't resolve multiple twig path #31

Closed Taloud closed 2 months ago

Taloud commented 3 months ago

Hello and thanks for this bundle !

I can't use it currently due to how the twigComponentResolver.ts is structured.

Twig components can use the twig.yaml paths to resolve themselves, and I have something like this in my config:

    paths:
        'templates/%my_app%': ~
        'templates/default': ~
        'templates': ~

And my twig_component.yaml looks like this:

twig_component:
    anonymous_template_directory: 'components/'
    defaults:
        App\Default\Components\: 'components/'

and more config for specific

twig_component:
    defaults:
        App\MyApp1\Components\: 'components/'

But in reality, we don't really care about the 'components/' because it will try to resolve all of my 3 paths and add components/.

But because of the twigComponentResolver.ts, if in my stories I put something like import Work from '../templates/my-app-1/components/Work.html.twig'; or import Work from '../templates/default/components/Work.html.twig';

It will crash with the error Unable to determine template name for file .... because it doesn't try to check all of my twig.yaml paths but will only try to resolve ../templates/components/Work.html.twig, and indeed, if I put my component in this folder and no subfolder it will work.. But it should check also ../templates/default/components/Work.html.twig and ../templates/my-app-1/components/Work.html.twig because of my twig.yaml conf

I will try to make a PR to use the paths config from twig.yaml.

squrious commented 3 months ago

Hello, thanks for the report!

There is actually a problem here, for components declared with #[AsTwigComponent(name: '..', template: '..')] as the naming is not predictable from the configuration... Is it your situation?

Taloud commented 3 months ago

Hello, if you specifically declare a name and a template in your Twig component, it shouldn't be a problem because you specify the path inside your template variable.

In my situation, I have multiple paths defined in my Twig configuration, and some need to override others. For example, I have two folders: templates/my-app and templates/default. Because of this configuration, Symfony knows that it will first check inside templates/my-app, and if it doesn’t find anything, it will then check inside templates/default.

This is currently working with what I tried in my MR. It just loops to try each case and not only one specific space.

But am I maybe missing something?

squrious commented 3 months ago

Ok got it, thanks!

stu-ad commented 3 months ago

Hi, firstly thanks very much for this bundle. As a FE lead joining a team with a mature Symfony app, this is exactly what I need (and was going to attempt to build until I found this!).

@Taloud - Thanks for this PR, it solves the exact problem that my team were facing while introducing Storybook to our workflow.

Would be great to see this in the next release!

Taloud commented 3 months ago

Hello, thank you for your feedback @stu-ad ! I'm glad that it can be useful for other teams as well, that's exactly why I built it.

I would love to see it included in the next release after some review.

Taloud commented 2 months ago

Resolved with https://github.com/sensiolabs/StorybookBundle/pull/32

stu-ad commented 1 month ago

Hi @squrious ,

Would it be possible to create a new release containing this change please?

squrious commented 1 month ago

Hi @stu-ad, I just published a new release :)

stu-ad commented 4 weeks ago

Hi @stu-ad, I just published a new release :)

Much appreciated! Thank you. This bundle is already proving really useful for my team. I hope to see more people using it as word spreads. I hope we can contribute in the future.

tommyvdv commented 3 weeks ago

In my case twig.paths is left empty in configuration but changed by a vendor package without me realizing it.

The change in behavior is down to this change not always adding directory templates to the list of directories to resolve.

I though it would be worth mentioning and might help anyone in a similar situation.