Deprecate module_set_weight()

Can someone explain to me how you can control the order of hooks when two modules both implement hook_module_implements_alter()? I have tried making one module a dependency of the other but it didn't help. It seems the only thing I can do is to set the weight of my module to ensure my hook_module_implements_alter() runs before the other module's hook_module_implements_alter().

The #7 still points to other issue to fix first - #1808132: Move module_set_weight() into ModuleHandler::setWeight(), add ModuleHandler::getWeight() to replace missing functionality →

I don't think #7 is saying that other issue has to go in before this one. I think it's just saying it's wrong.

• I rerolled #38 for 11.x
• removed a few more usages which can be handled with hook_module_implements_alter()
• added the trigger error for the deprecation
• added a legacy deprecation test

Fixed phpstan issue.

The last submitted patch, 45: 2968232-45.patch, failed testing. View results →

Fix page cache tests.

The last submitted patch, 47: 2968232-47.patch, failed testing. View results →

Once again a non-sense no value adding issue to kill a function. Why? Why the whole discussion, wasted man-hours? The suggested alternative hook_module_implements_alter is not a replacement, you cannot have control of the module weights/

Please re-read TR's comment #2968232-6: Deprecate module_set_weight() → . He listed imported use cases.

I'm not convinced that this function should be removed. I also don't see any reason. Why remove something that doesn't break anything, only to break things?

I'm not convinced that this function should be removed. I also don't see any reason. Why remove something that doesn't break anything, only to break things?

It's not because you disagree with an issue that it should be closed, I mean I think it needs to be discussed still.

I agree with @alexpott and @dawehner in the initial patches here, that having an explicit weight system is not the most robust and that it would be a lot better for modules to decide what other modules they should be fired after/before as that is a lot more explicit, manually setting weights can lead to conflicts and if I understand this issue correctly, we should see if we can module weight system not have explicit numbers but rather be configured on their dependencies instead, as the research by @dawehner suggests.

Back to needs work for a reroll and to fix the remaining test fails.

Hi added a reroll using patch #2 as @borisson said. Please revise.

For CC failures.

we should see if we can module weight system not have explicit numbers but rather be configured on their dependencies instead

Is that work being done somewhere? If so, that work should block this, right? If not, this work definitely shouldn't be merged, right? Until that refactor is done, module_set_weight() is critical functionality with no replacement.

Right? I'm not missing anything?

Fixed issues in #53 according to https://www.drupal.org/pift-ci-job/2690801 →

Seems to have test failure.

I took #47 and turned it into a MR. I'm going to look at some of the test fails.

I think we need sign-off from a framework manager that we want to remove this API.

This patch removes some another uses of the function module_set_weight

Needds to review more in deep testing fail for d7 migration

Note hook_module_implements_alter() will not work if you need your hook_preprocess_HOOK() to run after some other module's preprocess hook. Preprocess hooks are not invoked by the module handler and are not present in hook_module_implements_alter().

So removing this API will be problematic if you have a soft dependency on another module's preprocess hook, ie the module is not a requirement, but if it is present you need to preprocess something it also preprocesses, after it has done so. You don't want to declare a hard dependency in module .info. Altering module weight is the only way this will work, right?

It needs some rework in context of landed 📌 Hook ordering across OOP, procedural and with extra types Active

@johnpitcairn I am pretty sure you're supposed to use this https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Render%21... for preprocess.

This needs a pretty thorough recollection.

hook_module_implements_alter is now deprecated so we should use the hook ordering parameters.

As elin yordanov, johnpitcairn, myself and others have pointed out, module_set_weight() does not have a replacement. hook_module_implements_alter() is not a replacement. hook_theme_registry_alter() is definitely not a replacement. There is no reason to remove this function, especially when it has . I genuinely don't understand why so many people are gunning for this function. The "problem/motivation" is simply a non sequitur:

Module weights are from a time before autoloaded code and the ability to sort by dependency.

Lots of things are "from a time before autoloaded code". Just being old does not mean it should be removed. This issue should be closed or at least postponed until a real replacement that actually handles all of its functionality is in place. Or rationale is provided for why it should be removed.

Module weights are still relevant

hook_module_implements_alter is now deprecated so we should use the hook ordering parameters.

We have hook ordering attributes, but the order in which these are discovered and applied still depends on module order, and thus module weight. We may introduce new metta ordering mechanisms, but for now we are still in the same situation as with hook_module_implements_alter().

There are also other subsystems (plugin discovery, service discovery, event subscribers) that depend on module order and thus module weight. Event subscribers have their own weight/priority system, but the base order is still module order.

This said: It is not clear to me that configuration is the best place to set these weights.
Instead, there could be a system to declare module weights programmatically and not store explicit weights in config.

Dependency order is brittle

It was suggested here that modules should be ordered based on their dependencies. I don't like it, I suspect it to be brittle/fragile. If a new version of a module adds or drops a dependency, suddenly event subscribers are invoked in a different order.

Unclear issue goal

Also, the issue description proposes to deprecate and remove module_set_weight(), but it does not mention removing the actual weights the in core.extension.yml configuration. But then we find arguments as if we actually plan to remove these stored weights in the configuration.

Proposal: Keep module weights, provide alternative ways to set it

What we could do is o provide a service method to set a module weight, to use instead of the function.
This could be part of ModuleInstaller, and it could have a similar effect as installing or uninstalling a module, with a container rebuild.
However, as long as install and update hooks are procedural, there is no further damage from module_set_weight() being procedural too.

We could also provide a way for a module to set its own weight in its *.info.yml, instead of having to call something in hook_install().

First of all, part of deprecating this is identifying use cases and providing replacements if possible.

Second, hook_module_implements_alter was explicitly for the situations identified above, you would just need some module exists checks.

Hook theme registry alter is the hook explicitly to reorder preprocess hooks.

There have not been any ordering scenarios identified that cannot be managed with modern solutions.

The only time that hmia or htra fall down is when two modules order the same implementation, but there is nothing preventing the other modules from changing their weight too.
The ordering parameters on hooks will resolve 99% of all cases, ReorderHook will solve 90%of the remaining.

Beyond that we can use service providers.

If you have a use case that isn't covered by these please write a test here showing that case, that would be the only thing we need to make the case to keep module set weight on module handler.

As I mentioned in my comment hmia itself has been deprecated so it's not a replacement, but the replacements should convert your cases as well.

If you can show with a test a scenario where this is needed it's easy to preserve, but we do need an explicit test for that functionality before adding it back to the public api.

As for the overarching question why.

It is because includes are hard to test, generally fairly fragile and the weight system is more of a work around than a solution. If you show your use cases we can improve the overall system.