Check if there's sufficient test coverage for base fields with simultaneous code-based and config-based overrides

From #2283977-152: Create a new ConfigEntity type for storing bundle-specific customizations of base fields:

My opinion is we should change the name from core.base_field_override.*.yml to core.field_override.*.yml, so that contrib could use the exact same entity type for overriding non-base fields.

Implementation wise, it's overriding base fields only now - so having config that implies something that is not happening seems confusing to me.

So, actually the implemenation could be easily improved to handle all fields by using getStorageFieldDefinition() instead of getBaseFieldDefinitions() to fetch the storage field. Given that, I'd actually agree with effulgentsia that the config (+ entity type) should be core.field_override.*.yml then. I don't think the possible confusion around overriding field instances with config is not an issue - technically, you could do it, but there is no point in overriding a config field with config, so that's why we don't ;-)

So depending on what we decide here, we might want to this rename or not. But anyway, I think we shoud have this clarified before beta.

@topic:

We've got one usage of code based bundle field overrides left in core after [#9058387]: comment module. It could be moved to config by generating the config overridde upon bundle creation. That's a bit more work to do and maybe not so easy to follow as now, but it should work fine.

Personally, I think it makes sense to keep code-based bundle field overrides for the following reasons:

• Consistency. We've got code based base fields, so disallowing code based bundle fields seems arbitrary to me.
• Code based fields bundle fields offer more control, e.g. to remove display configurability. Thus, they are the only mechanism right now for making use of entity storage by bundle without having to expose configurability. We do not have code based FieldStorageDefinition and FieldDefinition (for bundles) yet, but that would be simple to add and could be even done by contrib (interfaces!).
• Easier handling / maintenance of code based overrides as thanks to the shallow-override of the code based bundle field you do not have to duplicate the common base field properties in your code. Not a biggie, but still.

When keeping it I think we should:

• Rename BaseFieldOverride to FieldOverrideConfig and core.field_override.*.yml + remove its coupling to base fields.
• Fix base field override DX by adding a *new* BaseFieldOverride class for code based overrides, which internally can defer to the real base field object and thus has an improved memory footprint.
• Provide code based FieldStorageDefinition and FieldDefinition and test coverage (just an addition, follow-up) ?

I gave this more thought on whether this is really necessary or it makes sense to work on simplifying things more.

Code based fields bundle fields offer more control, e.g. to remove display configurability. Thus, they are the only mechanism right now for making use of entity storage by bundle without having to expose configurability. We do not have code based FieldStorageDefinition and FieldDefinition (for bundles) yet, but that would be simple to add and could be even done by contrib (interfaces!).

We could achieve that as well by making sure BaseFieldOverrides and FieldInstanceConfigs support varying display configurability setting as well, what means bringing at least default-display settings back to the field instance config (API only). That way, modules could create field instances and or overrides with full display configurability flexibility.
I think for base field overrides that makes much sense anyway, for field instances it depends on whether we have code fields as alternative.

Thinking more about possible use-cases for code fields, I came up with another one:

• Implementing an entity type with remote storage and many different bundles. When e.g. you expose web service resources as entities (as done by https://www.drupal.org/project/fluxservice), you often have many similar ressources with some varying fields for which you'd provide different bundles. So while the shared fields would be base fields and thus defined in code, you'd have to add storage+instance config for the bundle fields in world without coded-bundle fields. It wouldb e quite weird if the system forces you to add config for bundle fields, while it doesn't for base fields. Also, while I see no real technical reason I must say it feels more natural / suiting to have code fields for that use-case.

First off, sorry for being a bit late to the party...

I (re-)read this issue as well as (re-)reading most of #2283977: Create a new ConfigEntity type for storing bundle-specific customizations of base fields as well the committed patch.

As I didn't get involved with that issue I'm aware that I can't complain about how it turned out, but since this issue is very much based on that, there's no around taking it into account.

Some background:
I am very adament about our current ability to provide code-based (base) fields via ContentEntityInterface::baseFieldDefinitions(). Additionally, I very much like the pattern of field.module not participating at all in the architectural decisions of the concept but instead simply adding its storage fields in hook_entity_field_storage_info().*

I think that pattern is something that we should utilize for more of the Entity API and I think it can perfectly address the problem of this issue and #2283977: Create a new ConfigEntity type for storing bundle-specific customizations of base fields, as well.

The problem with #2283977: Create a new ConfigEntity type for storing bundle-specific customizations of base fields:
#2283977: Create a new ConfigEntity type for storing bundle-specific customizations of base fields raised one issue with our current base-field/bundle-field architecture but instead of fixing at its root, it implemented a workaround, in my opinion.

The root problem, which was also discussed there, is that bundle fields are (and after #2283977: Create a new ConfigEntity type for storing bundle-specific customizations of base fields mostly were) used for two different things:

• Overriding properties of base fields: This was used e.g. by nodes to override the title label
• Adding additional fields: This is what field.module did to add its FieldInstanceConfig. To be able to add fields for a specific bundle and being able to store the data somewhere, the concept of storage fields had to be invented.

We already had a pattern for the different steps that would happen during the collection of base/storage/bundle fields. That is why I do not understand why we did not implement that in the usual way. The precedent we already had for content entities was:

• Base/Storage fields are provided by a method on the entity type baseFieldDefinitions() and then extended by a hook (hook_entity_(base_)field_(storage_)info())
• Bundle fields are provided by a method on the entity type bundleFieldDefinitions() and then extended by a hook (hook_entity_bundle_field_info())

So if we realize we need to introduce a third logical step the most logical thing to do, in my opinion, would have been: 1. Introduce a new method for entity types to provide bundle-specific overrides of base fields. For example Node's bundleFieldDefinitions() would be moved to this method. 2. Introduce a new hook so that modules can extend these overrides. content_translation.module could then use that hook to override translatability of base fields on a bundle level.
Some additional notes:

• If we were to pursue that route it might be worthwhile to get rid of the additional alter hooks as that seems sort of redundant
• This would greatly improve the DX of overriding base fields on a per-bundle level (what Node does) vs. how it was before #2283977: Create a new ConfigEntity type for storing bundle-specific customizations of base fields without having to introduce any workarounds (note: the fact #2283977: Create a new ConfigEntity type for storing bundle-specific customizations of base fields introduced workarounds is not my opinion but a fact. See the changes to NodeTypeForm that issue introduced if you do not believe me)
• This would in fact allow to get rid of the concept of storage fields alltogether. We could simply iterate of all bundles to fetch all possible bundle fields when building the schema which is exactly what storage fields are meant to encapsule.

As this approach is the most consistent with our current architecture I think it would be the best for DX (in addition to the DX gain mentioned above). The code that #2283977: Create a new ConfigEntity type for storing bundle-specific customizations of base fields introduced to EntityManager is just alien to that class when looking at how any other types of fields are collected.

* Yes, I know that base field != storage field. Proceed. :-)

Can be active

The tests/fixes part of this needs to happen after beta if the 'remove support for' doesn't - so only half the issue is beta deadline I guess.

Thank you for creating this issue to improve Drupal.

We are working to decide if this task is still relevant to a currently supported version of Drupal. There hasn't been any discussion here for over 8 years which suggests that this has either been implemented or is no longer relevant. Your thoughts on this will allow a decision to be made.

Since we need more information to move forward with this issue, the status is now Postponed (maintainer needs more info). If we don't receive additional information to help with the issue, it may be closed after three months.

Thanks!

Wanted to bump this one more time.

Don't feel comfortable closing so will put back into active.