Field template suggestions are colliding

No. We shouldn't remove this, as stated in my comment: #1962846-16: Use field instance name for header of comment list, drop comment-wrapper template.

I'm pasting the conversation that @andypost and I had in IRC. I'm postponing this issue on the now parent issue. I'm also bumping this to major because this affects DX and FX. Theme suggestions for fields is a pretty big deal.

[03:41 PM]  andypost:	 markcarver, I filed new patch https://drupal.org/node/2229355 I stull sure that mixing field type and field anme on one level is wrong
[03:41 PM]  Druplicon:	 https://drupal.org/node/2229355 => field templates are confusing #2229355: Field template suggestions are colliding => 2 comments, 1 IRC mention
[03:41 PM]  Druplicon:	 andypost: 2 hours 4 min ago <markcarver> tell andypost re: https://drupal.org/node/2035055: this issue is currently fixes the preprocess theme suggestion fix in the new hook: hook_theme_prepare. This issue is part of the meta theme architecture in an attempt to remove/rename preprocess. We may have to just keep hook_preprocess, but the latest patch doesn't reflect that. Like I said, the theme system is, unfortunately, borked when trying to d
[03:41 PM]  markcarver:	 andypost: it's not wrong, it's by design
[03:41 PM]  andypost:	 markcarver, is hook_theme prepare works now
[03:42 PM]  andypost:	 markcarver,  I kmpw it's by design be new field is a part of entity type so no more fields that could be shared betwen bundles
[03:43 PM]  andypost:	 *know
[03:43 PM]  andypost:	 dard, betwen entity types
[03:43 PM]  andypost:	 so field template and suggestions should be fixed
[03:44 PM]  andypost:	 markcarver, you cant render a field without entity
[03:45 PM]  markcarver:	 andypost: here's the thing... you're trying to solve a 1% use case
[03:46 PM]  markcarver:	 it is _extremely rare_ if a theme overrides/uses suggestion more than 1 level deep
[03:46 PM]  markcarver:	 maybe 2
[03:46 PM]  markcarver:	 'field__' . $element['#field_type'] . '__' . $element['#field_name'] . '__' .  $element['#entity_type'] . '__' . $element['#bundle'];
[03:46 PM]  markcarver:	 that will never be used
[03:47 PM]  markcarver:	 and if a site really _needs_ that level of granularity it can add it itself
[03:47 PM]  markcarver:	 via hook_theme_suggestions
[03:47 PM]  andypost:	 markcarver, for me the most used case is "make templates for field_description of some entity types
[03:48 PM]  markcarver:	 andypost: does the markup actually change or are classes being added to change the appearance?
[03:48 PM]  andypost:	 so for me "field_{{type}" is the common for custom fields, vase styling of floats, taxonomy and image
[03:48 PM]  markcarver:	 9/10 we don't actually ever end up changing the markup
[03:49 PM]  markcarver:	 it's for (or should be) injecting classes
[03:49 PM]  andypost:	 and sub-templates/styles of this fields withing some entity type
[03:49 PM]  andypost:	 so field_type__field_name my 80%
[03:50 PM]  andypost:	 markcarver, but comment or video field you should use field_type specific template first
[03:51 PM]  andypost:	 and it is a 90% of contrib fields
[03:51 PM]  markcarver:	 andypost: and are you creating unique fields each time, or re-using them?
[03:52 PM]  andypost:	 field type defines it's item properties so each property needs implementation in template
[03:52 PM]  markcarver:	 .... which is already first
[03:52 PM]  andypost:	 for d8 most common way is inheritance like currently text field inherited form string field
[03:52 PM]  markcarver:	 then field--field-name
[03:53 PM]  markcarver:	 or rather last...
[03:53 PM]  markcarver:	 it looks for the most specific suggestion first
[03:53 PM]  markcarver:	 so if I decide to provide a very specific template for this particular field-name
[03:53 PM]  markcarver:	 it will be called
[03:54 PM]  markcarver:	 otherwise if I have a generic field-type, it'll be called
[03:54 PM]  markcarver:	 field-type-name is almost redundant
[03:54 PM]  andypost:	 so hook_theme_prepare would be applied to field__TYPE and field_NAME so how inside of each prepare to detect variant, is this one a for type or name
[03:55 PM]  markcarver:	 andypost: thing of hook_theme_prepare === hook_preprocess, just fixed
[03:55 PM]  markcarver:	 s/thing/think
[03:55 PM]  andypost:	 yep
[03:56 PM]  andypost:	 field_TYPE defines properties
[03:56 PM]  markcarver:	 in fact, it should probably be renamed/postponed and the original issue should be moved to 8.x again
[03:56 PM]  markcarver:	 andypost: "properties" is a mute point
[03:56 PM]  markcarver:	 it's a template suggestion, plain and simple
[03:57 PM]  markcarver:	 if you provide a very specific field-name template, it's assumed that it's that "type"
[03:57 PM]  markcarver:	 because all field-name's are
[03:57 PM]  andypost:	 markcarver, I mean https://api.drupal.org/api/drupal/8/search/propertyDefinitions
[03:58 PM]  markcarver:	 ie: you cannot have to field-name's that are different types correct?
[03:59 PM]  andypost:	 field named "field_media" could be different types (image, video, audio) so all of them need own preprocess and templates
[03:59 PM]  andypost:	 field name couldbe shared within entity type only
[04:00 PM]  markcarver:	 but once you create "field_media" it's type is set, yes?
[04:00 PM]  markcarver:	 ie: it won't change
[04:00 PM]  andypost:	 field node.field_weight could be different type then user.field_weight
[04:01 PM]  andypost:	 sure, each field name with entity is unique
[04:01 PM]  markcarver:	 andypost: please stick to one example, it is hard to follow
[04:02 PM]  andypost:	 node.field_media is type of image but user.field_media has no relation so only field_instance makes sense
[04:03 PM]  andypost:	 also we can have field that just created as definition but have no instance
[04:03 PM]  markcarver:	 andypost: nonetheless, it's "type" is required, regardless if it has unique entity instance settings correct?
[04:03 PM]  markcarver:	 s/required/required and inherited
[04:05 PM]  andypost:	 "required" is a validator for each instance, you will use setRequired() in base entity fields and as property on field instance
[04:05 PM]  markcarver:	 andypost: I'm not that familiar with 8.x field, so let me explain how I understand 7.x
[04:06 PM]  markcarver:	 One creates a field by setting a label name (also the machine name) and chooses a "type"
[04:06 PM]  markcarver:	 Once that field has been created, it can be added as an "existing field" to any other entity
[04:07 PM]  markcarver:	 that field has a set "type" though. an entity cannot arbitrarily "change" that type as it's initial creation already set it
[04:07 PM]  markcarver:	 has that changed?
[04:08 PM]  andypost:	 now there's no difference except cardinality for node base fields and fields, for example: node.title is a string field with cardinality 1 with string widget and formatter
[04:08 PM]  andypost:	 http://goo.gl/AvS9lt also each field defines own properties that avalable as tokens and directly in twid {{ item.0.value }}
[04:09 PM]  markcarver:	 but the underlying type does not change
[04:10 PM]  markcarver:	 so if I create two fields: field_media and field_meda_specific
[04:10 PM]  markcarver:	 they're both the type "media"
[04:10 PM]  markcarver:	 however
[04:10 PM]  markcarver:	 I decide that I need a specific template for field--media-specific
[04:11 PM]  markcarver:	 it's assumed that this is of the type "media"
[04:11 PM]  andypost:	 markcarver, take a look http://goo.gl/MGZXbI
[04:11 PM]  markcarver:	 so i "inherit" the template field--media (if one existed)
[04:11 PM]  andypost:	 sure!
[04:12 PM]  andypost:	 with suggestion field__media__field_media_specific
[04:12 PM]  markcarver:	 that's what I am saying field__media__ is redundant!
[04:12 PM]  markcarver:	 I know it's of the type "media"
[04:13 PM]  markcarver:	 if I didn't provide a specific suggestion/template field__field_media_specific
[04:13 PM]  markcarver:	 it would fall back to field__media
[04:13 PM]  markcarver:	 because that's it's "type"
[04:13 PM]  andypost:	 yes, but media field template should add some data to template via preprocess function
[04:14 PM]  markcarver:	 andypost: and that is what the hook_theme_prepare issue is trying to fix: inherited preprocess invocations
[04:14 PM]  markcarver:	 because the theme system is broken
[04:15 PM]  andypost:	 but both suggestions will fire for fields when their name would be the same as field type name
[04:15 PM]  markcarver:	 preprocess should inherit/invoke all available functions
[04:15 PM]  markcarver:	 only the one template suggestion should actually be called though
[04:16 PM]  andypost:	 for example
[04:16 PM]  andypost:	 markcarver, field named "changed" which field type is "changed" too http://drupalcode.org/project/drupal.git/blob/HEAD:/core/modules/node/lib/Drupal/node/Entity/Node.php#l406
[04:17 PM]  andypost:	 so which suggestion will be parent?
[04:17 PM]  markcarver:	 field__changed
[04:17 PM]  andypost:	 yes
[04:17 PM]  markcarver:	 field__field_changed
[04:17 PM]  andypost:	 but is this override for type of field name
[04:17 PM]  markcarver:	 field__node__changed
[04:17 PM]  andypost:	 no, field name is "changed" as you see
[04:18 PM]  markcarver:	 andypost: did we stop prefixing the field names?
[04:18 PM]  markcarver:	 this is why the field_ prefix existed
[04:18 PM]  andypost:	 markcarver, that's what I try to tell you
[04:18 PM]  andypost:	 field name already has collisions with types
[04:18 PM]  andypost:	 fo placing them on one level of susggestions is wrong
[04:19 PM]  markcarver:	 no, because field__changed is assumed the type because of field__changed__changed existed, it'd be used
[04:20 PM]  markcarver:	 i see
[04:20 PM]  markcarver:	 ok
[04:20 PM]  andypost:	 markcarver, field type "created" is duplicating within node and user too http://drupalcode.org/project/drupal.git/blob/HEAD:/core/modules/user/lib/Drupal/user/Entity/User.php#l512
[04:20 PM]  markcarver:	 andypost: well, $suggestions[] = 'field__' . $element['#field_type'] . '__' . $element['#field_name'];
[04:20 PM]  markcarver:	 that is correct
[04:20 PM]  markcarver:	 the entity changes below it aren't
[04:20 PM]  andypost: http://drupalcode.org/project/drupal.git/blob/HEAD:/core/modules/node/lib/Drupal/node/Entity/Node.php#l400
[04:21 PM]  andypost:	 both created and could need separate templates
[04:22 PM]  andypost:	 so field__created__node and field__created__user
[04:23 PM]  markcarver:	 the point of having $suggestions[] = 'field__' . $element['#entity_type'] . '__' . $element['#bundle']; is so that an entity can alter all fields for a specific entity type
[04:23 PM]  andypost:	 but parent template field__created could provide common date formatting
[04:23 PM]  markcarver:	 regardless of the type or name
[04:24 PM]  andypost:	 but this collides in case of media field type and media entity name
[04:26 PM]  andypost:	 markcarver, so I think core will provide a minimal viable set of suggestions that has no collisions out of box
[04:27 PM]  markcarver:	 andypost: I think this is what it should be http://pastebin.com/wxhVKVyY
[04:29 PM]  andypost:	 markcarver, hm, makes sense too
[04:29 PM]  andypost:	 seems we need yched or berdir here
[04:30 PM]  markcarver:	 andypost: this would allow core/contrib to add things via preprocess suggestions (because the preprocess _should_/_will be_ inherited)
[04:30 PM]  markcarver:	 but any actual template would take precedence
[04:30 PM]  markcarver:	 perhaps the entity first suggestions should come before the field type specific ones
[04:31 PM]  markcarver:	 that way if there's a specific field type, it is used instead of an arbitrary entity type
[04:32 PM]  markcarver:	 andypost: mind if I paste this chat in the issue
[04:33 PM]  andypost:	 markcarver, please post this to issue
[04:33 PM]  andypost:	 this really should wait for preprocess conversion
[04:33 PM]  markcarver:	 andypost: ok, I'll paste and postpone this on the hook_theme_prepare issue
[04:33 PM]  andypost:	 but the same time we need more opinions from themers what is a most used need
[04:34 PM]  markcarver:	 andypost: agreed
[04:34 PM]  markcarver:	 I personally feel this is too granular and usually have very little use cases to go this deep into suggestions
[04:34 PM]  andypost:	 Druplicon, tell mortendk any ideas about order of suggestions for field templates https://drupal.org/node/2229355
[04:34 PM]  Druplicon:	 https://drupal.org/node/2229355 => field templates are confusing #2229355: Field template suggestions are colliding => 4 comments, 3 IRC mentions
[04:34 PM]  Druplicon:	 andypost: I'll pass that on when mortendk is around.
[04:34 PM]  markcarver:	 one or two usually all that's needed
[04:35 PM]  markcarver:	 just to add some classes or alter the markup slightly

