Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

subdomain support: dynamic manifest #173

Closed
tacman opened this issue Apr 17, 2024 · 13 comments · Fixed by #179
Closed

subdomain support: dynamic manifest #173

tacman opened this issue Apr 17, 2024 · 13 comments · Fixed by #179

Comments

@tacman
Copy link
Contributor

tacman commented Apr 17, 2024

Description

I'm working on a project where I'd like separate PWAs for each project, which are set by subdomain. Each project will have its own logo, shortname, etc.

It is possible to make the manifest a template?

Example

pwa:
    image_processor: 'pwa.image_processor.imagick'
    manifest:
        enabled: true
        background_color: "#c026d3"
        theme_color: "#c026d3"
        name: '{{ project.name }}'
        short_name: '{{ project.shortName }}'
        description: '{{ project.description }}'
        orientation: "any"

Similarly for the icons and screenshots.

Then https://project1.example.com/manifest.json would get the project from the subdomain and populate it accordingly.

@tacman
Copy link
Contributor Author

tacman commented Apr 18, 2024

In poking around the Manifest.php code, I see that shortName and description go through a translation provider. Perhaps there's some way to configure that to change based not only on the language but on some data from the database.

Is there an example of using the translation provider? I feels hackish, or course, so it's probably not the right solution.

I was thinking of changing the pwa.yaml to pwa.php, but I don't think services like the database are even available that early in the process.

@tacman
Copy link
Contributor Author

tacman commented Apr 18, 2024

https://pwa.spomky-labs.com/experimental-features/translations

