Closed (fixed)
Project:
Upgrade Status
Version:
8.x-2.x-dev
Component:
Code
Priority:
Normal
Category:
Feature request
Assigned:
Unassigned
Reporter:
Created:
15 May 2019 at 14:20 UTC
Updated:
27 Jan 2020 at 14:24 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
gábor hojtsyCrosslinking with https://github.com/mglaman/drupal-check/issues/29 :)
Compiling and catching deprecations there is an interesting concept. Is that how twig deprecations on the template side are implemented? (That could also work for contrib extensions to Twig that are used on the site so would be super useful).
Comment #3
gábor hojtsyFor clarity, I don't think drupal-check would include a template compilation tool given its focus on static analyses only, but I'd leave it to Matt to declare explicitly if not so :)
Comment #4
joelpittetSome examples would be helpful to narrow the scope of this and provide better targeted solutions(event if it's not PHPstan). What changes are you referring to?
Comment #5
lauriii@joelpittet I've been working on a Google Doc to summarize the changes affecting theme and module developers. Hopefully, this gives an idea of what kind of changes we might want to be able to analyze!
Comment #6
lauriiiI did some further research on this. I found asm89/twig-lint library which essentially compiles Twig templates and collects error messages that has been triggered while compiling them. Unfortunately, the library doesn't collect deprecation warnings out of the box, but we could fork the project and add that functionality. I did a very rough test to show that deprecation errors are triggered in the process of compiling templates, which can be found here.
Besides deprecations, we will have to integrate this to the Drupal so that Twig extensions added using Drupal APIs get taken into the process of compiling Twig templates.
Comment #7
wim leersI can speak to some extent about the riskiness of that dependency. Interesting coincidence: Drupal core already depends on a lib by the same author: https://github.com/asm89/stack-cors 🤓 In #2853201: [upstream] CORS breaks form submission unless allowed origins includes site's own host, Drupal core moved from version 1.0 to version 1.1 of that lib. The author was pretty responsive back then: https://github.com/asm89/stack-cors/issues/44.
I am cautiously optimistic that the author would be willing to work with us to also collect deprecation warnings :)
Comment #8
gábor hojtsy@lauri: if we are to run this while compilation happens (which sounds fine), do you envision we would add this to core (make this a core issue) or we clear the twig cache and recompile all the templates somehow in Upgrade Status when we check modules/themes? Do we have a reliable way in Drupal to collect all the twig templates on a site?
Comment #9
lauriiiWe can trigger the compilation of a template manually for code analysis purposes. The library has it's own custom compilation process which is currently triggered by a CLI command. The compiled Twig templates are only saved in runtime memory, they are not being stored anywhere for later consumption.
Collecting Twig templates can be done by looking for files with .twig on the name from all enabled modules, themes, and installation profiles. This comes with the downside that we would also check stale templates.
Comment #10
lauriiiI don't think there is any reason for us to move this to core. We can keep this here, but it might make sense to have this as part of mglaman/drupal-check as well.
Comment #11
gábor hojtsyI started discussing this with the linting library maintainers at https://github.com/asm89/twig-lint/issues/43
Comment #12
krzysztof domański1. Twig updated from 1.x to 2.x in Drupal 9.
2. Documentation for Twig:
Comment #13
gábor hojtsy@Krzysztof Domański: to use that we would need to be able to run all templates using all of their possible code paths (that may result in deprecation notices). I don't know how would that even be possible?
Comment #14
lauriiiI found out Twig has built in tooling for checking deprecations in templates so I ended up using that.
I'm having some issues with the batch processing so I couldn't run tests locally. Let's see if this passes.
Comment #15
lauriiiAdding missing test module.
Comment #16
lauriiiComment #17
lauriiiComment #19
gábor hojtsyLooks great, landed!