Sorry for all the noise. I'm still getting used to parent/child/related issues.

Why is this postponed on #939462: Specific preprocess functions for theme hook suggestions are not invoked?

Nothing here is actionable by a novice contributor yet, untagging.

Adding this related, older issue. Seems related to me, anyway.

We might not be able to change this after RC/beta, so tagging.

Task>>Find a proper way separate field name and its type.

I think we can add field__name- to identify that we want to use first param as field name
then old code will work like usually but we will get new feature

Please, see patch

https://www.drupal.org/files/issues/field-suggestions-2229355-10264875.p...

Task>>Find a proper way separate field name and its type.

I think we can add field__name- to identify that we want to use first param as field name

Please, see patch

https://www.drupal.org/files/issues/field-suggestions-2229355-10264875--...

Andy, patch with type

https://www.drupal.org/files/issues/field-suggestions-2229355-10265235.p...

+++ b/core/modules/system/system.module
@@ -309,7 +309,7 @@ function system_theme_suggestions_field(array $variables) {
   $suggestions[] = 'field__' . $element['#entity_type'] . '__' . $element['#bundle'];
   $suggestions[] = 'field__' . $element['#entity_type'] . '__' . $element['#field_name'];

Could these possibly collide also?

Trying to think through how this could be fixed if it didn't get in during RC.

Could we leave the existing suggestions in (with the collisions, deprecated), and add the new ones? That's messy but would at least mean this moves to 8.1.x instead of 9.x.

To have committers consider it for inclusion in RC, add a statement to the issue summary of why we need to make this change during RC, including what happens if we do not make the change and what any disruptions from it are. You can add a section <h3>Why this should be an RC target</h3> to the summary.

Also, it's not really feasible to decide whether an issue is safe to do during RC when there's no patch yet (because we can't evaluate the disruption), so untagging for now.

I think we need to decide whether this is fixable in RC, or 8.x at all, before there's any point in working on a patch for it. Re-tagging for now.

Based on #21, this either needs to be done in some messy BC way in a minor, or postponed until 9.x.

A few quick thoughts:

- Suggestions can be added easily, so more specific things can be targeted.

It is not necessary to remove the old suggestions to allow to prevent conflicts.

There will be confusion why there are two, so it would be nice to highlight deprecated suggestions in the twig output.

We might want to _always_ prefix suggestions and mark non-prefixed suggestions as deprecated.

Or we might want to use a new key: #deprecated_theme_suggestions that contain the same as theme suggestions to allow to differentiate.

+1 for the idea of just adding the new more specific suggestions.

Another idea would be to hide the deprecated theme suggestions from the theme debug list, but they would be still loaded normally if there is pre-existing template.

Discussed with @Cottser, @lauriii, @xjm, @joelpittet. We agreed that this is not a theme system issue so removing the Twig tag. Also removing the 'sprint' tag as this does not appear to be being sprinted on.

The discussion thought that we should be adding the new templates and deprecating the old template suggestions. We also thought that since we have a number of issues around template suggestions we should open an issue discussing how to make the system more robust.

Marking duplicate of #1367354: [PP-1] The list of theme hook suggestions for "field" is incorrectly ordered and has namespacing collisions.