feat: Plugin extension autoloader #7035

plain-solutions-gmbh · 2025-02-26T14:32:32Z

Description

I have been working on a adequate solution for an plugin autoloader.

Here is an example:

App::plugin(
    'vendor/plugin', autoloader: [
    	'cache' => true,
    	'classes' => [
    		'namespace' => 'Vendor\\Namespace'
    	],
    	'config' => [
    		'path' => './extends'
    	],
    	'blueprints', 'snippets', 'fields', 'translations',
    	'custom' => function($autoloader) {
	        $autoloader->merge([
	            "options" => [
	                "my_option" => 'foo'
	            ]
	        ]);
	    }
    ]
);

If true is passed to the autoloader, all tasks defined in the autoloader are executed.

Summary of changes

Add Autoloader class
implement to AppPlugin (not plugin class - cause info and root needs to be defined)
Add Method to AppPlugin to get allowed extensions keys

Reasoning

Plugins (if the developer use the autoloader) can be loaded from cache.
Extensions could be easly integrating into the plugin.

Additional context

I create it because of this rejected pull request
It would be nice if it's goes into V5. Cause Plugin already takes a new parameter license

Changelog

The autoloader executed if the parameter 'autoload' is either 'true' or an array. (Disabled by default)
The array contains tasks that are processed in sequence
Tasks can be modify, enable or extend. (Cache is enabled by default)
The result is cached and stored by default under ./site/cache/autoloder/{vendor}/{plugin_name}\{timestamp}.php. (Can be deactivated)
The plugin folder is modified. The cache is renewed.
Loads the classes (by default from classes folder) automatically
Fields, sections, blueprints, snippets, templates and translations are loaded from the root by default.
For other extensions is the subfolder ./config in charge.

Docs

I could write a cookbook if the feature will be applied

Ready?

In-code documentation (wherever needed)
Unit tests for fixed bug/feature
Tests and CI checks all pass

For review team

Add lab and/or sandbox examples (wherever helpful)
Add changes & docs to release notes draft in Notion

plain-solutions-gmbh · 2025-02-26T14:49:11Z

src/Plugin/Autoloader.php

+        $cache_folder = pathinfo($this->cache_file, PATHINFO_DIRNAME);
+        try {
+            Dir::remove($cache_folder);
+        } catch (\Throwable $th) {};


Fixed in commit Fix empty catch.

bastianallgeier · 2025-02-27T09:59:11Z

This looks very interesting. Give us a bit of time to review this, while we still struggle with the last v5 steps.

plain-solutions-gmbh · 2025-03-04T14:23:24Z

Todo:

use F::loadClasses to load classes.
remove cache task and move them to tasks parameter

distantnative · 2025-08-07T16:03:23Z

@plain-solutions-gmbh I finally had a chance to take a closer look again. Unfortunately, it's still quite far from being mergable. That's not to say it isn't functional, but the approach and code style differ greatly from other parts of Kirby. I'd be concerned about maintainability, etc.

It would need to be much less smart and automatic. For example, it shouldn't be a list of tasks that works with reflections. Rather, it should be a very explicit list of dedicated methods that deal with autoloading the different extensions. It shouldn't manipulate variables passed as arguments via references, but rather return values from methods and set them in the main context.

It wouldn't make sense to send you in circles working on this. That might end up being a frustrating experience, and we don't want that to be the result of such a well-intentioned code contribution. However, we also need to ensure the quality and style of the code we add to the core. We might eventually get to it ourselves, but it's not our highest priority at the moment.

plain-solutions-gmbh · 2025-08-09T12:42:45Z

That’s sad to hear.

For over three years, I’ve been working exclusively with the Kirby CMS codebase, and I thought I had largely adapted its style of writing code.

I have to admit that I demanded a lot of flexibility from this feature and, as you mentioned, ended up building in a bit too much automation. As Bastian emphasized several times during the introduction of K5, a new feature often sparks ideas for further improvements — and narrowing those down is truly a challenge.

If you could spare a little time, I’d really appreciate it if you could leave me a few line-comments on this pr. That way, I can see where I’ve strayed furthest from Kirby’s code style and work on improving the quality of my code.

Best regard Roman

distantnative

