Problem

It appears that in cases where there are nested for loops in Twig, the inner loop does not increment properly.

So for something like:


{% for foo in foos %}
  {% for bar in foo.bars %}
    {{ bar }}
  {% endfor %}
{% endfor %}

The variable bar will not increment properly.

Workarounds

Use the index to reference the inner values


{% for i,foo in foos %}
  {% for j,bar in foo.bars %}
    {{ foo.bars[j] }}
  {% endfor %}
{% endfor %}

or


{% for i in foos|keys %}
  {% for j in foos[i].bars|keys %}
    {{ foos[i].bars[j] }}
  {% endfor %}
{% endfor %}

Original summary by joelpittet

Note: Make sure that twig caching is off to see this issue

Problem:

{# Columns #}
  {% if colgroups %}
    {% for colgroup in colgroups %}
      {% if colgroup.cols %}
        <colgroup{{ colgroup.attributes }}>

          {% for col in colgroup.cols %}
            <col{{ col.attributes }}/>  {# PROBLEM HERE #}
          {% endfor %}

        </colgroup>
      {% else %}
        <colgroup{{ colgroup.attributes }}/>
      {% endif %}
    {% endfor %}
  {% endif %}

The first pass through the nested for .. in loop the attributes will print but in the subsequent loops they are marked as printed() and will not print out.

Temporary Fixes Found:

{% for i,col in colgroup.cols %}
  <col{{ colgroup.cols[i].attributes }}/>
{% endfor %}

or

{% for col in colgroup.cols %}
  {% set col_attributes = col.attributes %}
  <col{{ col_attributes }}/>
{% endfor %}

or less ideal but for testing:
Comment out if (!$value->printed()) from Attribute.php

Back Reference:
http://drupal.org/node/1778968#comment-6820122

Examples Tables:
colgroup2 shows the issue the best
https://github.com/joelpittet/twig-tables

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

c4rl’s picture

Title: Twig Attributes Issue - for in loop context attributes being marked as printed » Nested Twig 'for' loops behaving strange

I also noticed this issue when working on theme_table.

Will update summary to list workarounds.

c4rl’s picture

Project: » Drupal core
Issue summary: View changes

cleanup spacing

joelpittet’s picture

Project: Drupal core »

Steveoliver saw this too. Do we know anybody at SensioLabs maybe they could have a peek?

jwilson3’s picture

This should really be reported to Twig's issue queue.

Fabianx’s picture

Are we able to reproduce this with pure twig? It might be TwigReference object related.

bzitzow’s picture

Assigned: Unassigned » bzitzow
bzitzow’s picture

Status: Active » Needs review

Tested nested twig loop in drupal implementation (and outside drupal implementation) and found it is working as expected. Maybe it is resolved? Maybe the issue could be more explicitly illustrated?

It appears that in cases where there are nested for loops in Twig, the inner loop does not increment properly.

{% for foo in foos %}
  {% for bar in foo.bars %}
    {{ bar }}
  {% endfor %}
{% endfor %}

The variable bar will not increment properly.

Using the following data, I discovered the nested loop is incrementing correctly.

Data:

$view['foos']   = array(
            'foo.1' => array(
                'bars' => array('bar1.1', 'bar1.2', 'bar1.3'),
            ),
            'foo.2' => array(
                'bars' => array('bar2.1', 'bar2.2', 'bar2.3'),
            ),
        );

Twig:

<PRE>
{% for foo in foos %}
    {% for bar in foo.bars %}
{{ bar }}
    {% endfor %}
{% endfor %}
</PRE>

Result:

bar1.1
bar1.2
bar1.3
bar2.1
bar2.2
bar2.3
bzitzow’s picture

Assigned: bzitzow » Unassigned
c4rl’s picture

Status: Needs review » Active

I noticed this in another theme function, I believe as part of #1898454: system.module - Convert PHPTemplate templates to Twig (though against core and not the sandbox). I'll need some time to dig into this, but I think it is still happening.

bzitzow’s picture

I am still interested in looking into it if you can duplicate it.

c4rl’s picture

This is still happening both in core and in the front-end branch. Some variables seem to work, some do not, even outside of a nested context. Will attempt to dig into this more later.

joelpittet’s picture

Project: » Drupal core
Version: » 8.x-dev
Component: Twig templates conversion (front-end branch) » theme system
Issue tags: +Twig
joelpittet’s picture

Seems not to be an issue with twig but something in drupal is messing up the context...
I took down a stock version of Twig and the context issue doesn't seem to be there...

If you want to try it out (grabbed twig with composer): http://twig.sensiolabs.org/doc/intro.html#installation

PHP:

// index.php
require_once 'vendor/autoload.php';
$loader = new Twig_Loader_Filesystem('templates');

// Match some of Drupal's Environment
$twig_options = array(
	'cache' => FALSE,
	'autoescape' => FALSE,
	'strict_variables' => FALSE,
	'debug' => TRUE,
	'auto_reload' => TRUE,
);
$twig = new Twig_Environment($loader, $twig_options);

// Build up array similar to the one in update_report() conversion.
$project_types = array(
  0 => array(
    'label' => 'Drupal core',
    'projects' => array(
      'drupal' => array(
        'attributes' => 'class="ok"',
        'title' => '<a href="http://example.com/project/drupal">Drupal</a>',
      ),
    ),
  ),
  1 => array(
    'label' => 'Themes',
    'projects' => array(
      'update test base theme' => array(
        'attributes' => 'class="error"',
        'title' => '<a href="http://example.com/project/update_test_basetheme">Update test base theme</a>',
      ),
      'update test subtheme' => array(
        'attributes' => 'class="ok"',
        'title' => '<a href="http://example.com/project/update_test_subtheme">Update test subtheme</a>',
      ),
    ),
  ),
);

echo $twig->render('loop.twig', array('project_types' => $project_types));

Twig:

{# templates/loop.twig #}
{% for project_type in project_types %}
  <h3>{{ project_type.label }}</h3>
  <table class="update">
    <tbody>
      {% for project in project_type.projects %}
        <tr{{ project.attributes }}>
          <td>
            <div class="project">
              {{ project.title }} {{ project.existing_version }}
            </div>
          </td>
        </tr>
      {% endfor %}
    </tbody>
  </table>
{% endfor %}
joelpittet’s picture

FileSize
19.76 KB
32.24 KB

So as a second test, I've used the update_report twig file and dumped the array and twig file from #12 into a preprocessor and this is what I got.

Original twig file from #12:
drupal-upate.jpg

File from update_report.html.twig (with the project_title hack back to project.title)
drupal-update.jpg

function template_preprocess_update_report(&$variables) {

  // Build up array similar to the one in update_report() conversion.
  $variables['project_types'] = array(
    0 => array(
      'label' => 'Drupal core',
      'projects' => array(
        'drupal' => array(
          'attributes' => 'class="ok"',
          'title' => '<a href="http://example.com/project/drupal">Drupal</a>',
        ),
      ),
    ),
    1 => array(
      'label' => 'Themes',
      'projects' => array(
        'update test base theme' => array(
          'attributes' => 'class="error"',
          'title' => '<a href="http://example.com/project/update_test_basetheme">Update test base theme</a>',
        ),
        'update test subtheme' => array(
          'attributes' => 'class="ok"',
          'title' => '<a href="http://example.com/project/update_test_subtheme">Update test subtheme</a>',
        ),
      ),
    ),
  );
}

joelpittet’s picture

FileSize
16.31 KB
21.25 KB

I'm going to assume this is related for now but the loop indexing is all wonky.

Stock Twig:
twig-index.jpg

Drupal Twig:
drupal-twig-index.jpg

{% for project_type in project_types %}
  <h3>{{ project_type.label }}</h3>{{ loop.index }}
  <table class="update">
    <tbody>
      {% for project in project_type.projects %}
        <tr{{ project.attributes }}>
          <td>
            {{ loop.parent.loop.index }}.{{ loop.index }}
            <div class="project">
              {{ project.title }} {{ project.existing_version }}
            </div>
          </td>
        </tr>
      {% endfor %}
    </tbody>
  </table>
{% endfor %}
joelpittet’s picture

Here is an example that doesn't pass variables in, just straight twig same results as #14. (I just took a preprocess and gutted the contents)

{% set project_types = [
  {
    'label': 'foo',
    'projects': [
      {
        'attributes': 'class="ok"',
        'title': '<a href="http://example.com/project/drupal">Drupal</a>'
      }
    ]
  },
  {
    'label': 'bar',
    'projects': [
      {
        'attributes': 'class="error"',
        'title': '<a href="http://example.com/project/update_test_basetheme">Update test base theme</a>'
      },
      {
        'attributes': 'class="ok"',
        'title': '<a href="http://example.com/project/update_test_subtheme">Update test subtheme</a>'
      }
    ]
  }
]
%}

{% for project_type in project_types %}
  <h3>{{ loop.index }}) {{ project_type.label }} </h3>
  <table class="update">
    <tbody>
      {% for project in project_type.projects %}
        <tr{{ project.attributes }}>
          <td>

            <div class="project">
              {{ loop.parent.loop.index }}.{{ loop.index }}) {{ project.title }} {{ project.existing_version }}
            </div>
          </td>
        </tr>
      {% endfor %}
    </tbody>
  </table>
{% endfor %}
joelpittet’s picture

I did a bit more research to try and track down where this is coming from and have some hunches to what's up here.

TwigReference seems to be main culprit. And ArrayObject's exchangeArray may have something to do with the index number issue, though I could not get the nested variables to show up correctly. One thought is to replace ArrayObject with implements \ArrayAccess, \IteratorAggregate, \Countable and do all that stuff ourselves. Thoughts being it may not be doing things the way you want due to notes I found here
/vendor/symfony/validator/Symfony/Component/Validator/Tests/Constraints/CollectionValidatorCustomArrayObjectTest.php

Fabianx’s picture

Status: Active » Needs review

Here is a patch:

diff --git a/core/lib/Drupal/Core/Template/TwigTemplate.php b/core/lib/Drupal/Core/Template/TwigTemplate.php
index 7d7eeca..d092bc6 100644
--- a/core/lib/Drupal/Core/Template/TwigTemplate.php
+++ b/core/lib/Drupal/Core/Template/TwigTemplate.php
@@ -51,6 +51,12 @@
       return NULL;
     }
 
