EntityNG is now pretty much settled on naming:
- "field": the value objects found in an entity (e.g the value of node 12's 'body')
--> Drupal\Core\Entity\Field\Type\Field (+ Fieldnterface) and Drupal\Core\Entity\Field\FieldItemBase (+ FieldItemInterface)
- "field definition": the metadata about those fields (e.g the definition of 'body' on article nodes)
--> #1950632: Create a FieldDefinitionInterface and use it for formatters and widgets is about to introduce Drupal\Core\Entity\Field\FieldDefinitionInterface

Just like "fields" are both configurable ("Field API") fields and base fields, the FieldDefinitionInterface will unify the metadata formats for those two kinds of fields.

OTOH, what the "historic" Field API called "field" is the "field definition". In D8:
- the 'field_entity' entity type, and Drupal\field\Plugin\Core\Entity\Field (+ FieldInterface)
- the 'field_instance' entity type, and Drupal\field\Plugin\Core\Entity\FieldInstance (+ FieldInstanceInterface)
- the two being ConfigEntities, field.field.[field_name] / field.instance.[entity_type].[bundle].[field_name] in CMI

So we have:
- Field (entity ng): the values
implements FieldInterface
- Field (field api): the definition of a "configurable field"
implements FieldDefintionInterface
- FieldInstance (field api): the definition of a "configurable field" attached to a specific entity bundle
implements FieldDefintionInterface

(note, we have two more "Field" classes in core:
- the one for "static access to services provided by field.module", similar to the \Drupal class - could be renamed FieldAPI, maybe
- one in Views)

Remember that "terminology" slide in the "old & new Field API" prez in Portland ? :-)

It bugs me a lot, but can't really see another way than to rename Field API config entity classes...
But to what ? with which entity type names and CMI prefixes ?
There is a definite risk of those various things getting excruciatingly long...

Here's a proposal that tries to limit name length where I feel there is less ambiguity:
- ConfigFieldDefinition / ConfigFieldDefinitionInterface
entity type: config_field
CMI: field.field.[field_name] (unchanged)
- ConfigFieldInstanceDefinition / ConfigFieldInstanceDefinition
entity type: config_field_instance
CMI: field.instance.[entity_type].[bundle].[field_name] (unchanged)

Renaming classes / interfaces should be relatively painless.
Renaming entity types shouldn't be too hard thoretically, except we're in the middle of #1953410: [Meta] Remove field_create_*(), field_update_*() and field_delete_*() in favor of just using the ConfigEntity API ;-/
Renaming CMI files - not sure, possibly not that hard...

Thoughts ?

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

yched’s picture

Issue tags: +entity API, +Field API

Tags

swentel’s picture

+1 on that. We need to rename 'field_entity' anyway. I like 'config_field' and 'config_field_instance'. Only field would make hooks like 'entity_reference_field_update' also very confusing imo, see #2013939: Remove hook_field_[CRUD]_() in favor of hook_ENTITY_TYPE_[CRUD]() calls also.

We're not far yet with removing those crud functions, actually none got in yet, and a simple search replace from 'field_entity' to 'config_field' in those branches is not that hard, so the sooner we do this the better.

I don't see any change in the CMI files in the proposal, I'd keep them like they are anywa.

yched’s picture

Yes, I don't think this would affect the content of the CMI files or the properties of our ConfigEntities

effulgentsia’s picture

I think the names in the issue summary are an improvement. However, currently all of our core entity types are named the same as the module that defines them, or starts with $module_. The two exceptions are views.module defining an entity type view, and system.module defining an entity type menu.

Do we plan on renaming field.module to config_field.module?

Also, do we want to keep the "field" vs. "instance" terminology? Since per the issue summary, "field" is a data object, what Field API calls "instance" is the closer analog to a "field definition", since whenever you have a field data object, you know the specific entity it's part of, and therefore, its entity type and bundle. Meanwhile, what Field API calls a "field" is more like a "abstract field definition".

Given above, how about:
field_entity -> field_definition_abstract
field_instance -> field_definition_concrete

?

effulgentsia’s picture

