There are no more functions I am aware of that must reside in .module files, magic autoloading of .module files can be the source of some very hard to find bugs.
There is a simple script to assist with converting the hooks in modules and remaining functions can almost just be copy pasted.
We should deprecate the .module file extension
Important to note: This is not about deprecating the Extension type of module, just the procedural .module files.
N/A
Publish the CR only deprecation notice.
Check for .module extension files during hook collection.
Use the ExtensionFileIsConverted attribute core and contrib can use to skip deprecations.
Emit deprecations for any .module files without that attribute.
Identify documentation to update
https://www.drupal.org/docs/develop/creating-modules
https://www.drupal.org/docs/develop/creating-modules/let-drupal-know-abo...
https://www.drupal.org/docs/develop/creating-modules/adding-assets-css-j...
https://www.drupal.org/docs/develop/creating-modules/basic-module-buildi...
Update general documentation
Add requirements check: #3587107: Add requirements check for deprecated .theme file extensions: Add requirements check for deprecated .module file extensions
Test rector conversion:
https://github.com/palantirnet/drupal-rector/pull/325
https://gitlab.com/-/snippets/5975084
N/A
N/A
.module files are deprecated.
N/A
.module file extensions have been deprecated, all code in .module files should be converted to classes. See https://www.drupal.org/node/3619765 for more guidance.
| Comment | File | Size | Author |
|---|
Show commandsStart within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes
Comments
Comment #2
nicxvan commentedI added the plumbing
Comment #4
larowlanComment #5
nicxvan commentedComment #6
nicxvan commentedcan't do profile due to install_tasks and install_tasks_alter :/
Comment #7
nicxvan commentedI think this should mostly pass, I did not add the Attribute to modules that are going to be removed in 12. Any tests installing those will fail, but I don't want to risk moving the attribute to contrib.
This is on the 6 modules still being deprecated though.
I did not add it to a couple of fixtures, I'm not sure if they are needed.
One thing is the attribute has to be on a method, there are a few module files with no functions, the test ones I added a dummy, but json_api is kind of weird.
Comment #8
nicxvan commentedThe functional bits are ready for review even if it's still postponed.
Still need to do the comment search, .module shows up a lot across the codebase.
Every job will fail until toolbar and search leave.
Comment #9
berdir> One thing is the attribute has to be on a method, there are a few module files with no functions, the test ones I added a dummy, but json_api is kind of weird.
Interesting, we didn't think of modules that only have constants in them left. I know themes already landed, but what if also support it as an arbitrary string or annotation-like thing.
We could just do a str_contains on "@ExtensionFileIsConverted" or so for example within the @file block. Because the changes you have actually mean we'd lose the optimization for skip procedural.
> This is on the 6 modules still being deprecated though.
We could add the attributes/annotation to them anyway. We know we'll work through them until D13, seems acceptable to me if we want to land the deprecation.
Comment #10
nicxvan commentedYes, @catch already tentatively signed off on adding it to the modules we're working on.
I don't think we lose that much by adding the function, we could just update the attribute to support constants too.
The skip procedural was more about container size and with this change we still don't add anything to the cache.
Comment #11
nicxvan commentedComment #12
nicxvan commentedGot a few test failures after rebasing.
Comment #13
nicxvan commentedYes but I'm not super concerned we still bail on the first function anyway.
Comment #14
nicxvan commentedCreated #3622990: [pp-?] After move to contrib remove ExtensionFileIsConverted attribute and #3622992: [pp-?] After move to contrib remove ExtensionFileIsConverted attribute
Comment #15
nicxvan commentedThis should be ready for review now!
Important notes, as mentioned we do need a function for this to work, I think it's a rare enough case that we can just add a dummy function.
Also the skip scanning is not too common so I think doing a single loop is also ok.
I created follow ups for search and toolbar!
Comment #16
nicxvan commentedOk I went through and updated all comments and references to the .module extension I think are relevant.
The remaining ones are specifically talking about existing .module files for testing soon to be legacy functionality.
I'll update the few comment updates that I think warrant a closer look.
Comment #17
nicxvan commented@catch confirmed in slack we should split the comment changes out to a follow up.
Keeping this assigned to me I'll work on that in a bit.
Comment #19
nicxvan commentedToolbar was removed so I rebased.
This is ready for review.
Comment #20
xjmI had not been following this chain of issues closely. It's very satisfying to see how modern code patterns and technical debt management have made possible something that used to feel Sisyphean. 🎉
To have any chance of getting this in for 11.5/12.0, we should descope as much as possible here to get the essential bits in. We are much closer to tagging beta1 than a week ago, and unlike the exception deprecations and such, this really shouldn't happen during beta.
Comment #21
larowlanI'm in support of this, removing tag as Nic has been keeping us informed all the way through this process - so I'm confident other FMs agree
Comment #22
nicxvan commentedComment #23
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #25
nicxvan commentedComment #26
alexpott+1 for the CR. I think it is great to announce ASAP - I already have modules with no .module file - it's very nice.
Comment #27
nicxvan commentedComment #28
nicxvan commentedComment #29
nicxvan commentedI guess technically this is needs review for the cr.
Comment #30
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue. The merge request has merge conflicts and cannot be merged. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #31
nicxvan commentedComment #32
berdirNotes on the CR:
* the main CR about hooks seem to be missing, it just lists the special cases?
* hook_hook_info() is already unsupported in D12, if you're going to list it here you might want to mention that
* links should include a readable text so you know what what they are about
* Disagree with "As part of your conversion, set your module's minimum version requirement to Drupal 11.3.0". It's perfectly valid to just keep the legacy hooks. Later on you even talk about BC for non-hook functions. Can't be both. The .module files does _not_ need to be removed, it should stay and it can continue to exist even on D13, we'll just ignore it then. Of course a new major version until then that removes the deprecated stuff is allowed, but not required.
* Considering that we're linking this issue and the attribute already exists, I think we can still document to add the attribute. It won't do anything yet, but that's OK, it's just to silence the upcoming deprecation message.
Comment #33
nicxvan commentedGood feedback as always!
It was there, but I moved it up too so it's more clear.
Done!
I fixed the bare links.
Yeah that got tweaked, I agree it's not quite what I was trying to say. I've updated it to be clear that it's the easiest way forward, but not required.
Done, I had considered this and decided to leave it off, but it doesn't hurt to have it.
Comment #34
berdirThanks, CR now looks good to me and can be published IMHO. Not setting this to RTBC as it is not yet done.
My suggestion for next step would be to create an issue to update the 2-3 test .module files that should be converted per my review in the issue, but then not split this up further.
Comment #35
catchCR looks much better after the latest round of changes. I went ahead and published it. All a bit different from what we usually do but this is a unique case.
Comment #36
ressaThanks for the clean up!
Under "Remaining tasks", "Identify documentation to update" is mentioned, and I have added a few pages which mention a
.modulefile. Perhaps someone can create some child issues, and outline what needs to be done? Or should I do it?For example, there is a link in the Change Record The .module file extension has been deprecated to a DDEV script, which may seem a bit complicated for some site builders ... so it would be nice if a new "Converting from .module-file to an object oriented class" page was added under https://www.drupal.org/docs/develop/creating-modules, which outlines how to convert an existing function to use the new method.
See also #3581218-81: Deprecate .theme file extension where I made a similar request for documentation of the transition process for Themes.
Comment #37
nicxvan commentedYes, we do need to do that still, if you can start that I'll be happy to collaborate with you on that!
It's not really meant for site builders, the CR does not target them either.
That is a great idea!
I agree we need similar docs still for themes.
Comment #38
ressaThat sounds fantastic @nicxvan, thanks for fast feedback! I created #3625256: Document how to convert a function in a .module- or .theme-file to an object oriented class, and look forward to working on getting this documented with you.
I decided to group tasks for both modules and themes in the same issue, since the subject is identical, i.e. documenting OOP-conversion.