+    // Turn off references for looped items
+    // @todo: hide, show is not supported for them for now.
+    if (isset($context['loop'])) {
+      return $context[$item];
+    }
+
     // The first test also finds empty / null render arrays
     if (!$context[$item] || isset($this->is_no_reference[$item])) {
       return $context[$item];

Please test.

This solution is:

a) it keeps being fast (isset() is very very fast)
b) hide, show is not supported in loops, but that should be acceptable trade-off for now.

So this patch should have zero overhead to what is in core currently.

joelpittet’s picture

Worked great with the simple test! I am going to test it later tonight on theme_table and theme_update conversions to see how it fairs for those two suckers, then add it in here.

joelpittet’s picture

Here is a patch file from #17
Thanks @Fabianx.

joelpittet’s picture

Status: Needs review » Needs work

The last submitted patch, 1867090-twigreference-nested-for-loop.patch, failed testing.

joelpittet’s picture

Status: Needs work » Needs review
FileSize
710 bytes

I must not be awake yet, sorry. Here's the patch.

joelpittet’s picture

With some testing I found that the key is rarely 'loop' in the wild, so this patch didn't hold water. I am thinking we turn off the reference caching for now. Patch attached.

joelpittet’s picture

#22 but with '_seq' instead of loop from @Fabianx on IRC.

