Closed (fixed)
Project:
Drupal core
Version:
8.4.x-dev
Component:
theme system
Priority:
Normal
Category:
Bug report
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
4 Feb 2016 at 13:46 UTC
Updated:
20 Dec 2017 at 01:23 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
tstoecklerA note on the coincidental part in the issue summary: I would consistently have the include files loaded on time on the first request after a cache clear but not on subsequent requests. So for people trying to reproduce this you can add a template such as
search-result--node-search.html.twigto your theme and it will get loaded on the first request after a cache clear, and afterwards the default template will get picked up again.Comment #3
star-szrThanks for this! For the docs piece *.theme as well :)
Comment #4
afi13 commentedComment #5
tstoecklerWhy yes, for theme suggestions, we should not forget those, thanks!
(As if I would remember that those things even exist... ;-) )
Comment #6
afi13 commentedFor review
Comment #10
ajitsPatch doesn't apply anymore. Assigning it to me for a reroll.
Comment #11
ajitsPlain reroll.
Comment #13
rajeevkRe-rolling patch for 8.3.x
Comment #14
dawehnerThese hooks are also available in themes, see
given that I'm wondering whether someone could misread this documentation. It says it has to be placed in .module files, maybe change it to "must be placed in *.module or *.theme files".
Comment #15
artis commentedComment #16
artis commentedComment #17
greyghost commentedRerolled patch 13 against current 8.3 branch w/ comment change to add "*.module or *.theme files"
Comment #18
pritishkumar commentedCoding Standard Resolved "Short array syntax"
Comment #19
vegantriathleteComment #21
afi13 commentedOk for me.
Comment #22
xjmThanks for working on this and for providing better documentation so we don't make this mistake again in the future.
There's a small grammar error here -- "or" should not start a new sentence because it's a conjunction. We can fix this by simply joining the sentences and rewording it a little:
Implementations of this hook must be placed in *.module or *.theme files, or othewise ensure that the hook implementation is always available.Comment #23
rajeevkAttaching another patch with xjm suggestion and re-rolling for 8.4.x
Comment #24
rajeevkComment #25
tstoecklerPerfect, thank you!
Comment #26
xjmThis still needs some work on the grammar. "...or must otherwise make sure that the hook implementation is available at any given time."
Thanks!
Comment #27
ada hernandez commented#23 works are fixed
Comment #28
ada hernandez commentedComment #29
ada hernandez commentedthere were 2 comments with that (#26) correction
Comment #30
rajeevkThis should be fine now. Thanks.
Comment #33
xjmLooks good now, thanks!
Committed and pushed to 8.5.x.
I briefly considered not backporting it to 8.4.x -- while the file location of procedural code is not considered API (https://www.drupal.org/core/d8-bc-policy#code-locations) we have accidentally disrupted things in the past by moving code between files (so we avoid changes like that in patch releases. However, since these are hook implementations, the
.modulefile that provides them will always be available when the module is enabled -- and if the module is not enabled, the extending code would have to both be relying on a procedural code location and calling a hook implementation directly. That's two very unlikely things, neither of which are supported. So fixing the bug is definitely worth not protecting such a scenario.So, long story short, I backported this to 8.4.x as well.
Thanks everyone!
Edit: Forgot about the issue crediting crosspost, so reverting and recommitting with the correct issue credit.
Comment #38
xjmThere we go, added credit for substantive reviews. :)
Comment #40
vegantriathlete