We have a lot of hooks in system.api.php that are not related to the System module. This file should only contain hooks that are only invoked in the System module. Other core hooks should be moved to other files, such as core.api.php, file.api.php, etc.
This issue has been split up into several child issues.
Remaining hooks in system.api.php after all the existing child issues have been resolved:
Cache: hook_cache_flush(), hook_rebuild() ===> should probably be moved to core.api.php
hook_system_themes_page_alter() ===> this one actually belongs in system.api.php
Tokens: hook_token* ==> probably should be moved to its own token.api.php file, or to core.api.php
hook_link_alter() ==> should be in menu.api.php
Config: hook_config_import_steps_alter(), hook_config_schema_info_alter() ==> should be moved to either config module api.php file (if they are used only in the config manager module), or to core.api.php. Needs evaluation.
Comment | File | Size | Author |
---|---|---|---|
#3 | 2299715-v2.patch | 229.4 KB | jhodgdon |
Comments
Comment #1
jhodgdonI feel like doing this today. I'll make a list of where I'm moving each hook...
Comment #2
jhodgdonHere is a giant patch... I realize it is going to be hard to review, so I've made a list of what I moved where. I suggest that this get reviewed, to see if you agree with my decisions, and then you can review the patch to verify that I put things where I said I did.
---- List of what was moved -- I've grouped them a bit, and put them into the files in this order:
hook_cron(), callback_queue_worker(), hook_queue_info(), hook_queue_info_alter() => core.api.php
hook_hook_info(), hook_module_implements_alter() ==> core.api.php
hook_module_preinstall(), hook_modules_installed(), hook_module_preuninstall(), hook_modules_uninstalled() ==> core.api.php
hook_install(), hook_uninstall(), hook_install_tasks(), hook_install_tasks_alter() ==> core.api.php
hook_update_N(), hook_update_dependencies(), hook_update_last_removed() ==> core.api.php
hook_updater_info(), hook_updater_info_alter() ==> core.api.php
hook_requirements() ==> core.api.php
hook_mail(), hook_mail_alter(), hook_mail_backend_info_alter() ==> core.api.php
hook_cache_flush(), hook_rebuild() ==> core.api.php
hook_schema(), hook_schema_alter(), hook_query_alter(), hook_query_TAG_alter() ==> core.api.php
hook_tokens(), hook_tokens_alter(), hook_token_info(), hook_token_info_alter() ==> core.api.php
hook_link_alter() ==> core.api.php
hook_config_import_steps_alter() ==> core.api.php
hook_stream_wrappers(), hook_stream_wrappers_alter() ==> Made a new file: core/modules/system/file.api.php
hook_file_download(), hook_file_url_alter() ==> file.api.php
hook_file_mimetype_mapping_alter() ==> file.api.php
hook_archiver_info_alter() ==> file.api.php
hook_filetransfer_info(), hook_filetransfer_info_alter() ==> file.api.php
hook_js_alter(), hook_library_info_alter(), hook_library_alter(), hook_css_alter() ==> theme.api.php
hook_element_info(), hook_element_info_alter() ==> theme.api.php [note this file is for both render and theme hooks]
hook_page_build(), hook_page_alter() ==> theme.api.php
hook_theme(), hook_theme_registry_alter(), hook_template_preprocess_default_variables_alter() ==> theme.api.php
hook_ajax_render_alter() ==> form.api.php
hook_form_alter(), hook_form_FORM_ID_alter(), hook_form_BASE_FORM_ID_alter() ==> form.api.php
hook_batch_alter() ==> form.api.php [the batch callbacks are already in this file]
hook_data_type_info_alter() ==> entity.api.php [note: technically this is TypedData and not Entity API, but they are closely related so I thought it should be there]
hook_menu_link_defaults_alter() ==> core/modules/menu_link/menu_link.api.php
hook_menu_local_tasks(), hook_menu_local_tasks_alter(), hook_local_tasks_alter(), hook_menu_local_actions_alter(), hook_contextual_links_alter(), hook_contextual_links_plugins_alter() ==> core.api.php
hook_system_breadcrumb_alter() ==> core.api.php
hook_permission ==> core/modules/user/user.api.php
hook_countries_alter() ==> language.api.php
-------- Not moved:
hook_system_info_alter() -- not moved -- appears to be used in system.module functions, at least for now.
hook_help() -- not moved -- used by the System Help block (system module) and Help page (help module).
hook_system_themes_page_alter() -- function for the System module Themes page
Comment #3
jhodgdonSorry, previous patch missed the new file I added (file.api.php). New patch...
Comment #4
jhodgdonAdd remaining tasks, including one Novice review task and one maybe not novice, to the issue summary.
Comment #5
JeroenT@jhodgdon here is my manual review :
1.
2.
system.api.php :
These use statements are never used, so I think those can be deleted.
The same for entity.api.php :
Comment #6
JeroenTComment #7
JeroenTI first reviewed patch 1, but patch v2 looks ok to me.
Comment #8
jhodgdonThank you very much for reviewing!
Do you think the destinations make sense, or do you think we need to get someone more familiar with the hooks to make that decision?
Comment #9
JeroenT@jhodgdon,
The destinations made sense to me, but I think someone with more knowledge of the hooks should take a look at this.
Comment #10
webchickAdding a related issue that's soft-blocked on this due to re-roll hell. :)
Comment #11
webchickAaaand another one.
Comment #12
jhodgdonThanks... I hope someone can review this before too long...
Comment #15
jhodgdonI think the chances of getting this whole thing into one reviewed/non-conflicting patch is fairly small.
So I'm going to break it up into several smaller issues instead. I'll file them; they'll appear here as child issues.
Comment #16
jhodgdonThere are two to review:
#2307859: Move theme/render hooks from system.api.php to theme.api.php
#2307853: Move file-related hooks to new api.php file
I'll file others if these get done... they will start to conflict with each other.
Comment #17
jhodgdonAll the child issues are done, but there are still hooks to be moved. I'll file some more child issues soon.
Comment #18
jhodgdonJust filed a new child issue:
#2469941: Move database-related hook docs from system.api.php to a new database.api.php file
Also updating issue summary with list of other hooks still to move.
Comment #19
botrisDid a manual review of hooks mentioned in summary and all seems done:
Comment #20
star-szrThanks @boris sondagh and all. More specifically, system.api.php only contains
hook_system_themes_page_alter()
now, which is invoked from\Drupal\system\Controller\SystemController::themesPage()
.I think we can call this fixed now! I just followed #2401843: Move all *.api.php files except system.api.php out of system module directory to see where things go from here :)