Do not load translation files on every admin request
thefuxia opened this issue · 8 comments
\WP_Featherlight::run() loads the translation files for every admin request, including all AJAX requests, updates, dashboard and so on. This is a waste of resources. Much less than many other plugins, but still …
There are only two places where the files need to be loaded:
- The plugin page, hook:
admin_head-plugins.php. - When the checkbox label is displayed. This could be done directly in that place.
I've often wondered about how to reduce the overhead with regard to loading translation files, but I'm relatively unfamiliar with how the translation functions in WordPress actually work. So far the best I've been able to do is isolate it to the admin or front end based on how the strings are being used.
Do you know of any code that would be a good example for using load_plugin_textdomain in a more contextual way like this? Is it safe to just use it directly on those pages by checking a page hook or get_current_screen?
You could move the language handling into its own class (it is a separate task) and then either register its load() method for specific hooks and/or pass the object instance into the View object where it can be called directly.
Here is an example for such a class: toscho/t5-libraries/Core/I18n/Language_Loader.php.
Why do you always put callback registration and business logic into the same class? These are separate tasks. When you'd move the registration to the controller WP_Featherlight_Admin_Meta, you could reuse the language loader in other projects. A single business object shouldn't know anything about the rest of the control flow.
Why do you always put callback registration and business logic into the same class?
Mostly ignorance, probably. Would you mind showing me an example of how they should be separated?
A very simple example is the Mlp_User_Settings_Controller. It gets the View and the save callback (a model) per constructor and registers just the callbacks for the hooks. You can reuse the View, the Model and even the Constructor. The latter is something that I don't follow very strictly in most projects, but keeping the callback registration separate from everything else – view and business models – is very useful, because the resulting code is easy to read, to test and to change.
That means also that there should be no code executed in a file with a class declaration: defined( 'ABSPATH' ) || exit; doesn't belong to such a file.
Hmm... I think I follow, but I'm not 100% sure I understand how to implement something like that correctly. It's probably more complicated than usual since this plugin doesn't autoload classes right now.
I've also run up against issues with passing dependencies around in plugins that are more strictly OOP/MVC like you're describing. The way I'm handling it in this plugin is probably pretty hacky/stupid... Actually, even the newer methods I've implemented seem problematic. I looked into using a dependency injection container but it seems kind of difficult to do in PHP 5.2.
I'll read through the code in the plugin you linked to a bit and see if I can wrap my head around it for the next release. This plugin is pretty basic, so it's probably the best one for me to learn more about this stuff before trying to implement it on larger projects.
Yeah, no need to hurry. :) Stay away from DI containers as long as possible. It is too easy to make the code more difficult to read with those. Usually, you set up dependencies via interfaces. That's enough for most cases.
DI containers are used when the object creation is expensive and should wait as long as possible. You can get around that problem often with smaller objects or a different architecture. Anyway, this will never be a problem in this plugin.
Ah! That article on interfaces is really helpful. I'll work through this as soon as I have a chance. Thanks again for all the help. I'm closing this out for now and I'll work on the code cleanup in a new issue.