Determine how to deal with field types used in base fields in core entities

Another problem that we have with option 1), besides it being far from ideal, is actually the schema() method that is currently the only difference between the two "email" plugins.

Right now, we're pretending that "base" fields don't have (or need) a schema, but they do, it's just burried in the hook_schema() implementation that defines the base/data/revision tables of the entity type. I know @plach wanted us to get to a state where we could generate entity tables dynamically by introspecting the base field definitions, and I think we should still aim for this in D8.

You elude to a problem with 2. in the first part of the OP: user.module should be using the email field for user accounts, so with 2. user.module would have to depend on email.module, which would make email.module required.

I don't really see the downside of 3. TBH. My personal answers to your problems/questions about that approach would be:

- Where is the line ? is "text" basic ? what about "file", "image", "entity_ref" ?

I think the question should be the other way around: What makes something a module? Possible answers: The need for a schema, shipped configuration, hooks need to implemented, ...
None of these apply here so put it in Drupal\Core! :-)

- This means Drupal\Core has classes that extend ConfigurableFieldItemBase, which is currently provided by field.module

That is very unfortunate but really all those classes should not be in field.module but in \Drupal\Core\Entity\Field anyway. This would be very much in-line with the effort to remove field.module in favor of entity.module / \Drupal\Core\Entity

Then we also need to move along the widgets and formatters plugins provided by the former separate modules ?
How do we organize that ? The annotation discovery forces "all field types in a folder, all formatters in a folder, all widgets in a folder". Given that some of them use base classes, that's a lot of classes blended together...

This is a valid concern, but not something that should hold this up, in my opinion. Plugins have the concept of a 'provider', so we basically have 3 options here:
a) Decide that the entity system (\Drupal\Core\Entity) is the provider of all those fields, which would cause the problem you described.
b) Decide that the (currently non-existing) 'email component' (\Drupal\Core\Email) is the provider for the email field, and likewise for all other fields, which would do away with the crowded directories but also lose the nice namespacing of everything under \Drupal\Core\Entity
c) Somehow make the Plugin system more flexible, so that it finds stuff in \Drupal\Core\Entity\Email, etc.

@yched: Sorry, I thought "to elude" meant something else. That didn't make any sense, the way I said it.

I'm not super comfortable with plugins provided in lib/drupal/core, and their "moduley" pieces handled in entity.module.

Yes, I agree that would be an unnatural split. I wasn't aware of all the "moduley" pieces you mention in #4. That certainly convinces me that 2. is the way to go. I'm not sure we can really live with

"Are we OK with having contact.module, node.module, user.module... require text.module, number.module, email.module, maybe entity_ref.module... ? Means those modules are de-facto required on all sites"

, however. Required modules really suck... Again, user.module is still required and most likely will be in Drupal 8. So that makes at least text.module, email.module and date.module required as well. entity_reference.module would then be required by node.module "only".

Path 1) looks good to me:)

a). For path 3) and path 2), the unified ItemClass needs work for both, i.e., base fields and configurable fields. And one should depends on the other. This could confuse developers. For example, if I only need to provide a base field field_type in contrib module, this field_type plugin needs to implement ConfigFieldItemInterface too! This doesn't make sense to me:(

Despite this bundle of base field and configurable field, they do have different things that should be highlighted in their own ItemClasses or plugin annotation. For example schema(), and property 'is_configurable'.

EmailItem here looks like a happy coincident:) For other base field Item at this moment, e.g., 'IntegerItem', 'LanguageItem', 'StringItem', they don't implement 'ConfigFieldItemInterface' as EmailItem too. So I guess no need to enforce these base field items to implement 'ConfigFieldItemInterface' too, just for some unifying stuff?

b). I guess path 1) doesn't break the original purpose of 'FieldDefinitionInterface'. This interface is introduced to unfiy base fields and configurable fields in some sense, so they could be treated as the same in certain circumstances, e.g., field formatter needs to work for both, so they should be treated the same.

In the field item class context here, looks like they don't have to be the same. And they work perfectly with each own item class here. Eveything is clear to developer, that one field_type is not configurable, so it should only be used for base fields, and it shouldn't implement 'ConfigFieldItemInterface', which is only for configurable fields.

