Problem/Motivation

I might have mentioned this in issues before, but I think we don't have a dedicated issue with an actual idea and description of what's going on I think.

it's quite common to do something like this in twig:

{% if node.some_field.value %}

{{ if node.some_field.value|strip_tags }}

{{ if node.some_field.1.value|strip_tags }}

What twig does internally for that is complicated and slow, especially if there is no value, we should find ways to improve that and also make the return value more reliable.

Accessing some_field is pretty ok. it does an isset($node->some_field), which calls __isset() and then hasField(). Would be nice if we could optimize that to check if the object is a content entity and some_field is a field, then just return that.

The fun starts with .value on the field item list object, when empty, this happens:

1. It's an array access object, so it checks that first, but that's FALSE because there is no value. So it moves on.
2. Then it checks the object property but that's also FALSE since there is no item.
3. Then it moves on to methods, and since we access .value, it tries getValue() on the field item list, which actually exists. But that returns an array of all values, there are no values, so it's an empty array. That works fine for the if condition, as it evaluates to FALSE, but it returns php warnings because an array is passed to strip_tags/trim/...

I think 3 is happening since we stopped creating the delta 0 field item object for empty fields. Before, I think isset() returned TRUE and it returned the empty string or so.

Proposed resolution

What we could do is check if $object is FieldItemListInterface (or FieldItemInterface). If the the property is a number for the list, it's the delta, so return that. Done. If it's a string, just return it. __get() etc. will take care of not resulting in any php notices/warnings and any kind of check we could do would just slow it down.

Not sure if we can do that, can we override getAttribute()?

Remaining tasks

User interface changes

API changes

Data model changes

Comments

Berdir created an issue. See original summary.

Cottser’s picture

Issue tags: +Twig

If we override getAttribute we are out of sync with the Twig C extension. Our policy so far has been that overriding getAttribute is not something we would do.

Berdir’s picture

Thanks for the feedback. I didn't know that getAttribute() was part of the twig extension. That makes sense.

Any other ideas on how to achieve this? IMHO, the fact that node.empty_field.value returns an empty array is a bug, we should at least fix that.

Can we influence the generated php code? I guess we'd have to know in advance if something is a content entity, which we currently can't.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

joelpittet’s picture

Twig c extension is no longer recommended and not ported to php 7 as the gains wherent deemed significant.

All printed things we can type check but we aren't wrapping magic around conditionals or other constructs in twig.

Maybe this is worth a revisit, thanks for posting a reminder on twitter @berdir

dawehner’s picture

In general I'm wondering whether we could actually do some magic on twig compile time. If we somehow now types on compile time we could have special behaviour for entities.