joelpittet’s picture

Going too fast... argh.

joelpittet’s picture

Ok, so I tested this with update_report #1898466-40: update.module - Convert theme_ functions to Twig as well as the simple test in #15

This fixes the indexing issues for the looped items as well as the items in each iteration using the first TwigReference/Array in the loop.

Attached is just document updates to hopefully make it more clear what this is doing.

Thank you very, very much @Fabianx!

joelpittet’s picture

Issue tags: +Quick fix, +Twig engine

tagging.

joelpittet’s picture

Doc tweak again.

Fabianx’s picture

Status: Needs review » Reviewed & tested by the community

Looks good to me.

I can verify that _seq key is used only internally by Twig to store information about the loop it is in. It is also unset() as soon as loop context is left.

Please lets get this in as this unblocks some conversions.

=> RTBC

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Comment needs some clean-up...

+    // Return item instead of it's reference inside a loop.
+    // @todo 'hide' and 'show' are not supported inside a loop for now.
+    // Nested Twig 'for' loops behaving strange
+    // @see http://drupal.org/node/1867090
+    // This should be a non-issue as soon as this lands:
+    // @see http://drupal.org/node/1922304

Reduced to the following and fixed an it's/its:

+    // Return item instead of its reference inside a loop.
+    // @todo 'hide' and 'show' are not supported inside a loop for now.
+    // This should be a non-issue as soon as this lands:
+    // @see http://drupal.org/node/1922304

...aaaaand committed and pushed to 8.x. Thanks! :)

joelpittet’s picture

Thank you, Thank you @webchick and @Fabianx!

Automatically closed -- issue fixed for 2 weeks with no activity.

Anonymous’s picture

Issue summary: View changes

Revised summary