How to use the decorator feature for Asset methods

[gaufde]

Hello,

I'm not sure if this is the correct place to post something like this, so feel free to redirect me if needed.

I am trying to generate a stub so that PhpStorm doesn't complain that read() is not a method of Kirby\Filesystem\Asset when doing this: asset('logo.svg')->read().

My interpretation of the docs is that the decorator feature was designed to handle this, so I edited my /site/config/config.php:

return [
    'debug' => true,
    'email' => [
        'transport' => [
            'type' => 'smtp',
            'host' => 'localhost',
            'port' => 1025,
            'security' => false
        ]
    ],
    'lukaskleinschmidt.types' => [
        'decorators' => [
            \Kirby\Filesystem\Asset::class => [
                'read' => [
                    '@return string|false',
                ],
            ],
        ],
    ],
];

However, it doesn't work as I would expect. If I do a diff with the generated types file before and after I added the above to my config, I see that there is no change:

For fun, I also tried editing the /plugins/kirby-types/config.php file to add:

        Asset::class => [
            'fields' => [
                '@return' => 'string|false',
            ],
        ],

But that didn't seem to have an effect either. So, is there something I am doing wrong or is this a bug?

[lukaskleinschmidt]

The issue is that the read method does not exists on the Kirby\Filesystem\Asset itself. Rather it gets called with the magic __call method dynamically on s underlying Kirby\Filesystem\File object. So the decorators do not work in that case.

Usually you could do something like this for such dynamic classes:

namespace Kirby\Filesystem
{
    /**
     * @mixin \Kirby\Filesystem\File
     */
    class Asset
    {
        // ...
    }
}

This nothing this plugin can currently provide.
But perhaps an interesting concept to add.

[gaufde]

Oh interesting. I knew read() was a method of Kirby\Filesystem\File but I didn't realize that meant it should be inserted in a different manner compared to the methods dynamically inserted via closures.

Would it make more sense to ask the core team to add the @mixin annotation directly to Kirby\Filesystem\Asset?

[lukaskleinschmidt]

It would make sense to have this in the core. Not sure where their stance is on using @mixin though

[gaufde]

Okay! I'll close this topic here and then approach the Kirby team about the possibility of adding @mixin.

Do you know where the best place to approach them about this is? I think the options are GitHub, Discord, the forum, and https://feedback.getkirby.com/, but it isn't exactly clear to me what is the preferred place to reach the Dev team about something like this.

[lukaskleinschmidt]

Github is probably a good place to start. You could also try to directly create a PR for that if you can.

Feel free to reference this issue and you can leave it open. I'm considering adding the @mixin functionality it to the types plugin as it is probably helpful in other cases as well