Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
When theme has node_modules
directory scanning goes long and shows false positives
Steps to reproduce
STATUS LINE MESSAGE
--------------------------------------------------------------------------------
Check manually 0 The spaceless tag in
"themes/custom/custom_theme/node_modules/twig-loader/test/fixtures/in
clude/template.html.twig" at line 19 is deprecated since
Twig 2.7, use the "spaceless" filter with the "apply" tag
instead. See https://drupal.org/node/3071078.
Proposed resolution
Add node_modules
to exclude like https://github.com/mglaman/drupal-check/pull/184
Remaining tasks
patch, review. commit
User interface changes
no
API changes
no
Data model changes
no
Comment | File | Size | Author |
---|---|---|---|
#24 | 3162997-test-only.patch | 896 bytes | Gábor Hojtsy |
#23 | interdiff.txt | 896 bytes | Gábor Hojtsy |
#23 | 3162997.patch | 4.1 KB | Gábor Hojtsy |
#20 | 3162997-20.patch | 3.22 KB | vacho |
#13 | 3162997-13.patch | 3.28 KB | andypost |
Comments
Comment #2
andypostSimple patch, not sure it could be covered with tests
Patch applies to 8.x-2.x as well
Comment #3
andypostAdded example to steps to reproduce with custom theme
Comment #4
andypostComment #5
andypostIt needs work because twig provides no way to exclude directories from scan
Filed
- https://github.com/twigphp/Twig/issues/3382
- https://github.com/zimmo-be/twig-loader/pull/59
Comment #6
andypostWe could use Finder to exclude core provided directories
Comment #8
andypostThere's second place where templates are discovered, maybe it makes sense to create separate helper method (could be static)
Comment #9
uzlov CreditAttribution: uzlov at Skilld commentedcool
one note
can be more simple
just return ...
Comment #10
andypostThe https://www.php.net/manual/en/class.recursivecallbackfilteriterator.php expecting the boolean as result from callback
Comment #11
uzlov CreditAttribution: uzlov at Skilld commentedI mean, just
return $name[0] == '.' ? FALSE : !in_array($name, $exclude, TRUE);
Comment #12
andypostGreat hint, thanks!
Moreover I think better to add special
TwigTemplateIterator
to encapsulate itComment #13
andypostSeparate iterator approach, still needs test
Comment #14
Gábor HojtsyAdding the needs tests tag. I think adding a node_modules dir with a twig file using something that would become an error in one of the passing modules would be good to prove this :)
Hm, which part of this will exclude node_modules or bower_components for that matter?
If TemplateDirIterator already does so, why do we need this one specifically?
Comment #15
andypost($current->isDir() && !in_array($name, $exclude, TRUE))
this part, the same we do in coreComment #16
andypost++ to add tests
Comment #17
Gábor Hojtsy@andypost: are you still planning to work on this? Thanks!
Comment #18
andypostYes, I do, but not this week(
Comment #19
Gábor HojtsyWould love to add this with tests, but I don't have capacity to add tests unfortunately.
Comment #20
vacho CreditAttribution: vacho at Skilld commentedI only rebase by now the patch #13
Comment #21
vacho CreditAttribution: vacho at Skilld commentedComment #22
Gábor HojtsySent the 3.x for a retest on 8.9 for now as the patch applies locally.
Comment #23
Gábor HojtsyAttempted a simple test for this.
Comment #24
Gábor HojtsyActually the interdiff was supposed to be a test only patch that should fail.
Comment #27
Gábor HojtsyThat proved that the twig file lookup and the phpstan lookup both exclude node_modules now. Let's get this in! Thanks a lot!