Deprecate module_set_weight()

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

So are you saying that currently in Drupal 8, the order of execution of hooks is deterministic and guaranteed by the dependency declarations in the .info.yml files? And that this execution order is both documented and tested in core so we can be sure it works as intended?

What about two modules A and B which both depend on module C (the hook provider) but not on each other? Which gets fired first, module A's hook or module B's hook? Is the execution order currently guaranteed in this case? (And what IS the order? Alphabetical? Or depends on the class loader and can vary from system to system? Documented and tested?) Do we now have some mechanism other than module weights to alter this execution order if the default order is a problem?

What about conditional dependencies? For example, module A can use module B if it happens to be available, but module A is not dependent upon module B. And IF module B is installed, module A's hook must fire after module B's hook. How can we guarantee this order of execution since there is no dependency declaration involved?

I've never liked the module weight system, but it has been necessary in the past for situations like these, and will continue to be necessary unless there is some equivalent means to alter the default order of execution of hooks, no matter how that order is calculated. In the mean time, even the current module weight system is broken because there is no longer a way to set a module's weight relative to another module (other than to poke into the config directly - module_get_weight() was removed and nothing has replaced it).

Well, if by "autoloader" you're not referring to the PSR-4 autoloader (also known as 'class loader'), then I don't know what you mean. And googling "drupal autoloader" doesn't tell me anything that I don't know - it just tells me about the PSR-4 autoloader or the D7 autoloader module which implements PSR-4 under Drupal 7.

Regardless, your patch doesn't eliminate the concept of module weight, it just makes it inaccessible to the developer. That's not a solution - that's just a hindrance.

If you want to deprecate module_set_weight(), then:
1) You need documentation which explains exactly how hook execution order is determined, and exactly how to modify that execution order.
2) You need tests to show that the execution order is deterministic - i.e. that the order is exactly the same in every environment and is completely controlled by Drupal core, not by external factors like PHP version or Symfony version.
3) You need a change notice to explain how the Drupal core module weight system has changed, and how to use the new system.
4) You need to remove the concept of module weight from the core.extension configuration, and provide an upgrade path for all the contributed modules which currently use the module weight stored in this configuration.

And as far as hook_module_implements_alter() is concerned - how is the order of execution of this hook determined? Seems like a circular reference here - the only way to specify the execution order of hooks is to define a hook which is executed in an unknown order ...

In the meantime, we're left with a dysfunctional module weight system where contributed modules can't find out the current weights so they specify arbitrarily large or arbitrarily small weights in the hope that they will be enabled after/before other modules.

I tried to collect some usecases for changing the weight:

• mcapi.install: Alters local tasks
• smart_ip_ban.module: Does a module_set_weight in the .module file :(
• new_relic_rpm:
  

    // Set New Relic RPM module's weight to very low so we can trigger job state
    // changes early. This can be important in cases like hook_cron().
    module_set_weight('new_relic_rpm', -20);
  

• workspaces.install:
  

    // Set workspace earlier to alter entities before anyone else does.
    module_set_weight('workspace', 10);
  
  

• comment_notify:
  

    // Set module weight low so that other modules act on the comment first.
    module_set_weight('comment_notify', 10);
  

• date_repeat:
  

    // Make sure this module loads after date_api.
    module_set_weight('date_repeat', 1);
  

Reading through all of them I cannot find yet an instance where module_implements_alter wouldn't be the better way to handle that.

Note: module_implements_alter is not triggered on module_implements_alter, see \Drupal\Core\Extension\ModuleHandler::buildImplementationInfo

Maybe instead of setWeight() for extension we can convince developer to use only "weight/order" for hooks via hook_module_implements_alter()

Re-rolled patch against 9.4.x

The function module_set_weight() does not properly get deprecated.

The patch has style guide violations.

Fixed CS errors.

Hello! I didn't understand what's the objective of these issue now. Could you please explain me ?

It looks like duplicate of #1808132: Move module_set_weight() into ModuleHandler::setWeight(), add ModuleHandler::getWeight() to replace missing functionality

Rerolled patch against #22

Rerolled patch against #22

#26 is an empty file.

@longwave

Sorry , unfortunately it was an empty file. I will upload the patch in a while.

Sorry , unfortunately #26 it was an empty file.
Again Rerolled patch against #21 with diff

Rerolled patch against #30, worked on Custom Commands Failed issues

Rerolled patch against #31, worked on Failed issues

Reroll the patch #33 with Drupal 9.5.x

We cannot add new deprecations to 9.5.x since beta1 was released, this will have to go into 10.1.x now.

Reroll the patch #36 with Drupal 10.1.x

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.

Fix page cache tests.

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 implementing this hook. There's no way to make sure the execution order if multiple modules implements the same hook and try to change the execution order.

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 #3485896: Hook ordering across OOP, procedural and with extra types i.e replace hook_module_implements_alter

@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 no replacement. 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 any 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. We are not trying to pull the rug out from under you. I honestly believe the use cases have been replaced.

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 causing module set weight to fail 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 conver 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.

Further we don't take module weight into account during runtime so I'm not sure it does what you expect.

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

As usual I think donquixote hit the root of the issue. This no longer has any effect runtime on hook order without a container rebuild.

I'm not sure we want a runtime way to rebuild the container like that.

I've still not seen a real example where order, reorder and a service provider won't work.

Also since we now store the implementations in keyvalue there is yet another escape hatch for ordering even if it is not covered by the bc promise.

I wonder if that means we move the weight to .info and remove module set weight without replacement and mention the tools for ordering mentioned here.

That gives modules a way to set default weight and we have the other tools for ordering hooks.