Spomky-Labs / pwa-bundle

PHP library for generating a full featured PWA manifest
https://pwa.spomky-labs.com
MIT License
28 stars 1 forks source link

add ManifestBuilder events, #173 #183

Open tacman opened 3 weeks ago

tacman commented 3 weeks ago

Target branch: 1.2.x Resolves issue #173

Dispatches events during the manifest build step. For example, if there's no description in pwa.yaml, get it from composer.json

final class PwaConfigListener
{
    #[AsEventListener(event: PreManifestBuildEvent::class)]
    public function onPreBuildManifest($event): void
    {
        $config = $event->getConfig();
        if (empty($config['description'])) {
            $composerData = json_decode(file_get_contents(__DIR__.'/../../composer.json'), true);
            $config['description'] = $composerData['description']??'missing description!';
            $event->setConfig($config);
        }
    }
Spomky commented 3 weeks ago

Hello @tacman,

I am not in favor of an event that could alter the configuration. It may lead to undesired behavior. Also, it is possible to tweak the configuration using env vars or have a bundle extension that implements PrependExtensionInterface.

tacman commented 3 weeks ago

Hmm. I quite like the idea of an event altering the configuration.

I know I can write a script that literally alters the pwa file, but because Symfony's YAML parser removes comments, I didn't like that approach. Plus, I'd have to constantly rerun it.

I've temporarily giving up on a different manifest depending on the subdomain, but I have some ideas. But for things like getting the name / description / screenshots, I was indeed hoping to intercept the Manifest Builder to make adjustments.

What undesired behavior are you concerned about, other than BC breaks and bugs the application developer may introduce?

Spomky commented 3 weeks ago

Hmm. I quite like the idea of an event altering the configuration.

The problem is that you bypass the configuration validation and there are other means to tweak it (prepending the extension is the way I like).

I am wondering if the URLs (paths/params) could also by translated. Not only for screenshots ('app.screenshot.1' => 'assets/screenshots/landing_page.png'), but also e.g. links for shortcuts (at least the _locale parameter). Not an easy task, but could solve use cases

tacman commented 3 weeks ago

But prepending happens in the compiler pass, and is more difficult to deal with than in an event listener.

The validation could have a priority so that it validated the listener modifications. It would allow the user to have their own validation, like requiring a description ot a mobile screenshot.

On Thu, Apr 25, 2024, 2:12 PM Florent Morselli @.***> wrote:

Hmm. I quite like the idea of an event altering the configuration.

The problem is that you bypass the configuration validation and there are other means to tweak it (prepending the extension is the way I like).

I am wondering if the URLs (paths/params) could also by translated. Not only for screenshots ('app.screenshot.1' => 'assets/screenshots/landing_page.png'), but also e.g. links for shortcuts (at least the _locale parameter). Not an easy task, but could solve use cases

— Reply to this email directly, view it on GitHub https://github.com/Spomky-Labs/pwa-bundle/pull/183#issuecomment-2077875932, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEXIQKX4RKAT2ZVL3YNWW3Y7FBPPAVCNFSM6AAAAABGUTIHEGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANZXHA3TKOJTGI . You are receiving this because you were mentioned.Message ID: @.***>