Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
Adding the Form default theme files to be warmed up in Twig's cache#24608
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
fabpot commentedOct 18, 2017
Can't it be merged in 2.7 as well? |
| $coreThemePath =dirname(dirname($reflClass->getFileName())).'/Resources/views/Form'; | ||
| $container->getDefinition('twig.loader.native_filesystem')->addMethodCall('addPath',array($coreThemePath)); | ||
| $paths =$container->getParameter('twig.cache_warmer_paths'); |
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 don't like the use of a parameter. That exposes internal details.
3e346d9 to2ef619fCompareweaverryan commentedOct 19, 2017 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Ok, fixed back to not introduce a new parameter. I just tried again, it still works fine. This applies only to 2.8 and above. In 2.7, the cache warmer doesn't have a third (paths) argument at all:https://github.com/symfony/symfony/blob/2.7/src/Symfony/Bundle/TwigBundle/Resources/config/twig.xml#L48 So... it could technically be fixed in 2.7, but would be much different than this patch... the paths arg from 2.8 would need to be backported. Should we? |
fabpot commentedOct 19, 2017
Thank you@weaverryan. |
…g's cache (weaverryan)This PR was merged into the 2.8 branch.Discussion----------Adding the Form default theme files to be warmed up in Twig's cache| Q | A| ------------- | ---| Branch? | 2.8| Bug fix? | yes| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets | none| License | MIT| Doc PR | not neededHiya guys!So..... during a Symfony Live workshop, we found out that the form theme Twig templates are *not* included in the Twig cache warmup process. This fixes that. I believe this is the only "weird" case where we use a Twig template that is not in a bundle and also not added to Twig as a proper namespaces.I tested this on a 2.8 project. Before the patch, the form theme templates were not warmed up. After, they are warmed up. Booya.Cheers!Commits-------2ef619f Adding the Form default theme files to be warmed up in Twig's cache
Uh oh!
There was an error while loading.Please reload this page.
Hiya guys!
So..... during a Symfony Live workshop, we found out that the form theme Twig templates arenot included in the Twig cache warmup process. This fixes that. I believe this is the only "weird" case where we use a Twig template that is not in a bundle and also not added to Twig as a proper namespaces.
I tested this on a 2.8 project. Before the patch, the form theme templates were not warmed up. After, they are warmed up. Booya.
Cheers!