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.

CommentFileSizeAuthor
#3 2299715-v2.patch229.4 KBjhodgdon
#2 2299715.patch219.21 KBjhodgdon
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jhodgdon’s picture

Assigned: Unassigned » jhodgdon

I feel like doing this today. I'll make a list of where I'm moving each hook...

jhodgdon’s picture

Status: Active » Needs review
FileSize
219.21 KB

Here 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

jhodgdon’s picture

FileSize
229.4 KB

Sorry, previous patch missed the new file I added (file.api.php). New patch...

jhodgdon’s picture

Issue summary: View changes

Add remaining tasks, including one Novice review task and one maybe not novice, to the issue summary.

JeroenT’s picture

@jhodgdon here is my manual review :

1.

Function Moved to Status
hook_cron() core.api.php OK
callback_queue_worker() core.api.php OK
hook_queue_info() core.api.php OK
hook_queue_info_alter() core.api.php OK
hook_hook_info() core.api.php OK
hook_module_implements_alter() core.api.php OK
hook_module_preinstall() core.api.php OK
hook_modules_installed() core.api.php OK
hook_module_preuninstall() core.api.php OK
hook_modules_uninstalled() core.api.php OK
hook_install() core.api.php OK
hook_uninstall() core.api.php OK
hook_install_tasks() core.api.php OK
hook_install_tasks_alter() core.api.php OK
hook_update_N() core.api.php OK
hook_update_dependencies() core.api.php OK
hook_update_last_removed() core.api.php OK
hook_updater_info() core.api.php OK
hook_updater_info_alter() core.api.php OK
hook_requirements() core.api.php OK
hook_mail() core.api.php OK
hook_mail_alter() core.api.php OK
hook_mail_backend_info_alter() core.api.php OK
hook_cache_flush() core.api.php OK
hook_rebuild() core.api.php OK
hook_schema() core.api.php OK
hook_schema_alter() core.api.php OK
hook_query_alter() core.api.php OK
hook_query_TAG_alter() core.api.php OK
hook_tokens() core.api.php OK
hook_tokens_alter() core.api.php OK
hook_token_info() core.api.php OK
hook_token_info_alter() core.api.php OK
hook_link_alter() core.api.php OK
hook_stream_wrappers() file.api.php OK
hook_stream_wrappers_alter() file.api.php OK
hook_file_download() file.api.php OK
hook_file_url_alter() file.api.php OK
hook_file_mimetype_mapping_alter() file.api.php OK
hook_archiver_info_alter() file.api.php OK
hook_filetransfer_info() file.api.php OK
hook_filetransfer_info_alter() file.api.php OK
hook_js_alter() theme.api.php OK
hook_library_info_alter() theme.api.php OK
hook_library_alter() theme.api.php OK
hook_css_alter() theme.api.php OK
hook_element_info() theme.api.php OK
hook_element_info_alter() theme.api.php OK
hook_page_build() theme.api.php OK
hook_page_alter() theme.api.php OK
hook_theme() theme.api.php OK
hook_theme_registry_alter() theme.api.php OK
hook_template_preprocess_default_variables_alter() theme.api.php OK
hook_ajax_render_alter() form.api.php OK
hook_form_alter() form.api.php OK
hook_form_FORM_ID_alter() form.api.php OK
hook_form_BASE_FORM_ID_alter() form.api.php OK
hook_batch_alter() form.api.php OK
hook_data_type_info_alter() entity.api.php OK
hook_menu_link_defaults_alter() menu_link.api.php OK
hook_menu_local_tasks() core.api.php OK
hook_menu_local_tasks_alter() core.api.php OK
hook_menu_local_actions_alter() core.api.php OK
hook_contextual_links_alter() core.api.php OK
hook_contextual_links_plugins_alter() core.api.php OK
hook_system_breadcrumb_alter() core.api.php OK
hook_permission() user.api.php OK
hook_countries_alter() language.api.php OK

2.
system.api.php :

use Drupal\Component\Utility\String;
use Drupal\Core\Utility\UpdateException;

These use statements are never used, so I think those can be deleted.

The same for entity.api.php :

use Drupal\Component\Utility\String;
JeroenT’s picture

Status: Needs review » Needs work
JeroenT’s picture

Status: Needs work » Needs review

I first reviewed patch 1, but patch v2 looks ok to me.

jhodgdon’s picture

Issue summary: View changes

Thank 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?

JeroenT’s picture

@jhodgdon,

The destinations made sense to me, but I think someone with more knowledge of the hooks should take a look at this.

webchick’s picture

Adding a related issue that's soft-blocked on this due to re-roll hell. :)

webchick’s picture

jhodgdon’s picture

Thanks... I hope someone can review this before too long...

jhodgdon queued 3: 2299715-v2.patch for re-testing.

Status: Needs review » Needs work

The last submitted patch, 3: 2299715-v2.patch, failed testing.

jhodgdon’s picture

Title: Move core hooks from system.api.php to core.api.php » [meta] Move core hooks from system.api.php to core.api.php
Status: Needs work » Active

I 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.

jhodgdon’s picture

There 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.

jhodgdon’s picture

All the child issues are done, but there are still hooks to be moved. I'll file some more child issues soon.

jhodgdon’s picture

Title: [meta] Move core hooks from system.api.php to core.api.php » [meta] Move core hooks from system.api.php to core.api.php or other files
Issue summary: View changes

Just 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.

botris’s picture

Did a manual review of hooks mentioned in summary and all seems done:

Function Moved to Status
hook_cache_flush() core.api.php OK
hook_rebuild() core.api.php OK
hook_system_themes_page_alter() system.api.php OK
hook_tokens() token.api.php OK
hook_tokens_alter() token.api.php OK
hook_token_info() token.api.php OK
hook_token_info_alter() token.api.php OK
hook_link_alter() menu.api.php OK
hook_config_import_steps_alter() core.api.php OK
hook_config_schema_info_alter() core.api.php OK
star-szr’s picture

Assigned: jhodgdon » Unassigned
Status: Active » Fixed

Thanks @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 :)

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.