Problem/Motivation
When twig.config:auto_reload is true, the component's templates are not refreshed, they only refreshed after the change to the YML file.
This is also weird because the parsed YML file is cached separately anyway.
This issue lies in \Drupal\Core\Template\Loader\ComponentLoader::isFresh. It is checking the YML file, and if YML is not updated, it will never check the template itself.
Steps to reproduce
Set twig.config:auto_reload to true
Create component
Render component
Change the component's template
The component still renders the old contents while the usual templates are refreshed.
Proposed resolution
Always check if the template file is fresh. Not sure the YML file needs to be checked at all since its contents will be cached anyway.
Remaining tasks
-
User interface changes
-
API changes
-
Data model changes
-
Release notes snippet
-
Issue fork drupal-3456676
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #3
reinfate commentedComment #4
smustgrave commentedNice catch!
Feel this is something we should probably have test coverage for.
Comment #7
reinfate commentedAdded the test coverage for the
isFreshfunction inComponentLoaderI've removed the yml file change check, it is not needed, since the component schema will be refreshed only after the discovery cache clear anyway.
The test failures seem to be not related to this issue. I've rerun them multiple times on 11.x and 11.1.x branches, and each time different test was failed.
Comment #8
grimreaperHi,
Thanks for the bug report and the fix, I also witnessed this behavior to be in Twig debug mode and not have component refreshed after change in Twig.
Tested ok.
I just put one question on MR, so not RTBC for that.
Comment #9
grimreaperComment #10
alexpottI think if we're enabling
twig.config:auto_reloadi would expect changes to the yml or the template to result in changes automatically. Made a suggestion to make it work this way.Comment #11
reinfate commentedI don't see why we need to check the YML file.
The class is responsible specifically for the component's twig file.
Even if we assume that the YML discovery cache is disabled, the YML file itself doesn't affect the twig compilation in any way.
Unless the changes are in the twig file itself it shouldn't be recompiled therefore it is fresh.
Comment #12
pdureau commentedComment #13
grimreaperHi,
Even if we could disable YML discovery cache with
$settings['cache']['bins']['discovery'] = 'cache.backend.null';, it will disable all plugin discovery cache which could slow down development environment.So even if I would say we should not check for YML change, I agree that it could be handy to have that check.
I am not sure but with UI Patterns 2 features not yet in Core like with data normalization, you may change some prop types and it can impact the template results?
I will update the MR regarding @alexpott proposed changes, so maintainers can easily decide if they want it or not.
Comment #14
grimreaperMR updated.
Comment #15
grimreaperUnrelated fails in the pipeline. I rebased and repushed to see if that's better.
Comment #16
pdureau commentedComment #17
alexpottThe updates made by @grimreaper look great.
Comment #18
pdureau commentedGreat, I will commit.
Comment #21
pdureau commentedCommitted de5530c6 and pushed to 11.x. Thanks!
Committed 4c016c7 and pushed to 11.2.x. Thanks!