Instead of "abstract" and "concrete", I also considered "unbound" and "bound", since SCOTCH has been temporarily using those words to refer to a similar distinction between layout-independent Displays and layout-dependent Displays. However, I don't know if those temporary words in SCOTCH will become permanent, and regardless, whether we want to use them here. But throwing it out there in case any of you like them.

yched’s picture

- "field.module -> config_field.module"
Strictly speaking, yes, that would be a more accurate name for the module now.
The mere thought of this at this point in the cycle makes me want to curl up in a corner, though :-/.

- bound / unbound field - field / abstract field
There might be interesting metaphors here, but this makes me consider crawling to the next corner.

I've been considering for quite some time that a "field" is necessarily a field "of something". No such thing as a field in the void. So:
- what we call $instance is actually a "field (definition)" - as in "a field of an entity"
- what we call $field is essentially the definition of a "data bucket" - a storage location.
But I wasn't able to really draw the consequences - and most of all, the rename is daunting... We have $.*instance.* variables all over the place...

yched’s picture

Other than 'config', we also would go with 'custom'.
CustomFieldDefinition, CustomFieldInstanceDefinition
custom_field, custom_field_instance

I've repeatedly found that "configurable field definition", "configurable field type"... are ambiguous as to what the 'configurable' adjective applies to - the field, the definition, the type...
Maybe that's a bit less true with "custom field definition", "custom field type" ? Hmm, writing it down, not really...
Well, if 'custom' is not a real gain on that specific point, I guess 'config' is better.

"base fields exist because code says so, config fields exist because config says so" has some clarity to it.

effulgentsia’s picture

Well, we don't want our Field API maintainer being driven into a corner :)

The names in the issue summary are fine, if that's what we want to limit our scope to. Similar considerations are what led us to keep a views.module and a view entity: no one wanted to rename views.module to view.module.

Can we spell out Configurable in the class names though? I.e., ConfigurableFieldDefinition? I'd like to avoid the potential confusion of ConfigField implying a field on a ConfigEntity.

yched’s picture

Well... "Configurable" + "Definition" starts to make really long names ... :-)
ConfigurableFieldInstanceDefinition
entity_create('configurable_field_instance_definition')
hook_[configurable_field_instance_definition]_update()

I was hoping to gain a couple letters with "Config" instead (the class names in #1969728: Implement Field API "field types" as TypedData Plugins currently build on that too).
But was kind of wondering when the objections would come :-) - and yes, the implications with ConfigEntity are a bit unfortunate.

What do you guys think of "Custom", then ?

swentel’s picture

Custom sounds weird to me too, as it somehow implies it's a field that I have defined in custom code. That's just my gut feeling, maybe also because my native language is not english, not sure. Aargh, naming is hard :)

effulgentsia’s picture

For the most part, I like "custom". Where it might break down, is with Features. For example, if a "Job posting" Feature ships with an "Applicant" field, does it make sense to consider that "custom"? If that's okay, then +1 to Custom.

effulgentsia’s picture

Another possibility is we accept the reality that field.module isn't getting renamed for D8, and so we train ourselves to think "anything scoped to field.module implicitly means dealing with configurable fields". However, we don't want an entity type named "field_definition", because then we have hooks like hook_field_definition_*(), and it's not clear whether that's scoped to field.module or to Drupal\Core\Entity\Field.

So how about:
entity types: 'field_field_definition' / 'field_instance_definition'

Class names in the Drupal\field namespace are already scoped by virtue of their namespace, so for those classes, how about:
FieldDefinition / FieldInstanceDefinition

If it were doable, I would also suggest renaming the YAML files to field.field_definition.*.yml and field.instance_definition.*.yml, but I don't think that's necessary if we want to keep the effort here minimal.

amateescu’s picture

Is it an option to challenge the (unfortunate, imo) decision of renaming properties to fields? The distinction between a property and a field was really clear in D7 and I'm sure everyone is already used it.

yched’s picture

I like #12, but 'field_field_definition' as entity_type name is really weird IMO.
If it's just about hook_field_definition_update() & friends, I'm not sure I see the issue ?

@amateescu: that's where I started from too, of course ;-) But I have less ideas what this would look like. I tend to think EntityNG has won here...

effulgentsia’s picture

The distinction between a property and a field was really clear in D7