I left some in-code comments. I think you are right that the biggest "problem" currently is that the code tries to be too flexible, too magic, too smart. We have all been there ourselves (and will be again now and then), but we have learned that this usually backfires down the road. Refactoring those tasks/usertasks and magic walker methods into very concrete methods for each extension would be an important step.

src/Plugin/Autoloader.php

src/Plugin/Plugin.php

distantnative · 2025-08-10T18:57:16Z

src/Cms/AppPlugins.php

+	public function allowedExtensionsKeys(): array
+	{
+		return array_keys($this->extensions);
+	}
+


We don't need to base the logic on the available extensions. It might seem like a good idea to make it dynamic based on this, but some verbosity will go a long way here for understanding and maintaining this (even with a bit more effort). Look at classes like Kirby\Cms\Core, Kirby\Cms\AppPlugins - writing code for each extension manually helps, even if that means we have to change the code in multiple classes when we add/remove an extension.

plain-solutions-gmbh · 2025-08-17T16:24:06Z

Thank you for your detailed explanation.

I took the liberty of redesigning the autoloader based on your input. This time, I removed everything that made it confusing and slow. This means that caching is no longer necessary.

Now you can simply pass an extended autoloader class to the plugin and you're already extremely flexible.

distantnative

I think this was a good step in the right direction!

I would encourage you to even less think of flexibility. Simplicity might be the more important first goal. Once we have a simple, solid, working solution, we could still see where to make it more flexible.

One thought that came up while reviewing and I just want to share it, might not work in the end. But instead of defining a separate autoload option in the plugin registration call, couldn't we use the logic of e.g. for blueprints "if the blueprint extend is an array, use it as before. If it is a string, assume it's a directory and automatically autoload it"?

distantnative · 2025-08-17T17:58:28Z

src/Cms/AppPlugins.php

