Problem/Motivation
In D8, the @EntityType annotation has take over the role of D7's hook_entity_info().
However, hook_entity_info() has been retained as a way for modules to provide default values under certain conditions.
One example of this is Views UI:
/**
* Implements hook_entity_info().
*/
function views_ui_entity_info(&$entity_info) {
$entity_info['view']['controllers'] += array(
'list' => 'Drupal\views_ui\ViewListController',
'form' => array(
'edit' => 'Drupal\views_ui\ViewEditFormController',
'add' => 'Drupal\views_ui\ViewAddFormController',
'preview' => 'Drupal\views_ui\ViewPreviewFormController',
'clone' => 'Drupal\views_ui\ViewCloneFormController',
),
);
}
It enhances the definition of the 'view' entity type with its default values.
It is not an alter, as it shouldn't get in the way of any contrib or custom modules who want to alter this.
Proposed resolution
Rename hook_entity_info() to hook_entity_info_build().
Remaining tasks
User interface changes
N/A
API changes
The new-style hook_entity_info() will be renamed, but that was already an API change from D7 (takes $entity_info by reference)
Related Issues
#1981312: Move views_ui code out of the View entity type annotation
Comments
Comment #1
dawehnerJust for consistency this maybe should take the &$entity_info as parameter.
I guess on the long run we want to remove this extra translation parameters from EntityType to the info_build hook?
Comment #2
tim.plunkettYeah, that's happening in #1968970: Standardize module-provided entity info documentation and clean-up the @EntityType annotation
Comment #3
dawehnerGreat, so let's fix this small bit and RTBC it.
Comment #4
webchickWeird. I really don't understand this at all.
I was told the reason we introduced the horrifically convoluted annotations system in the first place was the DX benefit of keeping the declarations for things above the things they're defining. Killing hook_entity_info() was supposed to be part of this. Regardless of what it's ultimately called, if we keep the concept of hook_entity_info() around, isn't that working against the entire point of doing annotations in the first place? If I still have to look in at least two places anyway to figure out all of the "info," I'd much rather just kill annotations altogether (in favour of YAML or something) and thus one of the 6-odd ways there are of declaring things in D8.
The more logical (albeit, I'm hand-waving here) thing to do would be to create a ViewsUI interface or something, that has these defaults declared on it as annotations, like all the other plugins that have defaults defined, and then somehow pull that onto Views structure (I assumed that'd be through a hook_entity_info_alter(), and don't understand why doing it that way would prevent contrib from doing things too).
Comment #5
tim.plunkettIf you want to alter the Views UI, you have to have a module name that starts with w, x, y, or z, or have a module weight of 11, or implement hook_module_implements_alter() for it to work.
But more than that scenario, there is another common contrib use case that won't work in annotations or YAML: conditionals.
If you're okay with the gymnastics required for an alter, we can nix the hook.
I tried to do that, but sun demanded the hook be readded, and yched agreed.
Comment #6
tim.plunkettThis issue was opened under the assumption that we want to keep this hook. I wasn't attempting to rekindle a discussion about its very existence (especially since I was the one who removed it and "lost" the debate when it was re-added.
It was added as a follow-up in #1763974-203: Convert entity type info into plugins (the commit message implied it was temporary?!)
In the current entity.api.php docs, hook_entity_info() says:
And for hook_entity_info_alter(), it says:
Comment #7
valthebaldExistence of hook_entity_info() in D8 is very confusing, since it's meaning has significantly changed from D7; so I second proposal to rename hook_entity_info() for better DX
Comment #8
valthebaldTagging
Comment #9
dawehnerPlease don't add the "needs change notification" tag as long the change is not committed yet.
Comment #10
tim.plunkettReroll.
Comment #12
Les LimRe-rerolled.
Comment #14
tstoecklerIn the new world order shouldn't this be hook_entity_type_build()?
Comment #15
BerdirYep, hook_entity_type_build($entity_types) and hook_entity_type_alter(&$entity_types) I'd say.
Comment #16
BerdirHere's a patch for this.
Want to get #2165155: Change $entity_type to $entity_type_id and $entity_info to $entity_type/$entity_types in first and there's also a entity url related patch somewhere conflicting with this, but I wanted to get started.
Completely new patch, that now does the following:
- Rename hook_entity_info($entity_info) to hook_entity_type_build($entity_types)
- Rename hook_entity_info_alter() to hook_entity_type_alter($entity_types)
- Drop the unused (!) entity_get_info()
- Also drop the unfortunately still used entity_info_clear_cache(). Maybe that should be a separate issue (to check if some calls are really still needed) but it was close to entity_get_info() and I wanted to see if this works.
Comment #19
Les Lim16: core-1981858-16-rename_entity_info.patch queued for re-testing.
Comment #21
BerdirOk, reverted the cache stuff, unit tests are failing due to the drupal_static() call. Let's fix that first.
The test result looks like this because fatal errors in unit tests result in this :(
Manually grepped the test output for other fails, update.php was broken because the special handling for entity_info in UpdateModuleHandler didn't apply anymore. Fixed that.
Comment #23
BerdirOk, re-roll and fixed content_translation, used the wrong hook name in the alter.
Comment #24
BerdirUpdating title.
Comment #25
dawehnerWhile we are here, can we explicit typehint them with array but also keep the @var ?
Interesting we typehint here but not there.
Comment #26
BerdirYes, I started doing that but didn't complete.
Comment #27
BerdirWeird, I didn't mean to set this to RTBC, sorry :)
Comment #28
dawehnerThank you!
Comment #29
alexpottcore-1981858-26-rename_entity_info.patch no longer applies.
Comment #30
BerdirRe-roll, also noticed that I didn't rename $entity_info before for config and content translation.
Comment #32
Berdir30: core-1981858-30-rename_entity_info.patch queued for re-testing.
Comment #35
BerdirMissed a use of $entity_type as string there.
Comment #36
BerdirCreated https://drupal.org/node/2196275, this should be ready again :)
Comment #37
Les LimLooks good to me.
Comment #38
Berdir35: core-1981858-35-rename_entity_info.patch queued for re-testing.
Comment #39
alexpottcore-1981858-35-rename_entity_info.patch no longer applies.
Comment #40
longwaveRerolled.
Comment #41
BerdirRe-roll looks good, thanks!
Comment #42
alexpottCommitted c210e86 and pushed to 8.x. Thanks!
Made a little fix during commit...
Comment #43
tim.plunkettFollow-up: #2201879: Rename InfoHookDecorator to BuildHookDecorator
Comment #44
Berdir