That's cause in D7 properties and fields were really distinct. Properties couldn't have widgets, formatters, a translation UI, swappable storage, etc. We're still hoping that in D8, the only distinction between $node->title and $node->body is that the former is code defined and the latter is config file defined, so I don't think keeping / going back to the property/field terminology split would be helpful. Also, by being able to call $node->title a field, we can reserve "property" for the components of a field item (e.g., ->value, ->summary, etc.), which Field API currently calls "columns", but that's a word that only makes sense if you're using SQL storage.

If it's just about hook_field_definition_update() & friends, I'm not sure I see the issue ?

Well, in #1950632: Create a FieldDefinitionInterface and use it for formatters and widgets, we're gonna start having $field_definition appear in a lot of places. The value of that variable can sometimes be a Field API $field, sometimes (usually) a Field API $instance, and sometimes an entity base field definition. So, if you have a hook named hook_field_definition_create(), wouldn't it be weird for that to be only invoked for Field API $field entities?

'field_field_definition' as entity_type name is really weird IMO.

Agreed. But once you get used to it, it's unambiguous.

Anyway, this is still just brainstorming for now. Possibly the issue summary names are still the best?

yched’s picture

So, if you have a hook named hook_field_definition_create(), wouldn't it be weird for that to be only invoked for Field API $field entities?

Well, they will be the only ones to have the CRUD cycle of entities ?

effulgentsia’s picture

That's true. It just requires you to make that connection, rather than being immediately obvious. Maybe that's ok though.

yched’s picture

I'd be tempted to say it might be ok :-)

Hm - I was about to post that proposal in the summary but then I noticed:
If Field becomes FieldDefinition,
then FieldInterface becomes... FieldDefinitionInterface ? :-p

Sleeping on it...

effulgentsia’s picture

One more idea. If it makes you want to crawl into a corner, then forget it. But, if we're prefixing "config_" or suffixing "_definition", then we're gonna end up touching all the $field and $instance names anyway. Given that, what if we rename $instance to $field_definition and $field to $master_definition?

For entity types, this would be:
- 'field_entity' => 'field_master_definition'
- 'field_instance' => 'field_definition'

I think it's then fine for Drupal\field\FieldInstanceInterface to become Drupal\field\FieldDefinitionInterface. Because that interface then extends Drupal\Core\Entity\Field\FieldDefinitionInterface and ConfigEntityInterface. Which makes sense: it extends the former with configurability. Or in other words, it's a field.module "field definition", which is an extension of the entity system's "field definition".

