rcrowe / TwigBridge

Give the power of Twig to Laravel
MIT License
895 stars 165 forks source link

Normalize event view names like Laravel #424

Closed rudiedirkx closed 2 years ago

rudiedirkx commented 2 years ago

EventNode::triggerLaravelEvents() triggers events with the given template name, not the normalized name. Laravel (in ManagesEvents::addViewEvent()) saves events with the normalized name.

In my project I use / delimiters everywhere, because it's much more readable than Laravel's ., and that works (in Laravel and in Twig) because names are normalized. Except in custom triggering events since the Twig 3 upgrade.

I feel like EventNode should do a

$viewName = ViewName::normalize($templateName);

like View\Factory::normalizeName() does. But maybe I'm too me-focused right now. Laravel does the same for the first/main template render. I call it with view('home/index') in the controller, and Laravel (not TwigBridge) calls events for home.index. TwigBridge should do the same?


Something weird happens with the first/main template though. If I add a composer('*') listener, and print the $view->name() in the callback, I get the FULL path, AND the normalized path home.index. Only for the first/main template, not the includes and layouts called further in. Sounds a bit like #154 but not quite, and that's super old.

Some illustration (dump($view->name) in my composer('*')'s callback):

p1111 icares homeblox nl_


Also, why is there a sprintf in EventNode::compile()?

rudiedirkx commented 2 years ago

422 was first LOL

I am still curious about the full path event though....

hsingh124 commented 2 months ago

@rudiedirkx I am having a similar issue with the paths that you mentioned. Was there any fix found for this? I made an issue here for it: https://github.com/rcrowe/TwigBridge/issues/438

I'm having a problem where my view composers aren't being triggered because it's using the path instead of the template name.

rudiedirkx commented 2 months ago

I only noticed it, I didn't have a problem with it, so it didn't really look into it.

hsingh124 commented 2 months ago

Thanks for the update, @rudiedirkx. Thats all good, thanks anyway.