Discovered in #1867518-156: Leverage entityDisplay to provide fast rendering for fields

BaseFieldDefinition::createFromFieldStorageDefinition() creates a BFD that misses default values of field-level settings. This causes notices in code that tries to access them down the line.

BFD::createFromFieldStorageDefinition($storage_definition) returns (shortened) :
static::create($storage_definition->getType())->setSettings($storage_definition->getSettings())
- BaseFieldDefinition::create() has code that explicitly populates the default values for both storage-level and field-level settings.
- but then ->setSettings() overwrites the settings with an array that only contains storage-level settings, instead of merging.

So root of the issue is that TypedData\DataDefinition::setSettings($settings) overwrites existing settings, and we have no API to just merge.

Comments

amateescu’s picture

yched’s picture

Discussed with @fago and @Berdir :
TypedData doesn't have a notion of "default settings", so it's natural that TypedData\DataDefinition::setSettings($settings) overwrites, instead of merges.

But fields do have this notion of default (field-level / storage-level) settings, and a FieldDefinition that doesn't have an entry for an "official" setting is invalid / broken. There is no unsetting a setting.

So - for fields, setSettings() should merge instead of overwrite ?

yched’s picture

Status: Active » Closed (duplicate)

Okay - so #2236903: setSettings() on field definitions can unintentionally clear default values changes the behavior directly on TypedData\DataDefinition::setSettings($settings)

OK, let's (re)-discuss over there then.