+		if ($autoloader) {
+
+			//Allow to apply custom Autoloader
+			$autolader_class = is_bool($autoloader) ? Autoloader::class : $autoloader;


Question: What for would one use a custom autoloader class?

For example: In one of my cases i used it to extend the $this->extend() with data from a Class (Which is loading in the autoloader). So i made a custom Autoloader class and modified the toArray() method.
An other case could be, that you need to load snippets from another folder than snippets.

distantnative · 2025-08-17T17:59:07Z

src/Plugin/Autoloader.php

+        $classfolder = $root . '/classes/';
+        if (Dir::exists($classfolder)) {
+            $this->_classes($classfolder);
+        }


I would move this to a loadClasses() method.

distantnative · 2025-08-17T18:00:14Z

src/Plugin/Autoloader.php

+        foreach (Dir::dirs($this->root) as $dir) {
+            if (method_exists($this, $dir)) {
+                $this->$dir($this->root . '/' . $dir . '/');
+            }
+        }


This should be less magic. We can simply list all potential extensions:

$this->loadBlueprints(); $this->loadFields(); ....

distantnative · 2025-08-17T18:03:34Z

src/Plugin/Autoloader.php

+    public function _dirWalker(string $root, Closure $fnc)
+    {
+
+        foreach (Dir::index($root, true) as $path) {
+
+            //Check if the path is active
+            if (Str::contains($path, '/_')) {
+                continue;
+            }
+
+            $file = $root . $path;
+            if (F::exists($file)) {
+                $dirname = F::dirname($path);
+                $dirname = $dirname === '.' ? '' : $dirname . '/';
+                $fnc($dirname . F::name($path), $file);
+            }
+        }
+    }


I think a less flexible and more explicit approach would be better. Reading the code, the _dirWalker method comes across quite cryptic and hard to understand. I also don't understand why many extensions would need to run the index vs. just Dir::files(). That's why IMO it would be better to leave it to each dedicated load*() method to use the appropriate code and not this meta method.

That was my initial plan. However, this makes the entire file very repetitive. I use index so that I can include all subfolders at once.

distantnative · 2025-08-17T18:07:45Z

src/Cms/AppPlugins.php

+			$extends = $autolader_class::load(
+				name: $name,
+				root: $root,
+				data: $extends ?? []


Suggestion: Instead of passing $extends to the class and merging it in the class's ::toArray() method, the class could be responsible to only gather and return autoloader extensions. And then we rather merge the explicitly passed $extends and the result of Autoloader::toArray() here.

distantnative · 2025-08-17T18:12:18Z

src/Plugin/Autoloader.php

+    public function blueprints(string $root)
+    {
+        $this->_dirWalker($root, function ($path, $file) {
+
+            $name = F::relativepath($file);
+            //Set YAML-File or they content
+            $this->extends['blueprints'][$path] = (F::extension($file) === 'yml') ? $file : $this->_read($file);
+        });
+    }


As an example, I think this could be refactored more in this direction:

Suggested change

public function blueprints(string $root)

{

$this->_dirWalker($root, function ($path, $file) {

$name = F::relativepath($file);

//Set YAML-File or they content

$this->extends['blueprints'][$path] = (F::extension($file) === 'yml') ? $file : $this->_read($file);

});

}

public function loadBlueprints(): void

{

$root = $this->root . '/blueprints';

foreach (Dir::index($root, true) as $blueprint) {

$file = $root . '/' . $blueprint;

if (F::exists($file) === false) {

continue;

}

$this->extends['blueprints'][$blueprint] = $$file;

}

}

(Just writing this on GitHub so code is probably not fully right, but to communicate the general idea)

A good point. Would that mean that an array containing all specific folders with methods? For example, with:

$folders = [ 'snippets' => 'loadSnippets' ... ]; foreach (Dir::dirs($this->root) as $dir) { if (array_key_exists($dir, $folders) === false) { $method = $folders[$dir]; $this->$method($this->root . '/' . $dir . '/'); } } ´´´

I would not try to make it that smart/magic. I know it seems very verbose and repetitive, but from our experience "boring" code like that has often paid off down the road.

plain-solutions-gmbh · 2025-08-18T07:35:50Z

Alright. Looks nicer now.
Can you tell me why the tests where failing? Is there an report to check?

plain-solutions-gmbh · 2025-08-21T18:35:17Z

One thought that came up while reviewing and I just want to share it, might not work in the end. But instead of defining a separate autoload option in the plugin registration call, couldn't we use the logic of e.g. for blueprints "if the blueprint extend is an array, use it as before. If it is a string, assume it's a directory and automatically autoload it"?

That thought occurred to me as well. Developers typically select the 'Blueprint' folder to store blueprints, etc.
If only one definition is affected, that makes sense. However, when it comes to outsourcing api and areas, etc., writing a definition for each of them would make the autoloader obsolete.

For me, the development of the autoloader is also about making it easier for developers to get started with plugin development. Writing definitions that are self-explanatory would be an unnecessary barrier.

Add ultimate autoloader

511fbf0

github-advanced-security bot found potential problems Feb 26, 2025

View reviewed changes

Fix empty catch

5787d39

distantnative changed the base branch from v5/develop to develop-minor June 24, 2025 21:16

distantnative changed the title ~~The ultimate autoloader~~ feat: Plugin extension autoloader Aug 7, 2025

distantnative reviewed Aug 10, 2025

View reviewed changes

Roman Gsponer added 3 commits August 17, 2025 18:13

Simplify

56b9599

Set autoloader after getter

011d532

Prettify

27d7387

Fix classloader

96694ee

distantnative reviewed Aug 17, 2025

View reviewed changes

Roman Gsponer added 2 commits August 17, 2025 20:47

Add suggestion

c81b09c

Simplify autoloader

e5291be

Uh oh!

feat: Plugin extension autoloader #7035

Are you sure you want to change the base?

feat: Plugin extension autoloader #7035

Uh oh!

Conversation

plain-solutions-gmbh commented Feb 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Summary of changes

Reasoning

Additional context

Changelog

Docs

Ready?

For review team

Uh oh!

Check warning

Choose a reason for hiding this comment

Uh oh!

bastianallgeier commented Feb 27, 2025

Uh oh!

plain-solutions-gmbh commented Mar 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

distantnative commented Aug 7, 2025

Uh oh!

plain-solutions-gmbh commented Aug 9, 2025

Uh oh!

distantnative left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

plain-solutions-gmbh commented Aug 17, 2025

Uh oh!

distantnative left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

plain-solutions-gmbh commented Aug 18, 2025

Uh oh!

plain-solutions-gmbh commented Aug 21, 2025

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

plain-solutions-gmbh commented Feb 26, 2025 •

edited

Loading

plain-solutions-gmbh commented Mar 4, 2025 •

edited

Loading