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.
Updated: Comment #3
Problem/Motivation
Let's discuss whether we should enable strict_variables in Twig. This defaults to FALSE whereas for example autoescape is enabled by default. So we're not straying from the defaults.
Pros of enabling strict_variables
- Easier to catch typos
Cons of enabling strict_variables
- Higher quantity and more verbose if statements may be required (Example from Bolt CMS docs):
Before:
{% if content.image != "" %} (do something with the image..) {% endif %}
After:
{% if content.image is defined and content.image != "" %} (do something with the image..) {% endif %}
Proposed resolution
TBD
Remaining tasks
Discuss
User interface changes
n/a
API changes
n/a
Original report by @Fabianx
Currently accessing a not defined var in strict mode gives a FATAL error message. We might want to have a warning instead and return "null".
This needs discussion.
To check for a var in twig use:
if key in array
or more general like isset()
if array.key is defined
or
if arrayis defined
Comment | File | Size | Author |
---|---|---|---|
#8 | 1806538-strict-todo-fix-8.patch | 705 bytes | joelpittet |
Comments
Comment #1
podarokdo we really need make FATAL error message not FATAL?
Are there any example for using templates with undefided variables?
Comment #2
joelpittetJust following up: currently we have
strict_variables => FALSE
. I'm a bit unsure of what advantages we would gain from turning that to TRUE;Throwing exceptions at themers for not knowing a variable doesn't exist sounds like a pain.
Uncaught exception 'Twig_Error_Runtime' with message 'Variable "variable" does not exist
Though if we are going to decide to go the explicit route, we'd better decide before beta because turning this on after people start using it is asking for a headache.
Comment #3
star-szrUpdated the issue summary after doing a bit of research. I'm not convinced we should enable this.
Comment #4
star-szrFixing typo and slight reword in the issue summary.
Comment #5
forestmars CreditAttribution: forestmars commentedI'm still not seeing what the advantages would be of enabling strict_variables…?
Comment #6
star-szrThanks @forestmars, I'm still not feeling this either. Going to close this, we have plenty of other stuff to work on :)
Comment #7
star-szrActually. We need to remove the @todo lines before we can close this:
Comment #8
joelpittetHere it be.
Comment #9
star-szrComment #10
star-szrComment #11
webchickYeah that before/after code snippet is pretty ick, so I definitely support leaving this at the default of FALSE.
On IRC Cottser mentioned that since FALSE is already the default, we don't need that line right below the comments anymore, so I removed that on commit and pushed to 8.x. Thanks!
Comment #13
mikeker CreditAttribution: mikeker commentedCreated a followup issue: #2445705: Core themes should not throw exceptions when strict_variables is TRUE.