Page MenuHomePhabricator

Turn static Hooks class into HookManager service
Closed, InvalidPublic

Description

Problem statement

Hooks class is entirely static.

Proposal

Introduce HookManager which would have the same functionality as Hooks, but implemented as a proper service.

For backwards-compat, Hooks::foo() would call MediaWikiServices()->getHookManager()->foo().

Registration of hook handlers by extensions probably still needs to use the global $wgHooks array for now.

Event Timeline

daniel moved this task from Doing to To Do on the User-Daniel board.

Notes from a discussion with Brad at the DevSummit:

  • HookManager::getRunner should return per-hook-name singletons
  • wgHooks should perhaps be a magic ArrayAccess object (the HookManager itself)
    • wgHooks could start out as a regular array, and be replaced by the HookManager atr some point in Setup.php

See also replated notes on T154674

Krinkle updated the task description. (Show Details)
Krinkle added a subscriber: Krinkle.

Pending further discussion on parent task about how we, as TechCom, want to handle these three related RFCs.

Krinkle triaged this task as Medium priority.Jan 25 2018, 4:20 AM
Krinkle renamed this task from Introduce HookManager service to replace static Hooks class. to Turn static Hooks class into HookManager service.Dec 21 2018, 2:46 AM
Krinkle edited projects, added Platform Engineering; removed TechCom-RFC.

In the context of T212482, the surrounding tasks might be redundant, but I'm not sure. I think type safety and testability would now be reclaimed with the disciplines of the new system, without requiring callbacks to be created by factories or injected with services (can be claimed from the service container instead).

The HooksManager itself, though, still makes a ton of sense. Tentatively un-tagging as RFC as part of Phab clean-up around all the relating tasks. This change by itself seems simple enough and isolated to not require an RFC, but please re-tag if you disagree or if I misunderstood!

Superseded by / subtask of T240307?

yes