And then hook_field_definition_OPERATION() hooks make sense, because they operate on *actual* field definitions (meaning fields "of something", per #6). They wouldn't get invoked on nonconfigurable field definitions, but per #16, that's ok, given that the operations are CRUD lifecycle operations, so don't apply to them.

fago’s picture

They wouldn't get invoked on nonconfigurable field definitions, but per #16, that's ok, given that the operations are CRUD lifecycle operations, so don't apply to them.

Agreed.

then FieldInterface becomes... FieldDefinitionInterface ? :-p

Yeah, I think we should be explicit about interfaces being about the entities - i.e. the configuration storage objects, and not.
We've got a similar distinction for actions now:
- ActionConfigEntityInterface
- ActionInterface

So what about:

Proposal 1
entity_types:
-field_definition
-field_instance

Interfaces:
-FieldDefinitionConfigEntityInterface
-FieldInstanceConfigEntityInterface

Proposal 2
I'm not fond of the "instance" term either, so here is proposal 2 (my favourite):

entity_types:
-field_base_definition
-field_definition

Interfaces:
-FieldBaseDefinitionConfigEntityInterface
-FieldDefinitionConfigEntityInterface

Reasoning:
-> Really, both a field definitions - so we should call them that way. Features 7.x-2.x started using the term "base_field" to differentiate between fields+field instances, what I think makes sense as the term "base" definition suggests me that the "field_definition" builds upon it.

fago’s picture

@FieldDefinitionConfigEntityInterface:

Yes, that's long - but usually people should be working with the "FieldDefinitionInterface" anyway - right? If you choose the other one then for reason, so it should be clear that it is about the entity specifically.

effulgentsia’s picture

Longer and explicit interface names are ok with me too.

Re 'field_base_definition' (#20) vs. 'field_master_definition' (#19), either works for me, but if we use "base", does that limit us from referring to $node->title as an "entity base field"? I guess as long as "field_base_definition" is read as "the base definition of a field" rather than "the definition of a base field", then we're ok, but it's a bit subtle.

If we find some other term for $node->title though (e.g., nonconfigurable field, though that's a mouthful), then it's a nonissue.

andypost’s picture

I'd prefer to have
FieldDefinitionEntityInterface
FieldInstanceEntityInterface

Renaming of instance to field would lead to errors and misunderstanding for developers

gdd’s picture

I don't have any great thoughts on the naming, however I wanted to point out that one very likely conclusion of #1776830: [META-1] Installation and uninstallation of configuration provided by a module that belongs to another module's API is going to be that the first part of any config's name will be forced to be the module that implements its functionality (which is not necessarily the module that provides it.) So having something like config_field.[whatever] without a corresponding config_field.module is likely to break stuff.

fago’s picture

I guess as long as "field_base_definition" is read as "the base definition of a field" rather than "the definition of a base field", then we're ok, but it's a bit subtle.

indeed.

Re 'field_base_definition' (#20) vs. 'field_master_definition' (#19), either works for me, ..

yep, for me as well.

effulgentsia’s picture

Thinking about it more, I'm not liking "master" as much as I did a couple days ago. I thought of that word based on Views having the concept of a "master display", which serves a very similar purpose: storing settings that are shared across all the displays. Except when I just looked at HEAD right now, I saw that in code, that's referred to as the "default display", and the term "Master" only appears in a couple places in the UI, and only if you go out of your way to configure Views to show it. So I'm now leaning a bit more to #20.2, despite the potential confusion of #22. One consideration is that maybe the potential confusion will be mitigated by "base" being such a generic word that always needs context anyway. A "base class" is different than a "base definition" is different than a "base field" is different than a "base plugin" is different than a "base type", etc.

Renaming of instance to field would lead to errors and misunderstanding for developers

Can you elaborate on this? Is it just because D7 developers are used to the field/instance terminology? My concern is that not doing it will lead to errors and misunderstanding, since elsewhere in D8 Entity API, "field" means "a field of something", per #6.

andypost’s picture

since elsewhere in D8 Entity API, "field" means "a field of something"

Yes, this is a most confusing part. I'd prefer to have properties on entities by renaming baseFieldDefinitions() to basePropertyDefinitions() and leave Field and its Instance as is

chx’s picture

Priority: Normal » Critical

renaming entities: absolutely, the current situation is untenable. I do not know how this is not a release blocker so marking it one.

renaming field module: d9. It's unfortunate but D9.

catch’s picture

Is this really more confusing that entity vs. node, config entity vs. config and entity, db_add_field() vs. field_create_field() or #states vs. state()?

chx’s picture

The name is the same (so entity vs node is not the same). db_add_field is close but that's not runtime so to speak. That's update only. This is everyday code and it is *extremely* confusing.

yched’s picture

I have to agree :-(.
The examples in #29 are "acceptable" because the duplicates each belong to separate / semi-separate APIs or layers.

This case here is about the same API naming two closely related but conceptually different things (the data and the metadata, or the container and the containee) with the same name ("field").

More precisely, it's two separate APIs (D7 field API and D8 entityNG API) that used the same term with different meanings, and then merged. But that historical background is irrelevant; when you look at the current resulting API, it *is* a highly confusing WTF :-(

I myself am always confused about which is which between FIeld "the definition" and Field "the value" when looking at the autocomplete suggestions presented by my IDE...

effulgentsia’s picture

As a specific example of #31, I have so far refrained from renaming $items to $field in #2021817: Make widgets / formatters work on EntityNG Field value objects., even though that would be the consistent thing to do from an EntityNG perspective. That patch does typehint them to FieldInterface though, as in Drupal\Core\Entity\Field\FieldInterface, not Drupal\field\FieldInterface. Only the use statement at the top of the file, and the legacy parameter name $items, disambiguates that though.

effulgentsia’s picture

Assuming it's too late / undesired to rename the "field" and "instance" portion, how about this:

Entity types:
field_entity -> field_config
field_instance -> field_instance_config

Classes:
Field -> FieldConfig
FieldInstance -> FieldInstanceConfig

Interfaces:
FieldInterface -> FieldConfigInterface
FieldInstanceInterface -> FieldInstanceConfigInterface

Parameter names:
$field -> $field_config
$instance -> $field_instance_config

CMI files:
no change, because "config" is already implied in something being a CMI file.

If people prefer, we can expand any/all of the "config" abbreviations above to "configuration". However, we're already using "config" as an established abbreviation for "configuration" all over the place.

The key concept in the above is that per #31 we need some word to qualify "field" to make it clear that it's referring to the metadata about the field and not to its content data. "definition" is the word we use for that elsewhere, but I think it's challenging to use here because of the actual definition being a composition of "field"-level and "instance"-level information. But I think "configuration" works as a noun that implies both that it's about the definition rather than the content (because the whole point of the config system is to separate configuration from content) and that it's configurable.

yched’s picture

#33 works for me. Achieves the needed disambiguation, and is not too disruptive given the late time in the cycle. +1.

Thanks @eff !

alexpott’s picture

#33 looks good to me too

pcambra’s picture

Status: Active » Needs review
FileSize
329.98 KB

As it seems there's consensus in #33, here's an initial patch, there were a bunch of field_config and instance_config referring to config objects, I've rename those to field_properties and instance_properties, maybe we should find a better naming?

Triggering test bot to see see what fails. Do we want to get this in in pieces like other massive renames?

@TODO: Haven't replaced these yet:

Parameter names:
$field -> $field_config
$instance -> $field_instance_config

Status: Needs review » Needs work

The last submitted patch, 2019601-rename_field-36.patch, failed testing.

yched’s picture

Hm. Yeah. $field_config / $instance_config variables were already used in the (few) places that work with raw config objects (or their as-array content) - as opposed to the ConfigEntities.

Then maybe use the "_raw" suffix for those ?
$field_config_raw / $(field_)instance_config_raw ?

pcambra’s picture

Status: Needs work » Needs review
FileSize
373.36 KB
85.74 KB

Renamed the field_config and instance_config to "_raw" and replaced $field and $instance parameters.
Also fixed a rename problem in FieldInfo.php which I guess was causing the fails.

Status: Needs review » Needs work

The last submitted patch, 2019601-rename_field-39.patch, failed testing.

pcambra’s picture

Status: Needs work » Needs review
FileSize
373.2 KB

Rebased to latest HEAD

Status: Needs review » Needs work
Issue tags: -entity API, -Field API

The last submitted patch, 2019601-rename_field-40.patch, failed testing.

pcambra’s picture

Status: Needs work » Needs review

#41: 2019601-rename_field-40.patch queued for re-testing.

Status: Needs review » Needs work
Issue tags: +entity API, +Field API

The last submitted patch, 2019601-rename_field-40.patch, failed testing.

yched’s picture

Thanks @pcambra !

However, the code conventions state that class properties should use camelCase rather than underscores.
I don't know for sure where we stand on this regarding helper members used in tests, but for example
FieldInstanceConfig::$field_config / $this->field_config should definitely be fieldConfig.

Also, I guess, strictly speaking, FieldInstanceConfig::getField() should be renamed getFieldConfig() now :-/

pcambra’s picture

Status: Needs work » Needs review
FileSize
374.54 KB
11.17 KB

@yched: We've got some extra cleanup then, I've seen most of the classes with their properties using underscores.

Fixed comments in #45, let's wake up the testbot, not sure why is not installing. I didn't fix the tests classes.

yched’s picture

"I've seen most of the classes with their properties using underscores"

True, actually there is a semi-written convention that ConfigEntity properties that end up saved as keys in CMI files do use underscores. This field_config property in FieldInstanceConfig is a pure, non-CMI property though, so it should really use camelcase.

Status: Needs review » Needs work

The last submitted patch, 2019601-rename_field-46.patch, failed testing.

pcambra’s picture

Status: Needs work » Needs review
FileSize
403.76 KB
41.41 KB

Missed a field rename and some getFieldConfig().

Status: Needs review » Needs work

The last submitted patch, 2019601-rename_field-49.patch, failed testing.

effulgentsia’s picture

Thanks, pcambra.

not sure why is not installing

One cause of bot installation failure is if you have an error while starting out logged out, and then logging in via the /user/login page, since that's basically what bot does to log in. User entities have fields, so errors in Field API can cause such an error.

pcambra’s picture

Status: Needs work » Needs review
FileSize
22.53 KB
415.15 KB

This should look better! :)

Status: Needs review » Needs work

The last submitted patch, 2019601-rename_field-52.patch, failed testing.

pcambra’s picture

Status: Needs work » Needs review
FileSize
428.81 KB
28.21 KB

I reverted the replacement related with createFieldWithInstance as field and instance is created as literal there:

    $field = 'field' . $suffix;
    $instance = 'instance' . $suffix;

Not sure if we want to replace those too by field_config and field_instance_config.

I think the option tests will fail, I think I've tracked it to a cache flush issue, the test checks changing the allowed values to a different set, tested it via UI and the feature is working there.

Status: Needs review » Needs work

The last submitted patch, 2019601-rename_field-54.patch, failed testing.

pcambra’s picture

Status: Needs work » Needs review
FileSize
416.58 KB

Status: Needs review » Needs work

The last submitted patch, 2019601-rename_field-56.patch, failed testing.

pcambra’s picture

Status: Needs work » Needs review
FileSize
417.2 KB
999 bytes

Fixing the exception on the entity translation test.

Status: Needs review » Needs work

The last submitted patch, 2019601-rename_field-58.patch, failed testing.

swentel’s picture

Status: Needs work » Needs review
FileSize
2.15 KB
417.21 KB

Should be green.

effulgentsia’s picture

This is looking great so far.

But I notice some inconsistencies in tests. For example, some tests change $this->field and $this->instance to $this->field_config and $this->field_instance_config. But some tests do not. And this test changes one but not the other:

+++ b/core/modules/block/custom_block/lib/Drupal/custom_block/Tests/CustomBlockFieldTest.php
@@ -64,13 +64,13 @@ public function testBlockFields() {
-    $this->field = entity_create('field_entity', array(
+    $this->field = entity_create('field_config', array(
...
-    $this->instance = entity_create('field_instance', array(
+    $this->field_instance_config = entity_create('field_instance_config', array(

Within tests, I don't know if we need the disambiguation or not, so I could go for changing none or all, but let's be consistent. Any opinions on whether it should be none or all?

pcambra’s picture

FileSize
56.54 KB
447.43 KB

Added more field->field_config and instance->field_instance_config conversions in tests as for #61 but there are still a few related with createFieldWithInstance (see #54)

Status: Needs review » Needs work

The last submitted patch, 2019601-rename_field-62.patch, failed testing.

pcambra’s picture

Status: Needs work » Needs review
FileSize
447.44 KB

I had forgotten to include fixes from #60, same as #62 with those included.

Status: Needs review » Needs work

The last submitted patch, 2019601-rename_field-64.patch, failed testing.

pcambra’s picture

Status: Needs work » Needs review
FileSize
447.44 KB

Status: Needs review » Needs work

The last submitted patch, 2019601-rename_field-66.patch, failed testing.

pcambra’s picture

Status: Needs work » Needs review
FileSize
454.42 KB
11.13 KB

Seems that there were a couple of field entity creation additions in the middle, updated those too.

Status: Needs review » Needs work

The last submitted patch, 2019601-rename_field-68.patch, failed testing.

pcambra’s picture

Status: Needs work » Needs review
FileSize
660 bytes
454.5 KB

Fixed a missing variable

effulgentsia’s picture

Status: Needs review » Needs work
Issue tags: +Needs reroll

Needs a reroll for #2021817: Make widgets / formatters work on EntityNG Field value objects. and possibly other HEAD changes.

+++ b/core/modules/block/custom_block/lib/Drupal/custom_block/Tests/CustomBlockFieldTest.php
@@ -25,16 +25,16 @@ class CustomBlockFieldTest extends CustomBlockTestBase {
   protected $field;
...
-  protected $instance;
+  protected $field_instance_config;

Inconsistent to leave one but change the other. However...

+++ b/core/modules/block/custom_block/lib/Drupal/custom_block/Tests/CustomBlockFieldTest.php
@@ -64,28 +64,28 @@ public function testBlockFields() {
-    $this->field = entity_create('field_entity', array(
+    $field_config = entity_create('field_config', array(
...
-    $this->instance = entity_create('field_instance', array(
+    $field_instance_config = entity_create('field_instance_config', array(

Does this mean we can remove the protected $field; protected $field_instance_config; declarations from the earlier hunk entirely? If not, why change from member to local variables?

yched’s picture

[edit: you can spare reading the rant - according to #73, #1869574: Support single valued Entity fields is about to rename EntityNG's Field to FieldItemList, so basically the Field ConfigEntity can stay Field...]

OK, I really really feel bad for doing this after 70+ comments and when we have a 450k patch ready :-(, but...

Now that we're off the rush of last June, and after spending more time around the new shape of widgets, formatters, field types..., I really feel we're making the wrong decision by settling on "the word 'field' is for the value objects, what we called 'fields' in D7 are now called 'field definitions'".

  • In practice, it makes more convoluted sentences in code comments, documentation...
  • It disrupts years of habits and online/offline drupal litterature
  • More importantly, we cannot promote disruption by saying it's a good move, because it's not inline with actual use in common language, for example when discussing architecture when building a site:
    - "this is a text field"
    - "this would require a new field"
    - "should we share this field or create a new one ?"
    - ...
    When looking at "entities" (general, non-drupal sense), one key aspect in defining and naming concepts is answering the question "Is this the same 'thingie', are those different 'thingies' ?".
    - If 'field' means "the value of a given 'field definition' in a given entity" (see, even that sentence is horrid :-)), then "Is this the same field ?" just doesn't mean anything, it will never be "the same field". When talking about the definitions, then "It's the same field definition" feels ambiguous, conveys equality but not really identity.
    - If "field" means "the definition", as it did so far, then "is this the same field ?" has a meaning - and it's a sentence we've all been using in our day-to-day life as drupal devs or site builder.
  • It's not inline with our own UI: "add new field, manage fields".
    I don't see us replacing "field" with "field definition" in there ? Do we really want to have another example of "we say 'node' in code but 'content' in the UI" ?
    (would be actually be worse, the same word meaning one thing in the UI and something related-but-critically-different in code)

So, I'm now pretty firmly convinced (I'm just sorry that it took me so long):
that "field" should stay about the definitions: the 'body' field, the 'field_tags' field,
and that the class of the $entity->body object should be named something else than Field.

Fortunately, it's IMO much easier in that direction:
$entity->body is an object that extends ItemList, and that holds a list of FieldItem objects.
So: FieldItems ? FieldItemList ?

Most of the variables that refer to such objects in our current code are currently named $items anyway, because that's how CCK named them since D4.7... and I'd say it's not a coincidence that #2021817: Make widgets / formatters work on EntityNG Field value objects. just hinted a bunch of them as 'Field' but didn't feel like renaming them from $items to $field - it just feels too weird IMO.

I'll post a detailed proposal for the rename map in a next post, this one is too long already...

amateescu’s picture

Just noting that #1869574: Support single valued Entity fields already renames \Drupal\Core\Entity\Field\Field to \Drupal\Core\Entity\Field\FieldItemList, so it's very likely that we can just close this issue..

yched’s picture

LOL of all LOLs.

- "blablablabla blablabla bleblablablabla blabla blableblabla blablablablablablablablabla blabla blabla bleblablablablabla blablablabla blablablabla blablabla blablablabla blablabla bleblablablabla blabla blableblabla blablablablablablablablabla blabla blabla bleblablablablabla blablablabla blablablabla blablabla blablabla blabla blableblabla blablablablablablablablabla blabla blabla bleblablablablabla blablablabla blablablabla blablabla"
- "sure, dude, chill out, this has already been agreed over here..."

amateescu’s picture

For the record, as I already said it in IRC, I just found out about this after reading your post from #72, which reminded me about the single valued fields issue, decided to check it out and saw that @fago reached the same conclusion by himself :)

effulgentsia’s picture

Status: Needs work » Closed (won't fix)

With 2 Field API maintainers on board with that, and with an Entity API maintainer reaching the same conclusion in the other issue, I feel good about closing this as "won't fix". If someone disagrees though, please reopen.

yched’s picture

So, I'm not exactly sure we're fully done with renaming yet :-)
Reopening for now. At least it's probably not critical anymore.

If "field" stays "a field definition", then:
- it seems it would make sense to rename the "generic" Drupal\Core\Entity\Field\FieldDefinitionInterface used by widgets and formatters, to simply FieldInterface ?
- if so, then this clashes with Drupal\field\Plugin\Core\Entity\FieldInterface (the interface for the "configurable field" ConfigEntity) - and one extends the other :-).

If we don't need to burden with having "definition" in the class names, then we don't have to do the weird contorsions we went through above to avoid too long names, and then I think the following would be much clearer conceptually:

- Abstract "field definition" interface added by #1950632: Create a FieldDefinitionInterface and use it for formatters and widgets:
Drupal\Core\Entity\Field\FieldDefinitionInterface --> FieldInterface

- The getFieldDefinition() method in FieldItemList / FieldItem --> getField() (returns a FieldInterface)

- The "field definition" class for base fields being added at #2047229: Make use of classes for entity field and data definitions:
Drupal\Core\Entity\Field\FieldDefinition (current name in the patch) --> Field ? BaseField ?

- Configurable field ConfigEntities:
Drupal\field\Plugin\Core\Entity\Field --> ConfigurableField - entity_type 'field' - CMI: field.field.*
Drupal\field\Plugin\Core\Entity\FieldInterface --> ConfigurableFieldInterface
Drupal\field\Plugin\Core\Entity\FieldInstance --> ConfigurableFieldInstance - entity_type 'field_instance' - CMI: field.instance.*
Drupal\field\Plugin\Core\Entity\FieldInstanceInterface --> ConfigurableFieldInstanceInterface
but no need to rename existing $field / $instance variables IMO, which was probably 80% of the previous patch size.

yched’s picture

Priority: Critical » Major
Status: Closed (won't fix) » Active

Forgot to say so, but #77 or "won't fix" is up for discussion, of course.

plach’s picture

I'd say we definitely want to do this, it looks like an obvious DX win.

if so, then this clashes with Drupal\field\Plugin\Core\Entity\FieldInterface

What about renaming that to Drupal\Core\Entity\Field\ConfigurableFieldInterface? After all nothing prevents other modules from providing configurable fields. So we would have a common interface and no clash at all.

Drupal\Core\Entity\Field\FieldDefinition (current name in the patch) --> Field ? BaseField ?

I'd be ok with just Field, looks cleaner and does not introduce cognitive burden at first sight ;)

To sum up:

Drupal\Core\Entity\Field\FieldDefinitionInterface --> FieldInterface
Drupal\Core\Entity\Field\FieldDefinition --> Field
Drupal\field\Plugin\Core\Entity\FieldInterface --> Drupal\Core\Entity\Field\ConfigurableFieldInterface
Drupal\field\Plugin\Core\Entity\Field --> ConfigurableField
Drupal\field\Plugin\Core\Entity\FieldInstance --> ConfigurableFieldInstance
Drupal\field\Plugin\Core\Entity\FieldInstanceInterface --> ConfigurableFieldInstanceInterface

Not sure whether it makes sense to generalize also the concept of instance and move ConfigurableFieldInstanceInterface to the core namespace (as FieldInstanceInterface?).

Edit: +1 for method renames as proposed in #77, not sure about the related stuff in the Typed Data API. It would be cool to get rid of the property terminology once and for all.

plach’s picture

What about renaming that to Drupal\Core\Entity\Field\ConfigurableFieldInterface? After all nothing prevents other modules from providing configurable fields. So we would have a common interface and no clash at all.

Scratch this, I didn't realize it was so Field API specific :)

yched’s picture

Priority: Major » Critical
Status: Active » Closed (won't fix)
Issue tags: -Needs reroll

OK, this is probably better off in a fresh issue...
I opened #2051923: Rename \Drupal\Core\Entity\Field\Field, related subclasses and interfaces to *FieldItemList for the new proposal.

Huge apologies to @pcambra for what happened here :-(.
Thanks a lot for stepping up. We might be able to reuse some of the work in #2051923: Rename \Drupal\Core\Entity\Field\Field, related subclasses and interfaces to *FieldItemList...

yched’s picture

Issue summary: View changes

typo

effulgentsia’s picture