Right now, translations are using the default ('messages') domain. Perhaps that could be configurable. If false, it could even skip the translation (so they didn't show up in the debug toolbar).

But hat won only temporarily solve my problem, because custom Symfony translation providers are created during the cache warmup, I don't think there's a dynamic option. So if a project changed their description or name or whatever, it wouldn't show up in the translations until after the cache were cleared.

@tacman
Copy link
Contributor Author

tacman commented Apr 18, 2024

Maybe dispatch an event from the ManifestBuilder? Then the developer could do other things as well. That is, pass the manifest key from pwa.yaml and then the EventListener could tweak it based on environment, subdomain, etc.

Perhaps a PreManifestBuildEvent that passes in the array $config, or a PostManifestBuildEvent that passed in the Manifest $manifest object could be a solution. It would require injecting the EventDispatcher into ManifestBuilder. Would you accept a PR for that?

My example use cases is an Museum Audio Tour system, each museum I imagine will want their own "app", with logo, name and description, and probably screenshots (hmm). Or a Citizen Photojournalism project, each region or newspaper would want their own app as well. Pretty much for any multi-tenant website.

WDYT?

@tacman
Copy link
Contributor Author

tacman commented Apr 21, 2024

@Spomky Would you accept a PR for emitting events before and after creating the manifest? Any guidance?

I think it'll allow some interesting and powerful integrations, like validation, translations, and of course custom text and image options.

@Spomky
Copy link
Member

Spomky commented Apr 22, 2024

Hello @tacman,

The translation feature is not finished. Domain is not supported and it will always use the default locale.
In the end, I think the bundle will serve static files with dedicated paths such as /site.{_locale}.webmanifest. Or maybe I will let the user decide. I'm not sure about this question.
Anyhow, I do not think this is the way you should go.

Regarding the events, I am ok with that 👍🏼. This will be a nice addition to send the Manifest/Service Worker or any other Asset Mapper objects and let the developer hook onto it.

@tacman
Copy link
Contributor Author

tacman commented Apr 22, 2024

Domain is not supported and it will always use the default locale.

I'm hoping you'll make that configurable, including false. KnpMenuBundle does something like that.

https://github.com/KnpLabs/KnpMenuBundle/blob/8cf6329e63e4bb8a14336c6facaa8d5f48f80f52/Resources/doc/i18n.rst

Should I add the events and dispatch and submit a PR?

@tacman
Copy link
Contributor Author

tacman commented Apr 22, 2024

OK, I've added 2 classes

namespace SpomkyLabs\PwaBundle\Event;

use SpomkyLabs\PwaBundle\Dto\Manifest;
use Symfony\Contracts\EventDispatcher\Event;

/**
 * Dispatched during the ManifestBuilder::create method, after the configuration is denormalized
 */
class PostManifestBuildEvent extends Event
{
    public function __construct(private Manifest $manifest)
    {
    }

    public function getManifest(): Manifest
    {
        return $this->manifest;
    }
}

<?php


namespace SpomkyLabs\PwaBundle\Event;

use Symfony\Contracts\EventDispatcher\Event;

/**
 * Dispatched during the ManifestBuilder::create method, before the configuration is denormalized
 */
class PreManifestBuildEvent extends Event
{
    public function __construct(private array $config)
    {
    }

    public function getConfig(): array
    {
        return $this->config;
    }
}

And tweaked the builder

<?php

declare(strict_types=1);

namespace SpomkyLabs\PwaBundle\Service;

use SpomkyLabs\PwaBundle\Dto\Manifest;
use SpomkyLabs\PwaBundle\Event\PostManifestBuildEvent;
use SpomkyLabs\PwaBundle\Event\PreManifestBuildEvent;
use Symfony\Component\EventDispatcher\EventDispatcherInterface;
use Symfony\Component\Serializer\Normalizer\DenormalizerInterface;
use function assert;

final class ManifestBuilder
{
    private null|Manifest $manifest = null;

    /**
     * @param array<string, mixed> $config
     */
    public function __construct(
        private readonly DenormalizerInterface $denormalizer,
        private readonly EventDispatcherInterface $dispatcher,
        private readonly array $config,
    ) {
    }

    public function create(): Manifest
    {
        if ($this->manifest === null) {
            $this->dispatcher->dispatch(new PreManifestBuildEvent($this->config));
            $result = $this->denormalizer->denormalize($this->config, Manifest::class);
            assert($result instanceof Manifest);
            $this->dispatcher->dispatch(new PostManifestBuildEvent($this->manifest));
            $this->manifest = $result;
        }

        return $this->manifest;
    }
}

How do I wire the Dispatcher in services.php?

return static function (ContainerConfigurator $configurator): void {
    $container = $configurator->services()
        ->defaults()
        ->private()
        ->autoconfigure()
        ->autowire()
    ;

    /*** Manifest ***/
    $container->set(ManifestBuilder::class)
        ->args([
            '$config' => param('spomky_labs_pwa.manifest.config'),
            '$dispatcher' => ???
        ])
    ;

I usually use the new Reference(), but I'm doing it in a the bundle, I'm not familiar with how to wire it here.

Thx.

@Spomky
Copy link
Member

Spomky commented Apr 22, 2024

Looks good to me.
Normally, there is no need to change the service definition. The autoconfig statement tells Symfony to inject the service if it is available.
Instead of private readonly EventDispatcherInterface $dispatcher, you should have private readonly null|EventDispatcherInterface $dispatcher,. If null, the NullDispatcher should be set.

I will prepare a PR for that.

@Spomky Spomky linked a pull request Apr 22, 2024 that will close this issue
4 tasks
@tacman
Copy link
Contributor Author

tacman commented Apr 22, 2024

Thanks!!

@tacman
Copy link
Contributor Author

tacman commented Apr 22, 2024

Hmm, I'm wondering how to solve something. When I compile the assets, including manifest, there is no subdomain.

So it'll work fine in dev, but how do I compile/cache the manifest if it's dynamic? e.g. test.example.com could load the manifest from test.manifest.json and the short_name could be test, but how do I indicate that during the bin/console assetmap:compile?

@tacman
Copy link
Contributor Author

tacman commented Apr 22, 2024

Drat, this is going to be complicated. :-( Obviously the manifest needs to be cached, but maybe not during the AssetMapper compile. For example, it could be cached in redis. But this is more complicated than I'd thought!

@tacman
Copy link
Contributor Author

tacman commented Apr 23, 2024

Can you reopen this? #179 added an event during the compilation, but I wanted to add an event during the build step.

tacman added a commit to tacman/pwa-bundle that referenced this issue Apr 23, 2024
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 25, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants