 Let me record it. This session is from Daniel Kinsler about converting an extension to the new hook system. Hello everyone. Yeah. Doing a workshop with this setup is a little bit odd. I really like to just have everyone in the room and have a conversation while working on code. I hope we can make it work anyway. If you have any questions or comments, ideally write on Telegram. You can also write on IRC, and I'll see it just in the main hackathon channel. If you write on the YouTube chat, I should also get your message, but it will be copied over manually. So let me take a minute. Just for me to get any sense of who's here and how many people are attending, please just say hi using one of the channels that we have available within the next few minutes. So just so I get a sense, if there's new people or people I know, or people who are just dropping by, or what brings you here, that would be helpful for me. So this session is intended for people who maintain extensions and who are interested in converting their extension to the new hook system. The new hook system was introduced in MediaWiki 1.35, but we have not yet really started to convert extensions to it. We are getting to a point where this is becoming more interesting and more urgent, and the idea is to deprecate the old system and remove it in the not too far future, meaning one or two years, I suppose, until it is entirely gone. So far, I'm not seeing any messages from anyone, so I guess I'm just talking to myself. I will keep talking because this is recorded and perhaps it's going to be useful for someone in the future. If you have been working on extensions in the last, I don't know how many years, you have been using the extension.json system. If you will probably know that the way to register hook callbacks is to use the hook section in extension.json. The main idea of the new hook system is really to get away from static methods here, to non-static methods on an object instance. This has several advantages. It allows for dependency injection into the hook handler. It allows hook handlers to share a state in a clean way on the hook handler object, and it allows us to use proper interfaces to represent the signatures of hooks. I'm seeing the first hellos on the chat. That's nice. So I'm not entirely alone. Cool. Please do ask questions and interrupt me at any time. Today, the example I want to work on today is Category Tree. Category Tree is an extension that I originally wrote before I joined Wikimedia. I think there's still like an author notice from 2006 or something on there. I have not actually touched this extension in a long time. It has been maintained by other people since, and I don't actually know the code in detail anymore. This is really me coming in from the outside, just with the mindset of looking at how the hooks are defined and how they should be changed to use the new hook system without having deep knowledge of the code itself. There's two phases to this. In the first phase, we can blindly convert to the new system, which allows us to use a new system, but does not really allow us to benefit from it much. Once we have done that, we can start to see what converting away from static callbacks now allows us to change about the extension, change how the extension is written in general, moving it away from relying on global state. That's the idea. For the first phase, for just converting to the new system, we can simply... Well, first of all, if we move away from static methods towards instance methods, we have to create an instance. In order to do that, we introduce a new section into the extension.json file, which is called hook handlers. In most cases, we only have one hook handler, maybe two, maybe three, but not like a huge amount, not a different one for every hook that is not necessary. We have to name our hook handlers. Since we're just starting out with one, we call it main or default or my hooks. You can also call it Frank, nobody cares. So let's call it default. And the hook handler is defined as an object spec. So you give it a class name. That's the minimum we have to do. And the class name will just be the name of the place where all the hooks are currently. I mean, in typically in an extension, all the hooks, hook handlers, all the static methods for handling hooks would be in a single class. Maybe they're distributed over two classes. It really doesn't matter much, right? Okay, so once we have done this, we are ready to convert our first hook function. So the first hook function would be article from title and it implements the article from title hook. So what we do here is we will implement the respective hook handler interface, which is just the name of the hook with the word hooks attached to the end. And so my IDE is telling me, okay, we can do that, but now there's something missing, right? It's saying, oh, you're not actually implementing the methods that are in this interface. So let's look at what the article from title hook interface looks like. It has a single method, which is called on article from title. The name of the hook interface and the name of the method on that interface is always derived from the hook name and it's always derived in exactly the same way. You have for the interface, you have the word hook attached to the end and for the method name, you have the on attached to the front. The only exception is hook titles that contain columns. Sometimes the hook names contain columns, then they would be replaced by underscores. We will see that in a minute. Okay, so the method we have to implement is on article from title and I will copy the entire signature. Oh, maybe not yet. I will do that in a minute. So I know the method name now and the name that we actually had is article from title without the on, right? So I'll have to find that article from title and I'll just replace the name, right? Okay, so now again, the ID is complaining. Why is the ID complaining? It is complaining because this method signature here is much like the method signature here, but not exactly. Here we have three parameters with no type hints and here we have two parameters with type hints. The method signatures have to match exactly. So I will just copy what is here to end, of course. Oh yeah, it's still complaining that it's static, right? So it should not no longer be static, okay. Are there any questions so far? The reason that there are no type hints here is that as I explained in the presentation I gave two hours ago here, we are trying to move away from things like title and also we are trying to recover other old classes like article, which just exposed too much of the internals of Media Wiki. We want to convert hooks to only getting what they absolutely need, which makes it easier to change when and where they are called. It is easier to keep compatibility without having to replace the hook. And sometimes this means replacing the hook entirely, but if we don't have strict type hints and we know exactly how these hooks are used, we can sometimes get away with supplying a fake object here that just implements the methods that are actually used by extensions. That will, but of course that wouldn't work if we had a strict type hint here. Having the type declaration just in the dog block will still have the type enforced by IDEs and it will also have it enforced by our continuous integration tests via the fan utility. These will complain if you're using the wrong type. So there's still type safety here enforced when merging patches, but we don't have them enforced in at runtime, which gives us a flexibility to supply fakes for backwards compatibility in the future. Okay, all that being said, this is actually done, right? We are now properly implementing this interface. And now we can just say that instead of the method here, we just specify which hook handler to use. And we just give the name of the hook handler and then the new hook system will just look at this hook handler and look for the appropriate method on that hook handler and we'll call it. The method name will just be this name with on on the front. And all the hooks will all use the same instance, right? This instance here that is specified here will be created exactly once. Okay, so first hook down, next hook. Now here we have a hook that has columns in the name. We did that for some hooks in the past when we wanted to kind of have the class name in the hook name. That turns out to be a pretty bad idea for various reasons because it means that if we want to split up the class, the functionality of the class into multiple parts and now the hook gets called from somewhere else the name is completely misleading. Okay, anyway, so we have to put in the underscores here. We look at the hook interface. We have the method signature. Go back, look for the old method name which was on special whatever in progress. Okay, so this is the old signature. We put in the new one, remove the static. So when changing the method signature in this way, of course we have to make sure that we're not changing the parameter names so everything is still in place. The ID isn't complaining so I guess I didn't replace anything. So done, right? Next, this is a pretty boring process. There's not that much to do here. I may, I think I'll skip forward and skip to a most more interesting example. So we are sure to get to it. Copy signature, back on the unspecial. That's the one we already did. It's the other one. Based on the other signature, remove the static modifier. We're done. So the ID is complaining about these method names. This is not new. That was also the case before. They used the wrong casing. This way to spell getDB key is kind of sane, but it's wrong. So we have to change it to the less sane but correct spelling with capital DB and lower case key. I think I can just do that while I'm here. Trying to read the backlog on the chat. Am I reading this correctly that there's no comments so far? Yes, as far as I can see. No, no, not here. Okay, thank you. Everything is crystal clear. Yeah, so far it's kind of embarrassingly simple, right? It's kind of embarrassing. Yeah, it's not very, very complicated. Okay, let's skip forward because I want to actually, I want to demonstrate something. Let's look at the before page display one. So we do before page display hook message signature there. Note that this actually has the return type declared and we also have to declare that return type. Until some time in the past, hooks were generally expected to return a Boolean. This has changed already a couple of years ago. So only some hooks are supposed to return or are allowed to return false to avoid operation and prevent other implementations of the hook, other handlers of the hook from executing. This is only supported from some hooks and not for this one. So this declares the return type to be void and any handler trying to return a value will fail. So, okay, what's the old name? The old name here is add headers, completely unrelated to the actual hook. And I'm noting that the, there's actually two hooks using that. And in the old system, I could have two hooks using the same handler function. That is no longer possible. So what I do is I just create a new function, a new handler function here and I just call the other thing. I just calls self headers and I keep that static for now. It could, maybe this can also become an instance method in the future, but for now I'll just keep it static just to avoid any more confusing work, more complexity in this initial patch. Okay. Oh, I forgot to replace the up here. So, okay, we have now also done four page display. Now we move on to the next hook and we try to do the same thing as before, right? We put it up in here at the hook thing at the end and now the IDE doesn't find it. It's not there. And if I go to my code search and I try to find where this hook is actually called in core, right? It's not there. What happened? Well, this hook is not defined by core. It's defined by another extension and that extension is mobile frontend. And if I look at the mobile frontend extension there's actually two cases here but I don't unfortunately don't have an example for the second case. So, where am I? Okay, so if I look at mobile frontend I can see that this hook name is mentioned once in a place where this hook is called and it's called the hook call is the old style way, right? With hooks run. And there is no hook interface. So what now? And the answer to that is simply nothing. Just keep it a static method on the same class. No problem. You can still use static functions as hook handlers. It's not preferred but since mobile frontend is not offering a hook interface yet we will just keep it the old way. And that's it, right? We just do nothing and we'll keep working. We can have static handlers and instance method handlers on the same class. They don't, that doesn't hurt. Now, something that I can't demonstrate here right now is what if the hook interface is defined by another extension but the extension isn't there. What happens? In that case, you have to, let's pretend that is the case for a minute. I will undo this in a second. You have to create a separate handler class here like mobile frontend hook handler. And handlers, let's just call it that. And we'll also call the class this. And then in this other handler, you can use the hook handler interface defined by the other extension. The reason that this has to be in a separate class is that if that other extension isn't there, if it's not installed, then the interface will not be found. And if you put it on the main handler, creating the main handler, instantiating the main handler is going to fail because that interface is missing from this, because the extension isn't there. Now, if you have it separate, the hook system will only try to create the hook handler when the hook is called. But since the extension isn't there, the hook is never called, so that makes it safe. I suppose since most extensions are not converted to the new hook handler system yet, I'm not aware of any that is exposing handlers the new way. If someone can tell me about an extension that does, perhaps we can look at an example as well. But this is just to say that it is possible with the new system to register a handler for another extension that may or may not be here. Okay, we just found that we don't have to do anything here. This will just stay static. Since Jason doesn't allow comments, I can't make a comment here. Always found this a bit annoying, but okay. So let's do output page parser output as we did before. I can do this quite quickly, import, look at the signature, the signature, find the old implementation, which is just called parser output, and replace the signature. Note that we renamed the parameter, which is confusing, and now it's good. Yeah, what happens when the extension providing the hook is not installed? Well, if the extension that provides the hook is not installed, nothing will call the hook. And if nothing calls the hook, the handler system does not try to instantiate the hook handler, so nothing happens. Does that answer your question? I'm just looking through the chat whether I missed any other questions. Any warnings that you could help see the missing piece? I'm not sure what you mean by missing piece. So there's two cases here, I suppose. One is the other extension is an optional requirement. Well, if it's optional, there's no missing piece, right? If it's not there, it's not used, done. If it is a requirement, then that dependency should be declared, I think, also an extension JSON somewhere up here, right? And then the extension registry would complain. Yeah, so if the dependency is required, you will declare that, and the extension registry will complain. The other way, of course, to do that is you can just put the interface from the other extension on your main handler, and if the other extension isn't there, that will just explode, right? Now we get unrelated chatter about the work adventure in the chat. Okay, cool. Anyway, while I was looking for this method, I stumbled across the initialize method. There is a initialize function up here on the handler, and this is not mentioned here. This is not a hook handler, right? It is, let me find it. It's an extension function. Extension functions are like the ancient version of hook handlers. It's like the hook handler before hooks even existed. It's a callback function that is invoked after the setup phase is complete. And I will go on a little tangent here because this is something that might bite people. It is unrelated to the new hook system, but interesting, perhaps anyway. So one thing that is frequently done here is manipulate settings, maybe full in defaults that depend on other extensions or on other settings dynamically here. And this is problematic because this function is called when service instances in the main service container may already have been created. And so this is basically too late. Some service objects may not be picking up the changes you make here. Not a problem for category tree in particular, but it has turned out to be a problem for, oh, if I recall correctly, central auth. So one way we could fix this is we could make this a handler for the MediaWiki services hook. So what one thing we could do is we implement MediaWiki services hook, which is invoked immediately after the service container is created, which would be before nearly all service instances are created. There's two exceptions. There will be two services already created, which is the hook runner itself and the configuration factory. And everything else will be picking up on changes that you would make here. Okay, so MediaWiki services hook is on your hook. I'll copy the signature back. And we'll replace this. I could just, let's see if this is called anywhere directly. Nope. So I can just change the signature here to the new thing. And this means that we can remove the extension function up here and instead register a handler for the MediaWiki services hook would do. Okay, services. Wow, I've not seen that before. Okay, my ID just crashed. We will be back in a second with the regularly scheduled program. Sorry about that. Any questions in the meantime, comments, thoughts? No questions, but I have comments. You should read your notifications on MediaWiki because you have tons of notifications when you switch it up, I saw them. It's not a question, it's just to something that I recognize. Sorry, notifications on MediaWiki? Oh, yeah. Yeah. Yeah. We have a big backlog. Otherwise there is no questions on both channels. Be comfortable, I am following the channels. There is no questions yet. Okay, cool. Thank you. Yeah, I would actually be happy about questions, right? To me, a workshop works best when it's actually a conversation, more like me talking all the time. But I realized that that is difficult if we are not in a room together or at least in a shared video chat. Okay, how much did I lose? What happened? Are you asking me the time, you name it? No, I was just saying, my ID just crashed and I lost my last few edits but that's not terrible. I should still have like 20 minutes, right? Less than 20, but we are close. We'll put this in, copy the entire signature. That will work. In the future, we could look into a better system for manipulating configuration. This is under discussion, but for now it's important to at least do it at the correct time and no longer do it in the extension functions thing. So we can remove this and that should still work. If we do output page output, we did. So this can also be default. We have one that stays static and we still have three to go. I will do this quite quickly if I can. Signature, on skin, blah, maybe a quick side note about these reference parameters. Some of them are actually out parameters where you can modify the parameter add to a list for instance or replace an instance. Others used to be declared as reference parameters in the past because of historical reasons in PHP 3, objects would be copied if you didn't pass them by reference and that was absolutely terrible. But that has been fixed a long, long time ago. So we are no longer passing objects by reference just as a matter of course. And with the new hook interfaces, we fixed this in quite a few places. And so that's another change that you may see when you convert to the signature of the function in the hook interface. Okay, on skin side bar, it's done. Barser first call init is also somewhat interesting. We'll look at that in a second. Set hooks, okay. So we'll replace this. Now what this does is it sets our handlers for parser hooks or parser functions. And these again are static methods on this class. And we want to move away from global state. We want to move away from the static functions. We want to be able to use dependency injection on the instance and share a state on the instance. So I will just make them non-static, right? Instead of, I can just do this. And then I will look at where they are defined. Farser function is here. I'll just remove the static modifier. Right, here's the other one. When I do this, I should probably search whether there are direct callers elsewhere in this code. And okay, it turns out there are some, but they're in the same class. So since we are converting everything to be non-static, we can actually do this. I'm getting a lot of things here. Okay, yeah, Lukas likes the new system. Cool, very nice. Nestle, when you're copying things, could you copy them in the main channel? That way I don't have to switch channels. Thanks. That was I, okay. So this is still red, because we have not converted this method to non-static yet. And I think it's actually the last one left. Yes, this will go next, right? So in parser function, we already have it. We can just do this. Okay, do we have direct callers to this function? All right, so the next one will be lasso, lasso function. Okay, let's go. Output page, major category, links hook. I'm not finding it. Why am I not finding it? And this one actually I genuinely don't know. I expected to find that. Maybe I'm confused somehow, because I think that is in core and it should have an interface. Let's see what happens. Does exist. So either I got confused or my IDE got confused. Oh yeah, I mistyped that was all. Okay, fine. All right. This is something that I noticed working with PHP storm in this file. It is PHP storm is very smart about working with JSON and it will add and remove commas when you copy lines and insert stuff, which is kind of convenient. But for some reason it wants to add a comma at the very end of the file breaking everything. That's, I have no idea why that is happening. I noticed this yesterday. Okay, so this should still work. We have converted everything, I think. Nearly wait, default, default. Okay, let's see if it's still, of course, that is cached. And if we just do edit and preview, still have the category tree working on the page. If we go to category page, navigate up, I have the categories here to main zoom in so that seems to work, no exceptions. So this is basically the first step, quickly make a commit, sorry. And I will push this commit later and I'll clean it up, make sure. Well, actually I will push it now and I will make sure if it fails any tests or anything, I'll clean it up a little and I will paste, I can actually give you the link right now because why not? So I will paste it to the telegram. Whoops, books, workshop, much. No guarantees that this is 100% complete and correct, but it seemed to work. Now, we have like 10 minutes left. I would be happy to have a conversation about this or I could try and push on a little and have maybe get an idea of what this means, what this just freed us up to do. What would you like to do with the last 10 minutes? Oh yeah, good question, good question, Alex. Is there a situation when I want to use multiple hook handlers rather than one? Well, one situation, as I explained earlier is when I have a hook interface that comes from an optional dependency that may not be there sometimes. So then that has to be separated out. Another situation is, and we didn't really get into this. Now that everything on my hooks here are in instance methods, right? I could try and look at no longer using globals instead have config injected into the constructor. Similarly, I think we get a database somewhere. Yeah, so here, instead we could inject a load balancer, right? So we would have this, load balancer connection ref. And, oh, actually, yeah, that is actually a good point. I did not talk about this yet. So this is a load balancer. And we will want a constructor that actually takes, that initializes this, right? But we have to somehow tell the hook system to also supply this parameter. So we will have to specify which services to pass to the constructor here in the hook handler's specification in extension.json. And I will come back to your question, Alex, in just a second because there is a connection. And, oh yeah, with load balancer, typically the name of the service is just a class name, but here it's DB load balancer because load balancer itself is a very generic name. So this would make sure that we get the load balancer from the service container injected into the constructor here. Now, the more things I inject for the different handlers, the more cluttered the constructor gets. And if I notice that there is some things that are only used by one set of hook handlers and there's other services that are just used by another set, it makes sense to split the handler, right? Maybe I have one handler that does storage related stuff and that has like database connection and whatever storage manager is injected. And maybe the other one is for skin stuff. And so I have localization related things injected into that handler. And so I have all the skin related handlers grouped into one object and all these storage related handlers grouped into another. And yeah, just looking at what services and what configuration variables are used by which handler function will give you good guidance at what to group together. Does that answer your question? Okay, I'm reading through the comments but I only see unrelated chatter. Did I miss any questions? No, no you didn't miss. Yeah, but it's very good that this came up so I could actually show how dependency injection works here. We have still have like five minutes or so. Trying to think what else I could get into. One thing we could look at is how to inject configuration variables. There's actually two ways. Well, we can ask for the config object. We can just ask for it to be injected and remember it in a local variable added here. This is the main config. And just having the config here and accessing it directly is the easiest way, right? So, or something like here on Parasafras call in it, right? I want to know whether the category G allow tag is set. So I would do something like this, right? So that allows me to just access configuration variables without relying on globals. Having the full config object hanging around here is not super nice using a service options object as we do in other places is somewhat nicer. But yeah, I think getting into that here is a little bit overkill. Using the config object instead of global variables is already a big step forward. The same thing here, of course, we could now go through and convert all this access to global variables to the config. Any other questions? Oh yeah, I'm just looking at a bug, by the way. You may appreciate the fact that this is called sidebar up here. It's called sidebar down here. This is called bar. So the bug would be that this no longer works, right? As this got renamed, bug fixed. That's the kind of small thing that we'll have to pay attention to. It was especially tricky in this case. You know, I only noticed because of syntax highlighting on the environment was telling me that the variable was unused. So sometimes these kinds of hints can help. I am a, you know, if you can work with a full-fledged IDE, do it. It has so many advantages and it makes things so much faster and so much safer. Just give it a try for a week or so. And I don't know about other things but PHPStorm has, for instance, the mode that can emulate VI. If your fingers are totally stuck to the world of VI, you can just keep using all the weird key combos there. So highly recommended. Doesn't have to, I mean, PHPStorm probably is the best that I have used. But of course it's not open source. There is open source options. But yeah, it is way faster and safer than using. Just an editor. I'm not getting into editor post territory. I didn't recommend Emacs. And actually I keep, you know, I use VI for some stuff. For instance, I use it to write my commit messages. It's still for just logging in somewhere and quickly editing a file or two is my go-to thing for doing refactoring in an environment with like 15,000 files. Not so much. Okay. Any other questions? Brian says whatever editor works best for you is the best one. That is true, but it requires you to actually give you things a chance and actually take the time to try them out properly. That coming from me because I hate giving you things a chance. Change is just horrible and throws me off. But, you know, I never look back to be honest. Ooh, I see chatter about pause and the chat. I never got into properly using it, but I think pause is absolutely awesome. I agree. I really need to play with it more. If there's no more questions, then I think I'll just stop here. Thank you all very much. Thank you. I hope. All right. Bye. Bye-bye.