Problem/Motivation
Right now, fields support any kind of values as parameter to setValue(), e.g. you can pass 'foo' to $node->title->setValue() and it will figure it for you. However, this has the following problems:
- setValue() is run-time critical and eats about half of time necessary for creating entity field objects from plain values. Moving all that logic out of it saves a lot of run-time checks and two method calls per field.
- Entity classes receive $values in the constructor but cannot rely on any shape - so the code dealing with it has to check for lots of possible checks (ouch). We cannot pass it on to alter hooks either, what we might want to do in another issue.
- Having $entity->values shaped unexpectedly makes code fragile and hard to follow.
Proposed resolution
- Move value massaging logic in magic setters, e.g. such that you can still do
$node->title = 'foo';, but not$node->title->setValue(). - Improve EntityStorageBase::create() to normalize values before passing them to the entity.
- Cleanup Entity::__construct() to rely on $values being in the right shape
Remaining tasks
Do it.
Measure impact.
User interface changes
-
API changes
setValue() calls on field item lists and field items would stop supporting not properly shaped values, e.g. the following stops working: $node->title->setValue('foo'). $node->title = 'foo'; would still work.
Comments
Comment #1
fagoComment #2
yched commentedThis is connected with the question of normalization and code flow of $values at "entity create" & "translation create" time, thus related to #2382675: hook_entity_create() affects the data of new translations of existing entities in unexpected and undocumented ways
Comment #3
yched commentedComment #17
smustgrave commentedThank 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!
Comment #18
smustgrave commentedComment #19
smustgrave commentedWanted to bump this 1 more time before closing.
Comment #21
smustgrave commentedNot comfortable closing but believe this has gotten better with typehints being added slowly.