For the formatter and possible widget stuff, there would be some 'repeat' field_types perhaps. Like, date formatter and email formatter, they could need to declare they support both date(node.created) and configurable_date(complex date field) field_type. But for most field_types, they are clearly un-connected. For StringItem as base field(node.title) now, text formatter perhaps needs to declare it supports field_type 'Text' and 'String' too.

In a word, merits of path 1) surpass its shorcomings imho:)

Right, right, I took it the wrong way:) Contrib module doesn't have to implement ConfigFieldItemInterface for base field, but that's what Drupal core does, if we use option 2).

So does this make base field object a little bigger than it should be? For instance, StringItem should be relatively simple, but since it's using TextItem provided by Text module, String base field object could contain methods, like 'instanceSettingsForm', 'settingsForm', etc, which probably never be used for base fields. Does this make php-memory cost much when it comes to base field object?

Also, this probably adds difficulty for base field's FieldDefinition to extract field definition at #2047229: Make use of classes for entity field and data definitions. Configurable field's Field Definition has already highly depended on the field_type plugin/Field Item Class. For instances, configurable field's definition method -- FieldDefinitionInterface::getFieldSetting will count 'settings', and 'instance settings' in plugin annotation. What should base field's fieldSettings count? Only 'settings', but not 'instance settings'?

Ok, it's just a bit weird to create base field Item from configurable field item. As base field item are originally supposed to be a simple piece of data:)

I must say that I don't think option 1) is so bad. I'd like having simpe field types provided by Core, and then modules adding more complex ones.

We bloat our "field type plugins" registry with two separate field types, one for base fields, the other for configurable fields.

As to that, couldn't we just use a alter hook to swap out the plugin class and make it configurable? Then it remains the same field type, it just became configurable by install e-mail module.

What I really want to avoid is making more modules required. We want to get rid of them, not add more. required API's shouldn't be in modules. Every required module slows down the whole test suite, because every single web test needs to install them, and every module installation is a costly container rebuild, plugin caches get reset and so on.

Other than that, not sure. Not convinced that replacing core plugins is a good idea, that seems like it could trigger unexpected behavior.

If it's just about moving a schema method into the Core classes and use the @FieldType annotation instead, that would be fine with me. Looking at email.module and ConfigurableEmailItem, the constraint stuff should be in the Core class anyway.

@yched
When I said, 'a simple piece of data', what I meant was , StringItem atm belongs to namespace 'Drupal\Core\Entity\Plugin\DataType'. So it's kind of like a 'data'...

But yeah, thanks for your so much clarification:)

Yes, but it needs to be moved - see #2023563: Convert entity field types to the field_type plugin.

@fago, yeah, thanks for clarifying that:)

couldn't we just use a alter hook to swap out the plugin class and make it configurable?

This sounds a good plan. It will decouple dependency between configurable field ItemClass and base field ItemClass, and yield the unified field_type for similar fields.
While this solves @Berdir's 'required modules' issue greatly, it remains all these ItemClasses existing in code, kind of duplicating code as @yched unhappy with. Looks like this is a compromised solution for option1 and option2 + option3:)

- This is a weird split of code between Core/Entity and entity.module, but well...

Yes, I think this split is already weird: Please see #2031717: Make entity module not required. If it would go away, the stuff would even have to go the system module.

I think we should try to keep really essential base fields out of modules and just add the more complex ones with module(s). Having all in modules feels wrong as it renders the core component useless without modules.

Put their "moduley" bits (hooks, config settings, update hooks... routes ?) in entity.module

How much of that exists. What would be candidates for moving to the core component?

Regarding email LegacyConfigFieldItem, see #2087381-4: Decouple EmailItem field class from LegacyFieldItem

@yched so is there any meta with roadmap to fix that? what we should do first and next?

I tried to merge e.g. the email class in #2023563: Convert entity field types to the field_type plugin but we need the FieldDefinition class that is being added in #1994140: Unify entity field access and Field API access.

Tagging.

The two issues got in.

The base field issue already came up with a pattern for now, that is non-configurable are altered to configurable with hook_field_info_alter(). I think we decided to re-visit that after #2050835: Move Widget, Formatter, and FieldType plugin types to the Core\Field system, when we can have configurable fields + widget/formatters in Drupal\Core.

bla

Setting back to active.

If we decide that we want to move the email field completely into core, or the text field types, which are already in a required module, then we could try that now.

Or we can say we're ok with the current state and just close it.